Error in very similar functions while manipulating strings in C

user_12911856 0
Viewed 61

I am learning C and I came across a problem while manipulating strings. In a problem I was solving I was supposed to write a function to take a string and a character and delete all occurrences of the given character, and then I had to return the modified string. The function I wrote is this:

char *strdelc3(char *s, char ch){
for(int i=0,j=0; i!=strlen(s)+1; ++i)
 if(s[i]!=ch){
  s[j]=s[i];
  ++j;
 }
return s;
}

And when I pass a string and a character as arguments:

main(){
char s[20]="mary";
puts(strdelc3(s,'r'));
}
The output is: Segmentation fault(core dumped),

which by my research means I am accessing memory that does not belong to me. The solutions had this code:

char *strdelc4(char *s, char ch){ /*Correct*/
int i,j;
for(i=0, j=0; s[i]!='\0'; ++i)
 if(s[i]!=ch){
  s[j]=s[i];
  ++j;
 }
s[j]='\0';
return s;
}

Which is basically equal to mine, however this piece works fine! Since the two codes are so similar I don't see anything wrong with mine... I have already studied both but I don't see what is the problem with mine... Could someone help?

c-strings c
2 Answers

The problem is in your loop conditional:

i!=strlen(s)+1

You're attempting to use strlen(s)+1 here to avoid having to add the null byte. But in doing so, strlen(s) changes once you move the terminating null byte.

On the first 4 iterations through the loop, strlen(s) is 4. On the next iteration, i is 4 and strlen(s)+1 is 5 so you enter the loop again. You then move the null byte. Now on the following iteration, strlen(s) is 3 and i is 5. The conditional is still true so you keep going, walking off the end of the string. This invokes undefined behavior which in this case causes a crash.

The second piece of code addresses this issue by explicitly looking for the null byte based on the index of i and appending a null byte to the resulting string after the loop.

user_16871190

An even simpler version of the code would use the do - while loop instead of for():

char *strdelc5idx(char *s, char ch){
    int i=0, j=0;

    do {
        if (s[i] != ch)
            s[j++] = s[i];
    } while (s[i++] != 0);

    return s;
}

This will copy the string-terminating NUL character before testing it, so you needn't have a separate instruction for it. However, that requires deferring the i++ incrementation so that the loop condition at the end of an iteration tests the same character which was copied in the iteration. As a result the i++ and j++ do no longer appear together, which may make this code less legible at a first glance.

An equivalent pointer version:

char *strdelc5ptr(char *s, char ch){
    char *d = s, *f = s;

    do {
        if (*f != ch)
            *d++ = *f;
    } while (*f++);

    return s;
}
user_7336370
Related