1
\$\begingroup\$

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;
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$

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);
\$\endgroup\$
  • \$\begingroup\$ Your solution actually raises a SegFault \$\endgroup\$ – Amr Ayman Nov 15 '14 at 13:08
  • \$\begingroup\$ 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? \$\endgroup\$ – Olaf Dietsche Nov 15 '14 at 14:51
  • \$\begingroup\$ Oh, yes you're is fine. It seems I just forgot to remove strcpy(buffer, haystack) from above :) \$\endgroup\$ – Amr Ayman Nov 19 '14 at 15:19
  • \$\begingroup\$ Great! I am glad, it works finally. \$\endgroup\$ – Olaf Dietsche Nov 19 '14 at 16:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.