Function to find a substring within a string [closed]

Question

I've just completed an exercise in which I had to create a program that searches for a string within another bigger string.

If the substring exists then it is outputted to confirm and the starting index is given where the first character can be found.

Aside from printf and scanf, I am not allowed to use any other functions unless I write them myself.

My code works, however I'm concerned that I've written far more code than is necessary. WITHOUT using pointers (something I'll learn next), could I have considerably cut down the amount of code I wrote?

#include <stdio.h>
#include <stdbool.h>

int findString(char mom[], char babe[]);
int stringLength(const char string[]);
bool equalStrings(const char s1[], const char s2[]);

int main(void)
{
    char mommy[80];
    char baby[80];
    int index;

    printf("Which string do you want to search?\n");
    scanf("%s", mommy);

    printf("What do you want to search for?\n");
    scanf("%s", baby);

    index = findString(mommy, baby);

    if(index >= 0)    
        printf("Yup, baby was found in mommy at index %i\n", index);
    else
        printf("Sorry, baby not found\n");

    return 0;
}

int findString(char mom[], char babe[])
{
    int index, i, j;
    int momLength = stringLength(mom);
    int babeLength = stringLength(babe);
    char test[babeLength + 1];

    for(i = 0; i <= babeLength-1; i++)
    {
        for(j = 0; j <= momLength; j++)
        {
            if(babe[i] == mom[j])
            {
                test[i] = babe[i];

                if(babe[0] == mom[j])
                    index = j;
            }
        }
    }

    test[babeLength] = '\0';

    if(equalStrings(test, babe) == true)
        return index;
    else
        return -1;
}

int stringLength(const char string[])
{
    int count = 0;

    while(string[count] != '\0')
        count++;

    return count;
}

bool equalStrings(const char s1[], const char s2[])
{
    int i = 0;
    bool areEqual;

    while(s1[i] == s2[i] && s1[i] != '\0'){
        i++;
        if(s1[i] == '\0' && s2[i] == '\0')
        {
            areEqual = true;
        }
        else
        {
            areEqual = false;
        }
    }
    return areEqual;
}

Answer 1

Sorry but your code is an accident waiting to happen.

1. Using a constant to declare the available space in you char arrays allows your code to be more maintainable.
2. scanf() can overflow your buffer and corrupt everything after your buffer. Forcibly set the BufferSize-1 character to NULL after you have run scanf() its the best you can do with the limitations you have be given.
3. Why are you declaring index so far up the code, always declare a variable as near to the point of first use as you can, also it could be const, which will make it clear to the reader that its value will never change.
4. Unless you compiler won't allow it declare loop variables inside the for statement i.e. for (int i = 0; i < momLength; ++i). The ++i makes no difference for simple types, but when you move on to iterators it does.
5. Always try and use meaningful variable names, i and j are not very meaningful. You might read this again in 12 months, so make it easy for yourself.
6. Now the bad news, I think you algorithm is wrong. Why are you creating a copy of a string (test)? I think it would be easier to scan through mum until you find the first character of babe (index). Then match every character of babe to the characters of mom. If you get to the end of babe then you have a match starting at index, if a character doesn't match then start scanning mum again at index + 1.
7. You need to check for zero length mom and babe values entered by the user.
8. stringLength() could go on "infinitely" As it stands if you scan a value which overflows the buffer then you could keep going until you run out of memory. You need to impose a maximum length of BufferSize.
9. And finally, stick some comments in, it will make it easier when you are getting marked and you'll get a better score.