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] != '\0'; 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] = '\0';

    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[] = {"?","\n"};

    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

I will suggest something that to me looks a bit more clear as an alternative, in place of a proper dynamic string implementation. Exception handling is left as an exercise for the reader to add. :)

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

char *appendn(char *to, char *from, int length)
{
  return strncat(realloc(to, strlen(to) + length + 1), from, length);
}

char *replace(char *string, char *find, char *sub)
{
  char *result = calloc(1, 1);

  while (1)
    {
      char *found = strstr(string, find);
      if (!found)
        break;

      result = appendn(result, string, found - string);
      result = appendn(result, sub, strlen(sub));
      string = found + strlen(find);
    }

  return appendn(result, string, strlen(string));
}

int main()
{
  const char text[] = "some [1] with [2] to [3] with other [2]";
  char *find[] = {"[1]", "[2]", "[3]", NULL};
  char *sub[] = {"text", "words", "replace"};
  char *result, *s;
  int i;

  result = malloc(sizeof(text));
  (void) strcpy(result, text);
  for (i = 0; find[i]; i ++)
    {
      s = replace(result, find[i], sub[i]);
      free(result);
      result = s;
    }

  (void) printf("%s\n", result);
  free(result);
}

Answer 2

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;