C buffer overflow

Question

I tried to make a function that replaces all occurrences of str1 in a text t with str2 but I keep getting a "buffer overflow" error message. Can you please tell me what is wrong with my function?

#include <stdio.h>
#include <string.h>
#include <assert.h>

//replace all *str1 in *t with *str2, put the result in *x, return *x
char * result(char *str1,char *str2,char *t)
{
    char *x=NULL,*p=t,*r=t;
    x=malloc(400*sizeof(char));
    assert(x!=NULL);
    x[0]='\0';
    r=strstr(t,str1); //r is at the first occurrence of str1 in t, p is at the beginning of t
    while(r!=NULL)
    {
        strncat(x,p,r-p); //copy r-p chars from p to x
        strcat(x,str2); //copy str2 to x
        p=r+strlen(str1); //p will be at the first char after the last occurrence of str1 in t
        r=strstr(r+strlen(str1),str1); //r goes to the next occurrence of str1 in t
    }
    strcat(x,p);
    return x;
}

I did not used the gets() function to read any char array.

My compiler is gcc version 4.6.3

---

I updated the code, it works, but the result is not the as expected.

main() function:

int main(void)
{
    char *sir="ab",*sir2="xyz",*text="cabwnab4jkab",*final;
    final=result(sir,sir2,text);
    puts(final);
    free(final);
    return 0;
}

printed string:

b

I expected cxyzwnxyz4jkxyz

Answer 1

It looks like you've got your strncpy arguments mixed up: the second argument is the source string, not the limit on the number of chars to copy, which should be the third argument:

 strncpy(x, p, r - p); // copy r - p chars from p to x

Furthermore, you want to use strcat instead of strcpy. Using strcpy, you'll just overwrite the contents of the result with the replacement string, every time. Using strcat, be sure to initialize the result with \0 before starting.

Finally, you're returning a reference to a local variable x from your function: you can't do this as the memory isn't usable after the function returns.