Find and Replace Text Function

i tried doing it diferently and playing with the code you guys gave me, but i still cant get it right myself.

any one see why this seg faults?

Code:

int replace(char *src, char search[], char replace[]) {
  int search_length = strlen(search);   /* String lengths to be used later for copying. */
  int replace_length = strlen(replace);

  int count = 0;  /* Number of times spent looking for search.  */
  int time = 0;   /* Number of replacements needed. */

  char *value = NULL;
    
  char *find;     /* String position info. */
  char *next_find;
  char *spot = src;

  while( (find = strstr(spot, search)) != NULL ) { /* Figure out how many replacements must be made. */
    ++count;

    spot += search_length;
  }

  spot -= (search_length * count);  /* Reset spot to orignal location. */

  if(count > 0) {
    while(time < count) {
      find = strstr(spot, search);                      /* Find where the search is. */
      next_find = strstr(spot + search_length, search); /* Find where to end the copy. */

      realloc(value, find - spot + 1);
      /* ***Problem start here*** */
      strncat(value, spot, find - spot);                /* Copy everything before search. */

      realloc(value, replace_length + 1);
      strcat(value, replace);                           /* Add the replacement text. */
                                           
      spot += (replace_length + 1);                     /* Update spot's location. */
      realloc(value, next_find - spot + 1);
      strncat(value, spot, next_find - spot);           /* Copy everything in between searches. */
    }
  }

  strcpy(src, value); /* The long awaited lines... */                      
  free(value);
  
  return 0;
}

Code:

while( (find = strstr(spot, search)) != NULL ) { /* Figure out how many replacements must be made. */
    ++count;

    spot += search_length;
}
spot -= (search_length * count);

This is wrong. Consider the following:

"the dog and the fish went to the store"

You're finding "the".
Your loop makes count equal three.
However, 'spot' points to the last 'the'.

spot = spot - ( 3 * strlen( "the" ) );

You only move back 9 characters. This is not the start of the string. This is not the start of anything. This is some arbitrary point in the middle of your string.

Simply call strstr for the first time again on the source string instead.

spot = strstr( src, search );

Now you correctly point to the first one.

[edit]
What you should be doing is this:
1) Count the number of items to replace.
2) Get the modified string length:
... a) s1 = count * strlen( search )
... b) r1 = count * strlen( replace )
... c) r1 < s1 ? no need to reallocate : reallocate space, adding r1-s1 more bytes.
3) Malloc the space you need.
4) Copy until the first replace spot.
5) Copy the first "replace" string in.
6) Continue copying from the original string past the first spot until the next.
7) Repeat 5 and 6 until you're done.
8) Add on any remainder string.

Now you could use realloc, but personally, I'd malloc a new string, then call realloc on the original once I was done and copy the new string in its place. Or, pass it a poitner to a pointer, and just update that, freeing the original.
[/edit]

Quzah.

im sure doing obviously wrong here but it still seg faults

Code:

int replace(char *src, char search[], char replace[]) {
  int search_length = strlen(search);   /* String lengths to be used later for copying. */
  int replace_length = strlen(replace);

  char *value = NULL;
    
  char *find;     /* String position info. */
  char *next_find = NULL;
  char *spot = src;

  spot = src;  /* Reset spot to orignal location. */

  while( (find = strstr(spot, search)) != NULL  && spot != '\0' ) {
    printf("\nTest 1\n");
    next_find = strstr(spot + search_length, search);

    printf("\nTest 2\n"); /*Crash starts here */
    realloc(value, find - spot + 1);
    strncat(value, spot, find - spot);

    printf("\nTest 3\n");
    realloc(value, replace_length + 1);
    strcat(value, replace);

    printf("\nTest 4\n");
    spot += replace_length;
    realloc(value, next_find - spot + 1);
    strncat(value, spot, find - spot);
  }

  strcpy(src, value);
  free(value);
  
  return 0;
}

Here's few problems to start with:

1. 'spot' should be 'find' in
next_find = strstr(spot + search_length, search);

