C: string replace in loop (c beginner)

Question

I need to replace a strings in some text. I found this function here at stackoverflow:

char *replace(const char *s, const char *old, const char *new)
{
    char *ret;
    int i, count = 0;
    size_t newlen = strlen(new);
    size_t oldlen = strlen(old);

    for (i = 0; s[i] != ''; i++) {
        if (strstr(&s[i], old) == &s[i]) {
        count++;
        i += oldlen - 1;
        }
    }

    ret = malloc(i + count * (newlen - oldlen));
    if (ret == NULL)
        exit(EXIT_FAILURE);

    i = 0;
    while (*s) {
        if (strstr(s, old) == s) {
            strcpy(&ret[i], new);
            i += newlen;
            s += oldlen;
        } else
            ret[i++] = *s++;
    }
    ret[i] = '';

    return ret;
}

This function works for me fine for single replacement. But i need to replace a whole array "str2rep" to "replacement". So what i'm trying to do(im just a beginner)

****
    #define MAXTEXT 39016
    int l;
    int j;
    char *newsms = NULL;
    char text[MAXTEXT];
    char *str2rep[] = {":q:",":n:"};
    char *replacement[] = {"?","
"};

    strcpy((char *)text,(char *)argv[5]);

    l = sizeof(str2rep) / sizeof(*str2rep);

    for(j = 0; j < l; j++)
    {
        newsms = replace(text,(char *)str2rep[j],(char *)replacement[j]);
        strcpy(text,newsms);
        free(newsms);       
    }

    textlen = strlen(text);

This code even works locally, If I build it from single file... But this is asterisk module, so when this is being executed, asterisk stops with:

* glibc detected * /usr/sbin/asterisk: double free or corruption (!prev): 0x00007fa720006310 *

Answer 1

Issues:

1. ret = malloc(i + count * (newlen - oldlen)); is too small. Need + 1.
   Consider what happens with replace("", "", ""). If your SO ref is this, it is wrong too.

2. Questionable results mixing signed/unsigned. count is signed. newlen, oldlen are unsigned.
   I think the original code works OK, but I do not like using the wrap-around nature of unsigned math when it can be avoided which is what happens when newlen < oldlen.

   // i + count * (newlen - oldlen)
   size_t newsize = i + 1;  // + 1 for above reason
   if (newlen > oldlen) newsize += count * (newlen - oldlen);
   if (newlen < oldlen) newsize -= count * (oldlen - newlen);
   ret = malloc(newsize);
   

3. Insure enough space. @hyde Various approaches available here.

   // strcpy(text, newsms);
   if (strlen(newsms) >= sizeof text) Handle_Error();
   strcpy(text, newsms);
   

Minor

2. No need for casts

   // newsms = replace(text, (char *) str2rep[j], (char *) replacement[j]);
   newsms = replace(text, str2rep[j], replacement[j]);
   

3. Better to use size_t for i. A pedantic solution would also use size_t count.

   // int i;
   size_t i;