Why is this instance of strstr giving me segmentation faults?

2
votes

Here is the full code

int count_substr(const char *str, const char *sub)
{
    char *ret;

    int a = 0; // used to tell where the pointer has moved to
    int count = 0;

    ret = strstr(str, sub);

    while (strstr(ret, sub) !=  NULL) {
        printf("The substring is: %s\n", ret);

        for (a = 0; a < strlen(sub); a++) {
            ret++;
        }

        printf("The substring after moving pointer is: %s\n", ret);
        count++;
    }

    return count - 1;  
}

I don't understand what's happening here, I'm not using the null pointer once 

strstr(ret,sub)

becomes null, so why is it giving me seg faults?

Valgrind would state that

Invalid read of size 1 and  Address 0x0 is not stack'd, malloc'd or (recently) free'd

2
c
If the first strstr() returns NULL (in ret), you are calling strstr(NULL, ...) in the while loop. - dhke

2 Answers

6
votes

You do not test if the initial call ret = strstr(str,sub); succeeds.

If it returns NULL, the next call strstr(ret,sub) will definitely invoke undefined behavior.

Furthermore, your code does not update ret correctly in the while loop, you must set ret to the first char past the match, not merely advance it by the length of the match. Here is a simpler version:

int count_substr(const char *str, const char *sub) {
    /* returns the number of non overlapping matches of sub in str */
    const char *ret = str;   
    int count = 0;

    if (ret && sub && *sub != '\0') {
        while ((ret = strstr(ret, sub)) != NULL) {
            ret += strlen(sub);
            count++;
        }
    }
    return count;
}
0
votes

You should definitely check return value of strstr (man strstr (3)):

RETURN VALUE
       These functions return a pointer to the beginning
       of the located substring, or NULL if the substring is not found.