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

Segmentation fault doesn't mean it failed to allocate memory. That is reported by malloc() returning NULL (which you don't check for!). The fault means you overwrote valid memory, i.e. you have a bug. Run the program in a debugger, and/or Valgrind if available. — unwind
– unwind, Commented Nov 28, 2014 at 16:26
that is not a compilation error, if it is your compiler is having the problem, not your program. Another thing you should not call strlen in a for loop like that, it is expensive. You should instead store the value in a variable and use that. — Iharob Al Asimi
– Iharob Al Asimi, Commented Nov 28, 2014 at 16:28
You shouldn't use a for-loop at all for such a simple task, memcpy is there for you. And this is also where you have at least one bug, you don't terminate str with a 0 byte. — Jens Gustedt
– Jens Gustedt, Commented Nov 28, 2014 at 16:37
Memory allocated with malloc is never freed, so you got a memory leak with each strrpl function call. — Ari0nhh
– Ari0nhh, Commented Nov 28, 2014 at 16:38
thank you very much to all, i rechecked my code, and with scanf it only reads until ' '(space character), and if the find string is in the middle, it will always return NULL, so i used gets() to read my strings — cristi.gherghina
– cristi.gherghina, Commented Nov 28, 2014 at 23:38

n. m. could be an AI · Accepted Answer · 2014-11-28 16:38:43Z

4

I overrided a memory block or something?

You have:

A potential buffer overflow when you allocate firstStr. Who says the result will be less than 100 characters?
Another potential buffer overflow when you copy the answer back to the input string. Who says it will fit?
A potential buffer overflow each time you use scanf.
A memory leak each time you call malloc.
An inefficient implementation of strcpy just before return str;.
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.
A potential issue with strncpy which leaves your string not NUL-terminated if there's not enough space for NUL.

answered Nov 28, 2014 at 16:38

community wiki

n. m. could be an AI

Sign up to request clarification or add additional context in comments.

Comments

Sergey Kalinichenko · Accepted Answer · 2014-11-28 16:36:14Z

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.

user-ag · Accepted Answer · 2014-11-28 17:12:28Z

0

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

answered Nov 28, 2014 at 17:12

user-ag

2244 silver badges21 bronze badges

2 Comments

user-ag Over a year ago

All the allocated memory should be freed after the usage.

cristi.gherghina Over a year ago

thank you very much, i rechecked my code, and with scanf it only reads until ' '(space character), and if the 'find' string is in the middle, it will always return 'NULL'

Furinkazan · Accepted Answer · 2023-06-22 12:51:35Z

0

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

edited Jun 22, 2023 at 12:51

answered Jun 22, 2023 at 12:40

Furinkazan

11 bronze badge

Collectives™ on Stack Overflow

find- replace string function in C

4 Answers 4

Comments

Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related