Is there any way to improve this function which replaces occurrences of substrings with another string in an malloc allocated string?

I'm very new to C and I decided to make a function called str_replace which replaces strings inside strings which have been made using malloc. It appears to work but can anyone find any room for improvements.

Any advice will be appreciated. I'd like to know if people think finding the number of occurrences to calculate the new string size was a good idea and if my use of pointers makes sense.

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

char * str_replace(char * string,char * find,char * replace){
    //Replaces each occurence of a particular string inside a malloc-made string with another string
    char * pos = string;
    size_t replace_size = strlen(replace);
    size_t find_size = strlen(find);
    size_t excess = replace_size - find_size;
    //Get number of occurences
    int x = 0;
    while (1) {
        pos = strstr(pos,find);
        if (pos == NULL){
            break;
        }
        pos++;
        x++;
    }
    if (!x){ //No occurences so return with original string
        return string;
    }
    char * new_string = malloc(sizeof(char)*(strl开发者_运维百科en(string) + excess*x + 1)); //Plus 1 for null termination
    pos = string; //Reset pointer
    char * string_track = string; //Used to move around string.
    char * new_string_begin = new_string; //Begining of new string to return
    while (1) {
        pos = strstr(pos,find);
        if (pos == NULL){
            strcpy(new_string,string_track); //Fill in remainder
            break;
        }
        pos++;
        size_t seg_len = pos-string_track; //Length between found string and last
        strncpy(new_string,string_track,seg_len); //Copy string to just before pos
        new_string += seg_len - 1; //Go to point for replacement by adding what was added to pointer.
        strncpy(new_string,replace,replace_size);
        new_string += replace_size; //Go to point after what was replaced
        string_track = pos + find_size - 1; //Original string should go to point after found string.
    }
    free(string); //Remove old string
    return new_string_begin; //return new string
}

int main (int argc, const char * argv[]) {
    char * string = malloc(sizeof(char)*21);
    strcpy(string,"No,yes,no,yes,no,yes");
    printf("%s\n",string);
    string = strreplace(string, "no", "nope");
    printf("%s\n",string);
    free(string);
    string = malloc(sizeof(char)*21);
    strcpy(string,"No,yes,no,yes,no,yes");
    printf("%s\n",string);
    string = strreplace(string, "hello", "nope");
    printf("%s\n",string);
    free(string);
    string = malloc(sizeof(char)*21);
    strcpy(string,"No,yes,no,yes,no,yes");
    printf("%s\n",string);
    string = strreplace(string, "yes", "y");
    printf("%s\n",string);
    free(string);
    return 0;
}

Overall that's pretty solid, some things I would suggest would be

1) don't name the first argument "string" .. I think thats a little risky (doesn't string.h define a "string" symbol?)

2) I wouldn't free the old string in the str_replace function, it wasn't allocated by that function, so it shouldn't free it IMO, not really important for this example but it's generally a good habit. That would also mean theres no need for the "string_track" variable, since the first arg is just a copy of a pointer to a string, you can muck with it and just not care where it ends up cause it gets thrown away when the function exits.

One suggestion: If you intend to call the function with the syntax string = str_replace(string, "no", "nope");, then I would suggest changing the first argument to a char**. That way, you can modify the pointer directly instead of having to assume that the user is using that particular calling notation. Since you free the original string, the following code block is dangerous:

char* str1;
char* str2;
str1 = malloc(SIZE_1);
/* Write something into the buffer here.. */
str2 = str_replace(str1, "from", "to");
free(str1);  // Whoops! Double free!

By changing the first parameter to a char**, the user can safely free the same pointer that they used with malloc regardless of how they called the function. In that case, you can either make your function return void or simply return a copy of the pointer to the original string (*string).