Find-replace string in C
There are many search / replace functions on the internet, but I cannot find why it doesn't work ... (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;
}
Compilation gives me a "segmentation fault" error, but I can't figure out what memmory it is trying to highlight and it cannot. Did I override the memory block or something else? Please, help:)
thank
source to share
Here's the closest problem: when it strstr
returns NULL
, your code is oblivious. Add this line:
char *pt = strstr(str, find), *firstStr;
if (!pt) return str;
Another problem is that the call is strncpy
wrong:
strncpy(firstStr, str, strlen(str) - strlen(pt));
it will leave firstStr
unterminated because it is str
longer than the substring being copied. Follow-up call
strcat(firstStr, replace);
will work on a string that is not null terminated, causing undefined behavior.
The "Shotgun" approach to commit would be to use calloc
instead malloc
to put zeros in firstStr
. A more precise approach would be to place it '\0'
at the end of the copied substring.
With these fixes, your code works fine ( demo ). However, there are several problems that need to be addressed:
- You are not freeing any of the resources that you dynamically allocate - it leaks memory.
- You are not calculating how much memory is allocated . If a 5-character string is replaced for a 100-character string in a 100-character string, you overflow a temporary buffer.
- You are using it
strncpy
wrong - the function is for fixed length strings. Use insteadmemcpy
. - You use
strcat
instead ofmemcpy
orstrcpy
- it is inefficient.
source to share
Did I override the memory block or something else?
You have:
- Potential buffer overflow during allocation
firstStr
. Who's to say that the result will be less than 100 characters? - Another potential buffer overflow while copying the response to the input string. Who said he would fit?
- Potential buffer overflow on every use
scanf
. - Memory leak on every call
malloc
. - Inefficient implementation
strcpy
just beforereturn str;
. - Crash (formally undefined behavior) when the input string does not contain the replacement string.
strstr
returnsNULL
when there is no match and you never check for it. - Potential issue with
strncpy
that leaves your line notNUL
-terminated if there isn't enough room forNUL
.
source to share
You haven't checked 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;
}
source to share