String search & replace in C

Question

I created a function to do that, and I want to know if there is a problem I'm overlooking about it ..

void replaceStr(buffer, haystack, needle, rep)
 char *buffer, *needle, *rep;
 const char *haystack;
{
 if (!buffer || !haystack || !needle || !rep)
 return;
 size_t rep_length = strlen(rep);
 long long diff = strlen(needle) - rep_length;
 memcpy(buffer, haystack, strlen(haystack) + 1);
 while ((buffer = strstr(buffer, needle))) {
 memcpy(buffer, rep, rep_length + 1); /* +1 for the null byte */
 if (diff > 0) {
 long long i;
 for (i = diff; i <= diff; ++i)
 buffer[i] = buffer[i+1];
 buffer[i] = 0;
 }
 }
}

Answer 1

Since this is about strings, I would rather use strcpy instead of memcpy. This has the benefit, that it saves the unnecessary strlen

strcpy(buffer, haystack);

You can declare the other parameters const too, e.g.

char *buffer;
const char *needle, *rep;

The for loop is a bit confusing, because it copies just one character at offset diff, no matter how large diff is. If you want to close a potential gap, you could again use strcpy

size_t needle_length = strlen(needle);
...
strcpy(buffer + rep_length, buffer + needle_length);

The other way round - if rep is larger than needle, you need to make space for the larger replacement string. Otherwise, you overwrite parts of haystack! (Unless this is what you intended, of course.)

---

You can avoid this, when you copy from haystack to buffer as you go, something like

size_t needle_length = strlen(needle);
char *found;
while ((found = strstr(haystack, needle))) {
 /* copy haystack part until needle */
 size_t n = found - haystack;
 strncpy(buffer, haystack, n);
 /* copy replacement string */
 strcpy(buffer + n, rep);
 /* adjust pointers */
 buffer += n + rep_length;
 haystack = found + needle_length;
}
/* copy remaining haystack */
strcpy(buffer, haystack);

• Amr Ayman: Your solution actually raises a SegFault
• Olaf Dietsche: Then it needs a review too :-) I only tested it with a simple call like replaceStr(buffer, "Hello, world!", "world", "Olaf"); and this works properly. How did you test it?
• Amr Ayman: Oh, yes you're is fine. It seems I just forgot to remove strcpy(buffer, haystack) from above :)
• Olaf Dietsche: Great! I am glad, it works finally.