2. realloc returns the new pointer so it should be:
value = (char*)realloc(value, find - spot + 1);
this is whats causing the crash, value = NULL.

3. when 2. is fixed initialize value as it will contain rubbish and
strncat will add to end of the rubbish. Init only once!

4. ?

I would suspect the call to realloc. To me it looks something like this.

Code:

int main(void)
{
    char *ptr;
    malloc(50);
    strcpy(ptr, "Was that return value important?");
    return 0;
}

Here is a recent example using realloc.

>> value = (char*)realloc(value, find - spot + 1);
>>this is whats causing the crash, value = NULL.
I'm not sure I get what you mean, but if you're saying that the first parameter to the realloc() function is NULL and will therefore cause a crash, you are incorrect. If the first parm is NULL, realloc() merely acts like malloc().

This issue is hugely overcomplicated. Do (again) what I already stated:

1) Count the needed replacements.

Code:

while( (find = strstr(spot, search)) != NULL ) {
    ++count;
    spot += search_length;
}

2) Do the math.

Code:

difference = count * search_length < replace_length ? replace_length :
    (search_length-replace_length);

3) Malloc a new block.

Code:

newptr = malloc( strlen( original ) + difference + 1 );

4) Repeat the following until you're at the end of the original string:
... a) copy from the original string to the new one, up to the "search" spot.
.... b) advance that pointer past the spot.
.... c) copy the "replace" text to the new string

5) Null terminate your new string.
6) Do one of the following:
... a) Make the original pointer point at the new string, free the old
... b) Realloc the original string to the length (+1) of the new string, then copy the new string over it and free the new one.

It's VERY simple. The answer has been provided multiple times. Why do I get the feeling I'm beating my head into a wall explaing this to you?

Quzah.

Why do I get the feeling I'm beating my head into a wall explaing this to you?

probally because im not going to copy and paste anything! i am trying to learn this. im writing what i learned, im not that worried with the code, yet. so instead of taking your answers im gonna screw up for awhile until i come up with my own. ill stop if i have a feeling im giving some one a concussion, thats about all ill stop at.

im not really seeing why i have to have a replacement count at all now. that seems like it would complicate it.

i have to look for the replacements and change all of them. so i need 2 points. i stop before the first, insert the replace and copy untill the 2nd point. then i have to start it over again. using realloc seems to be alittle easier right now, but that might change.

This thread and its companion have been quite animated; I've enjoyed playing along at home. Since the main players seem to be throwing in the towel, I thought I'd post my latest for comment.

Code:

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

/*
 * Description:
 *   Find and replace text within a string.
 *
 * Parameters:
 *   src  (in) - pointer to source string
 *   from (in) - pointer to search text
 *   to   (in) - pointer to replacement text
 *
 * Returns:
 *   Returns a pointer to dynamically-allocated memory containing string
 *   with occurences of the text pointed to by 'from' replaced by with the
 *   text pointed to by 'to'.
 */
