# String search & replace in C

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;
}
}
}


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);

• 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? – Olaf Dietsche Nov 15 '14 at 14:51
• Oh, yes you're is fine. It seems I just forgot to remove strcpy(buffer, haystack) from above :) – Amr Ayman Nov 19 '14 at 15:19