CODEWITHSUNDEEP
cstring
random_programmer
random_programmer

Reputation: 21

Code does not appear to print concatenated strings correctly

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.

Upvotes: 1

Views: 78

Answers (1)

H.S.
H.S.

Reputation: 12679

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.

Upvotes: 1

Related Questions