Locating and replacing a substring (C language)

Question

I wrote the following code with the intention of using pointer arithmetic on strings to find, and replace, a targeted substring. Obviously, it's not elegant, but unfortunately it's also incorrect - it adds extraneous characters to the string.

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

int main() {
    char string[] = "The quick brown fox jumped over the lazy dog.";
    char target[] = "brown"
    char replacement[] = "ochre";
    char segment[80+1];
    char pre_segment[80+1];
    char post_segment[80+1];
    int S = strlen(string), T = strlen(target);
    for (int i = 0; i < S; i++) {
        strncpy(segment, string + i, T);
        if (strcmp(segment, target) == 0) {
        >>> strncpy(pre_segment, string, i); <<<
            strncpy(post_segment, string + i + T,
                S - (i + T));
            strcat(pre_segment, replacement);
            strcat(pre_segment, post_segment);
            printf("%s\n", pre_segment);
        }
    }
    return 0; 
}

After the line marked like >>>this<<<, extraneous characters have been prepended to replacement before replacement is concatenated with pre_segment.

Can someone give me suggestions on how to debug this? (Suggestions for a nicer solution are also welcome, but please try to be explicit. Also, I'm not supposed to be using dynamic memory allocation for this.)

Answer 1

Don't use strncpy. It almost certainly doesn't do what you think it does. In particular, it doesn't guarantee NUL-termination, while fooling people into thinking that it does. If you want to copy exactly n characters, use memcpy(dest, src, n); and then explicitly NUL-terminate with dest[n] = '\0';. The lack of NUL termination is likely to be causing your problems. (Check that in your debugger!)

However, there is no need to do the strncpy at all. Use strncmp or memcmp. (Only use memcmp if you know that there are at least strlen(target) bytes left in the string.) If strlen(target) bytes starting at some point in string match target, then you've found a match.

Even better would be to use strstr to find the next occurrence of the string.

Answer 2

You should always try to split your code into smaller parts(functions), we can identify two key steps in the process of replacing a substring, finding the substring and then replacing it. This is a solution i am proposing for you, it's been three years since i wrote my last c line of code so this is not perfect but it does the job:

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


int find_occurence(char* original, char *target){
  size_t counter = 0;
  char *index = original;
  do{
    while(target[counter] == index[counter])
      counter++;
    if (counter >= strlen(target))
      return (int)(index-original);
    else
      counter = 0;
  }
  while(*(index++));
  return -1;
}

void replace(char *original, char *target, char *replacement,char *destination){
  int index = find_occurence(original, target);
  if (index == -1)
  {
    strncpy (destination, original, strlen(original)+1 );
    return;
  }

  char *last_part;

  //Copy the string before target
  strncpy (destination, original, index );

  //Copy the replacement
  strncpy (&destination[index], replacement, strlen(replacement) );

  //Extract the part after the target
  last_part = &original[index+strlen(target)];

  //Copy the part after the target plus the \0 character
  strncpy (&destination[index+strlen(replacement)],last_part,strlen(last_part)+1);
}


int main() {
  char *original = "I want to replace literally this by that";
  char *target = "this";
  char *replacement = "that";
  char destination[100];

  replace(original,target,replacement, destination);

  printf("%s\n",destination);


}

Answer 3

I can not agree with @rici that exclaims that the function strncpy should not be used. Any function can be used incorrectly. And strncpy is not an exception. You should keep in mind that it is not necessary that the function will copy a string. So you have yourself to append explicitly a zero-character to the copied sequence of characters.

And you forgot to do this.

Though your implementation is too complicated and confusing nevertheless in any case it should be written carefully.

Here is an updated version of your program. Pay attention to these statements

segment[T] = '\0';
pre_segment[i] = '\0';
post_segment[S - ( i + T )] = '\0';

Or if you prefer tp use pointers then you could write

*( segment + T ) = '\0';
*( pre_segment + I ) = '\0';
*( post_segment + S - ( i + T )) = '\0';

Here you are.

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

int main(void) 
{
    char string[] = "The quick brown fox jumped over the lazy dog.";
    char target[] = "brown";
    char replacement[] = "ochre";
    char segment[80+1];
    char pre_segment[80+1];
    char post_segment[80+1];

    size_t S = strlen(string), T = strlen(target);

    for ( size_t i = 0; i < S; i++) 
    {
        strncpy(segment, string + i, T);
        segment[T] = '\0';

        if ( strcmp( segment, target ) == 0 ) 
        {
            strncpy(pre_segment, string, i);
            pre_segment[i] = '\0';

            strncpy( post_segment, string + i + T, S - (i + T));
            post_segment[S - ( i + T )] = '\0';

            strcat(pre_segment, replacement);
            strcat(pre_segment, post_segment);
            printf("%s\n", pre_segment);
        }
    }

    return 0;
}

The program output is

The quick ochre fox jumped over the lazy dog.