find- replace string function in C

Question

There are a lot of find/replace functions available on the internet, but i can't find why this is not working...( my own solution ) Here is what i tried

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

char* strrpl(char *str, char* find, char *replace)
{
    int i;
    char *pt = strstr(str, find), *firstStr;

    firstStr = (char* )malloc(100 * sizeof(char));
    // copy just until i find what i need to replace
    // i tried to specify the length of firstStr just with pt - str
    strncpy(firstStr, str, strlen(str) - strlen(pt));  

    strcat(firstStr, replace);
    strcat(firstStr, pt + strlen(find));

    for(i = 0; i < strlen(firstStr); i++)
        str[i] = firstStr[i];
    return str;
}

int main()
{
    char *s, *s1, *s2;
    s = (char* )malloc(100 * sizeof(char));
    s1 = (char* )malloc(100 * sizeof(char));
    s2 = (char* )malloc(100 * sizeof(char));
    scanf("%s", s1);
    scanf("%s", s2);
    scanf("%s", s);

    printf("%s", strrpl(s, s1, s2));
    return 0;
}

The compilation gives me the error "segmentation fault" but i can't figure what memmory is he trying to alloc and he can't. I overrided a memory block or something? Please help :)

Thanks

Answer 1

What about memmove ?

Assuming that src is long enough when shifting to the right :

char *str_rpl(char *src, char *pattern, char *rpl) {
  char *dest, *origin;
  char *found = strstr(src, pattern);
  
  if(!found) {
    return NULL;
  }

  /* Shifting right or left to fit new size */
  dest = found;
  origin = found + strlen(pattern);

  if(strlen(rpl) > strlen(pattern)) {
    dest += strlen(rpl);
  }   
  else {
    origin -= strlen(rpl);
  }   
  memmove(dest, origin, strlen(origin) + 1);

  /* Replacement */
  memcpy(found, rpl, strlen(rpl));

  return found;
}

Answer 2

You have not checked for the return value of strstr. Try the below code.

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

char* strrpl(char *str, char* find, char *replace)
{
    int i;
    char *pt = strstr(str, find);
    char *firstStr;
    if(pt == NULL){
        printf("cannot find string \n");
        return NULL;
    }

    firstStr = (char* )malloc(100 * sizeof(char));

    // copy just until i find what i need to replace
    // i tried to specify the length of firstStr just with pt - str

    strncpy(firstStr, str, strlen(str) - strlen(pt)); 
    strcat(firstStr, replace);
    strcat(firstStr, pt + strlen(find));

    for(i = 0; i < strlen(firstStr); i++)
        str[i] = firstStr[i];
    return str;
}

int main()
{
    char *s, *s1, *s2, *s3;
    s = (char* )malloc(100 * sizeof(char));
    s1 = (char* )malloc(100 * sizeof(char));
    s2 = (char* )malloc(100 * sizeof(char));
    s3 = (char* )malloc(100 * sizeof(char));

    scanf("%s", s);//input string
    scanf("%s", s1);//string to find
    scanf("%s", s2);//string to replace

    s3 = strrpl(s, s1, s2);
    if(s3 != NULL)
        printf("%s \n",s3);

    return 0;
}

Answer 3

I overrided a memory block or something?

You have:

1. A potential buffer overflow when you allocate firstStr. Who says the result will be less than 100 characters?
2. Another potential buffer overflow when you copy the answer back to the input string. Who says it will fit?
3. A potential buffer overflow each time you use scanf.
4. A memory leak each time you call malloc.
5. An inefficient implementation of strcpy just before return str;.
6. A crash (formally, undefined behaviour) when the input string does not contain the replacement string. strstr returns NULL when there is no match and you never check for it.
7. A potential issue with strncpy which leaves your string not NUL-terminated if there's not enough space for NUL.

Answer 4

Here is the immediate problem: when strstr returns NULL, your code does not pay attention. Add this line:

char *pt = strstr(str, find), *firstStr;
if (!pt) return str;

Another problem is that the call of strncpy is incorrect:

strncpy(firstStr, str, strlen(str) - strlen(pt));  

it will leave firstStr unterminated, because str is longer than the substring being copied. The subsequent call

strcat(firstStr, replace);

will operate on a string that is not null-terminated, causing undefined behavior.

"Shotgun" approach to fixing it would be to use calloc instead of malloc to put zeros into firstStr. A more precise approach would be placing '\0' at the end of the copied substring.

With these fixes in place, your code runs OK (demo). However, there are several issues that need to be addressed:

• You do not free any of the resources that you allocate dynamically - this results in memory leaks.
• You do not compute how much memory to allocate - If a 5-character string is replaced for a 100-character string in a 100-character string, you overrun the temporary buffer.
• You are using strncpy incorrectly - the function is intended for fixed-length strings. Use memcpy instead.
• You are using strcat instead of memcpy or strcpy - this is inefficient.