char *replace(const char *src, const char *from, const char *to)
{
   /*
    * Find out the lengths of the source string, text to replace, and 
    * the replacement text.
    */
   size_t size    = strlen(src) + 1;
   size_t fromlen = strlen(from);
   size_t tolen   = strlen(to);
   /*
    * Allocate the first chunk with enough for the original string.
    */
   char *value = malloc(size);
   /*
    * We need to return 'value', so let's make a copy to mess around with.
    */
   char *dst = value;
   /*
    * Before we begin, let's see if malloc was successful.
    */
   if ( value != NULL )
   {
      /*
       * Loop until no matches are found.
       */
      for ( ;; )
      {
         /*
          * Try to find the search text.
          */
         const char *match = strstr(src, from);
         if ( match != NULL )
         {
            /*
             * Found search text at location 'match'. :)
             * Find out how many characters to copy up to the 'match'.
             */
            size_t count = match - src;
            /*
             * We are going to realloc, and for that we will need a 
             * temporary pointer for safe usage.
             */
            char *temp;
            /*
             * Calculate the total size the string will be after the
             * replacement is performed.
             */
            size += tolen - fromlen;
            /*
             * Attempt to realloc memory for the new size.
             */
            temp = realloc(value, size);
            if ( temp == NULL )
            {
               /*
                * Attempt to realloc failed. Free the previously malloc'd
                * memory and return with our tail between our legs. :(
                */
               free(value);
               return NULL;
            }
            /*
             * The call to realloc was successful. :) But we'll want to 
             * return 'value' eventually, so let's point it to the memory
             * that we are now working with.
             */
            value = temp;
            /*
             * Copy from the source to the point where we matched. Then
             * move the source pointer ahead by the amount we copied. And
             * move the destination pointer ahead by the same amount.
             */
            memmove(dst, src, count);
            src += count;
            dst += count;
            /*
             * Now copy in the replacement text 'to' at the position of 
             * the match. Adjust the source pointer by the text we replaced.
             * Adjust the destination pointer by the amount of replacement
             * text.
             */
            memmove(dst, to, tolen);
            src += fromlen;
            dst += tolen;
         }
         else /* No match found. */
         {
            /*
             * Copy any remaining part of the string. This includes the null
             * termination character.
             */
            strcpy(dst, src);
            break;
         }
      }
   }
   return value;
}

void test(const char *source, const char *search, const char *repl)
{
   char *after;
   after = replace(source, search, repl);
   printf("\nsearch = \"%s\", repl = \"%s\"\n", search, repl);
   if ( after != NULL )
   {
      printf("after  = \"%s\"\n", after);
      free(after);
   }
}

int main(void)
{
   const char before[] = "the rain in Spain falls mainly on the plain";
   printf("before = \"%s\"\n", before);
   test(before, "the", "THEE");
   test(before, "the", "A");
   test(before, "cat", "DOG");
   test(before, "plain", "PLANE");
   test(before, "ain", "AINLY");
   return 0;
}

/* my output
before = "the rain in Spain falls mainly on the plain"

search = "the", repl = "THEE"
after  = "THEE rain in Spain falls mainly on THEE plain"

search = "the", repl = "A"
after  = "A rain in Spain falls mainly on A plain"

search = "cat", repl = "DOG"
after  = "the rain in Spain falls mainly on the plain"

search = "plain", repl = "PLANE"
after  = "the rain in Spain falls mainly on the PLANE"

search = "ain", repl = "AINLY"
after  = "the rAINLY in SpAINLY falls mAINLYly on the plAINLY"
*/

>.I'm not sure I get what you mean, but if you're saying that the
> first parameter to the realloc() function is NULL and will therefore
> cause a crash, you are incorrect. If the first parm is NULL,
> realloc() merely acts like malloc().
Hammer,

The address of 'value' after the first realloc was still NULL, the
crash happens when 'value' is used in the following lines of code.
Realloc returns the new address, and the address of realloc can change.
Passing 'value' to realloc does not update its pointer.

realloc(value,,) should have been value=realoc(value,,)

>>realloc(value,,) should have been value=realoc(value,,)
Ideally it should have been
>>tmpvalue=realloc(value,,)
to be on the safe side. Remember that if value already points to malloc'd memory, and realloc fails, your version will loose the pointer value and create a leak. Using a different, temporary pointer to catch the return from realloc() enables you to test it for NULL first, and then assign the value to value.
>>value = tmpvalue;

a couple more questions here, why did you use size_t and memmove?

and i still dont get why some of my other versions dont work, like this (its only the millionth time i tried, ill guess righ eventually, righ?)

Code:

