Code does not appear to print concatenated strings correctly

Question

I have some code here where, given a .txt file whose contents is

find replace pre pre cpre

,I want to find every instance of "pre", and append "k" to it. ie the file should become "find replace kpre".

So I first set out to create a string that is the concatenation of k and pre (assume k and pre are argv[1] and argv[3], respectively)

char appended[1024]; strcpy(appended, argv[1]); strcat(appended, argv[3]); printf("appended string is %s", appended); //prints kpre, which is good char *replaced = replace(buf, argv[3], appended); //*string is a line in the file char* replace(char *string, char *find, char *replace) { char *position; char temp[1024]; int find_length = strlen(find); int index = 0; while ((position = strstr(string, find)) != NULL) { strcpy(temp, string); index = position - string; string[index] = '\0'; strcat(string, replace); //add new word to the string strcat(string, temp + index + find_length); //add the unsearched //remainder of the string } return string; } ................. fputs(replaced, temp);

Checking on the console, appended = "kpre", which is correct, but when the code is run the file looks like

find replace kkkkkkkkkkkkkkkk.....kkkkkkk kkkkkkkkk......kkkkk ckkkkk....kkkkk

the k's go on for a while, I cannot see pre when scrolling all the way to the right. I'm having difficulty figuring out why the code doesn't replace the instance of 'pre' with 'kpre', even when the appended variable appears to be correct. I have a feeling it has to do with the fact that I set a 1024 character for temp, but even then I'm not sure why k was copied so many times.

Here's a string "pre", replace all instances of "pre" with "kpre". Ok. That give me "kpre". Now, starting from the beginning of the string, repeat that. Um... if you start searching for "pre" from the beginning of the string you're going to find one for sure, because you just wrote one. So, the next result would be "kkpre", then "kkkpre", then "kkkkpre" etc. . See the problem? You have to move where you search from to not include the text you just replaced. A pointer would really be ideal for that, btw. — WhozCraig
– WhozCraig, Commented Feb 3, 2019 at 2:38
I thought the line index = position - string would update the index to the point where it is already past the portion of the string that has already been searched? — random_programmer
– random_programmer, Commented Feb 3, 2019 at 3:10
How would your search function (strstr) know that this "index" variable exists, much less what its value is? — JaMiT
– JaMiT, Commented Feb 3, 2019 at 4:10
What happens if replace is longer than find and find isn't the last word in string? — David C. Rankin
– David C. Rankin, Commented Feb 3, 2019 at 5:23

H.S. · Accepted Answer · 2019-02-03 05:31:29Z

Here

 while ((position = strstr(string, find)) != NULL) {

you are passing string to strstr() function. The strstr() will return the pointer to the first occurrence of find in string. When you replace pre with kpre and calling again strstr(), it is retuning the pointer to the first occurrence of pre in string which is a sub string of replace string. After some iterations of while loop, it will start accessing the string beyond its size which will lead to undefined behavior.

Instead of passing string to strstr(), you should pass pointer to string and after every replace operation, the make the pointer point to after the replaced part of string. Other way is you can traverse the string character by character using pointer instead of using strstr(), like this:

#define BUFSZ 1024 char* replace(char *string, const char *find, const char *replace) { if ((string == NULL) || (find == NULL) || (replace == NULL)) { printf ("Invalid argument..\n"); return NULL; } char temp[BUFSZ]; char *ptr = string; size_t find_len = strlen(find); size_t repl_len = strlen(replace); while (ptr[0]) { if (strncmp (ptr, find, find_len)) { ptr++; continue; } strcpy (temp, ptr + find_len); // No need to copy whole string to temp snprintf (ptr, BUFSZ - (ptr - string), "%s%s", replace, temp); ptr = ptr + repl_len; } return string; }

Note that above code is based on the example you have posted in your question and just to give you an idea about how you can achieve your goal without using strstr(). When writing code, take care of the other possibilities as well like, replace is a huge string.

Collectives™ on Stack Overflow

Code does not appear to print concatenated strings correctly

1 Answer 1

Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Related