int replace(char *src, char search[], char replace[]) {
  int search_length = strlen(search);
  int replace_length = strlen(replace);

  char *value = NULL;
  char *spot = src;

  char *first = NULL;
  char *next = NULL;

  while(spot != '\0') {
       first = strstr(spot, search);
       next = strstr(spot + search_length, search);

       if(first != NULL) {
           if(next != NULL) {
             value = realloc(value, first - spot + 1);
             strncpy(value, spot, first - spot);

             value = realloc(value, replace_length);
             strcat(value, replace);

             spot += search_length + 1;
             value = realloc(value, next - spot + 1);
             strncat(value, spot, next - spot);
           } else {
             value = realloc(value, first - spot + 1);
             strncat(value, spot, first - spot);

             value = realloc(value, replace_length + 1);
             strcat(value, replace);

             spot += search_length + 1;
             value = realloc(value, strlen(spot) + 1);
             strcat(value, spot);
           }
       } else {
         value = realloc(value, strlen(spot) + 1);
         strcat(value, spot);
       }

  printf("\n\tvalue = %s\n", value);
  }

  strcpy(src, value);
  if(value != NULL)
    free(value);
    
  return 0;
}

>why did you use size_t and memmove?
I used size_t in part because that is the return value of strlen. I used memmove because an earlier version I was developing attempted to modify the string in place -- where overlap would occur.

Code:

value = realloc(value, length);

This is something to avoid, the point of my somewhat sarcastic response. If realloc fails, you cannot free the previously allocated memory (as has been pointed out already several times). This may not be the cause of your troubles, but it doesn't help to pick up bad habits as you go.

To me it seems likely that a problem would be asking realloc for the wrong amount of memory and/or writing where you don't have access. Where are you keeping track of the total string length? And it is a good idea to check whether a pointer is NULL before using it.

Another thing that has been bothering me in your posts is this.

Code:

int replace(char *src, char search[], char replace[])

It doesn't help to have ambiguous symbols.

Where are you keeping track of the total string length?

my while loop is supposed to stop everything when \0 is found
.

This is something to avoid, the point of my somewhat sarcastic response. If realloc fails, you cannot free the previously allocated memory (as has been pointed out already several times).

and yes, i remembered about realloc, im just more worried about getting it work once, ill try to make it work relibly next.

do you think me not using memmove is the problem? i thought all the cats remove the \0 and add the text and add another \0

>do you think me not using memmove is the problem?
No.

>my while loop is supposed to stop everything when \0 is found
Yes, but when you allocate memory, you need to ask for some amount that will contain the whole string.

Code:

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

int main(void)
{
   const char search[] = "this";
   const char replace[] = "these";
   const char src[] = "so this is the first occurrence and this is the second";
   size_t search_length = strlen(search);
   size_t replace_length = strlen(replace);
   char *first = strstr(src, search);
   char *spot  = strstr(first + search_length, search);
   size_t length = spot - first + 1;
   printf("src = \"%s\"\n", src);
   printf("search = \"%s\"\n", search);
   printf("first = \"%s\"\n", first);
   printf("spot = \"%s\"\n", spot);
   /*
   value = realloc(value, first - spot + 1);
   strncpy(value, spot, first - spot);
   
   value = realloc(value, replace_length);
   strcat(value, replace);
   */
   printf("first - spot + 1 = %lu\n", first - spot + 1);
   printf("length = %lu\n", length);
   printf("replace_length = %lu\n", replace_length);
   return 0;
}

/* my output
src = "so this is the first occurrence and this is the second"
search = "this"
first = "this is the first occurrence and this is the second"
spot = "this is the second"
first - spot + 1 = 4294967264
length = 34
replace_length = 5
*/

From your code in the comments, you first attempt to provide allocation for what would be a negative amount of memory. But since realloc takes an unsigned 2nd parameter, it converts it to some really big number. If this succeeds, you pad this string with more than enough null bytes, because again the negative number is taken by strncpy as a large positive value.

But then you realloc to only the length of the replace text, 5 bytes. And then attempt to concatenate (the original size, null-terminated 33 bytes, plus the new 5) into the 5 bytes you've allocated.

Or something like that. You need to keep a running total of the size of the whole string for memory allocation purposes...

>and yes, i remembered about realloc, im just more worried about getting it work once, ill try to make it work relibly next.

...which is why it can be better to put safety first.