wrong redimension of a string in c

Question

I am trying to make a function that receives a dynamic string and removes from it all occurrences of the character also passed as a parameter. The string should finally contain just enough space to contain characters not deleted

void delete(char *cad, char c){
    int i, cont = 0;
    char *aux = NULL;
        
    i = 0;
    while(cad[i] != '\0'){
        if(cad[i] != c){
            aux = (char*)realloc(aux, sizeof(char) * cont + 1);
            aux[cont] = cad[i];
            cont++;
        }
    i++;    
    }
    
    cad = (char*)realloc(cad, sizeof(char) * cont);
    i = 0;
    while(aux[i] != '\0'){
        cad[i] = aux[i];
        i++;
    }
    
}

Now I have a segmentation fault

Answer 1

1. You do not check the result of the realloc.
2. IMO it will be better to return the pointer to the new string instead of using double pointer. Double pointer may cause hard to track memory leaks, and function will not work with the const strings - for example string literals
3. You do not null character terminate the string.

In this example, I did not change your allocation algorithm but in real life more efficient will be first to count how much memory you need to allocate, allocate it and then process the string again:

char *delete(const char *cad, char c){
    size_t nchars = 0;
    char *aux = NULL;
    char *temp;
        
    while(*cad)
    {
        if(*cad != c)
        {
            temp = realloc(aux, sizeof(*temp) * nchars + 1);
            if(temp)
            {
                aux = temp;
                aux[nchars++] = *cad;
            }
            else
            {
                /* handle allocation error */
                free(aux);
                aux = NULL;
                break;
            }
        }
        cad++;
    }
    if(aux) aux[nchars] = 0;
    return aux;
}

Some minor changes: use objects instead of types in sizeof and do not cast result of malloc. You can also add NULL pointer parameter check.

Answer 2

Every time you are reallocing inside the while loop, you are essentially giving the variable aux a new address each time.

I advise you to not do that and allocate the memory you want to allocate at the start of the function.

You will need to calculate how much memory you would need before allocating the memory. That is, count how much element you would delete.

If you want me to further elucidate or add a code fragment, please feel free to ask it in the comments.

Answer 3

Instead of many calls to realloc() I would just perform an in-place substitution of the characters; this substitution leaves unused allocated characters at the end of the string and is illustrated by the delete_no_realloc() function below.

If you want to get rid of these unused ending characters in the allocated string, then only one call to realloc() is needed as illustrated by the delete() function below.

Note that when a function uses realloc() on a parameter which is a pointer, it must obtain the address of this pointer to adjust it with the result of realloc().

/**
  gcc -std=c99 -o prog_c prog_c.c \
      -pedantic -Wall -Wextra -Wconversion \
      -Wwrite-strings -Wold-style-definition -Wvla \
      -g -O0 -UNDEBUG -fsanitize=address,undefined
**/

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

size_t // new length
delete_no_realloc(char *cad,
                  char c)
{
  size_t w=0;
  for(size_t r=0; cad[r]; ++r)
  {
    char ch=cad[r];
    if(ch!=c)
    {
      cad[w++]=ch; // store and advance write index
    }
  }
  cad[w]='\0'; // ensure string termination
  return w;
}

void
delete(char **cad_ptr,
       char c)
{
  char *cad=*cad_ptr; // forget this embarrassing indirection
  size_t new_length=delete_no_realloc(cad, c);
  cad=realloc(cad, new_length+1);
  if(cad==NULL)
  {
    abort();
  }
  *cad_ptr=cad; // don't forget to adjust the string
}

int
main(void)
{
  const char *msg="this is a message";
  char *cad=malloc(strlen(msg)+1);
  if(cad==NULL)
  {
    abort();
  }
  strcpy(cad, msg);
  printf("before: <%s>\n", cad);
  delete(&cad, 's'); // pass the address of the string
  printf("after: <%s>\n", cad);
  free(cad);
  return 0;
}

Answer 4

You can simplify your delete() function by simply using a read and write index within the original string, removing all c characters found, and then make a single call to realloc() to reallocate storage to exactly fit the remaining characters.

You can do something like:

void delete (char **cad, char c)
{
    if (!*cad || !**cad)            /* check if cad is NULL or empty-string */
        return;
    
    size_t write = 0;               /* write index */
    
    for (size_t read = 0; (*cad)[read]; read++) {   /* loop over each char in cad */
        if ((*cad)[read] != c)                      /* if char not c */
            (*cad)[write++] = (*cad)[read];         /* copy incrementing write */
    }
    (*cad)[write] = 0;                              /* nul-terminate */
    
    void *tmp = realloc (*cad, write + 1);          /* realloc to exact size */
    if (!tmp) {                                     /* validate realloc */
        perror ("realloc-cad");
        return;
    }
    
    *cad = tmp;         /* assign reallocated block to *cad */
}

A full example would be:

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

void delete (char **cad, char c)
{
    if (!*cad || !**cad)            /* check if cad is NULL or empty-string */
        return;
    
    size_t write = 0;               /* write index */
    
    for (size_t read = 0; (*cad)[read]; read++) {   /* loop over each char in cad */
        if ((*cad)[read] != c)                      /* if char not c */
            (*cad)[write++] = (*cad)[read];         /* copy incrementing write */
    }
    (*cad)[write] = 0;                              /* nul-terminate */
    
    void *tmp = realloc (*cad, write + 1);          /* realloc to exact size */
    if (!tmp) {                                     /* validate realloc */
        perror ("realloc-cad");
        return;
    }
    
    *cad = tmp;         /* assign reallocated block to *cad */
}


int main (int argc, char **argv) {
    
    if (argc < 3) {
        fputs ("usage: ./prog \"string with c\" c\n", stderr);
        return 1;
    }
    
    size_t len = strlen (argv[1]);
    char *s = malloc (len + 1);
    
    if (!s) {
        perror ("malloc-s");
        return 1;
    }
    memcpy (s, argv[1], len + 1);
    printf ("%s (%zu chars)\n", s, len);
    
    delete (&s, *argv[2]);
    printf ("%s (%zu chars)\n", s, strlen(s));
    
    free (s);
}

Example Use/Output

$ ./bin/delete_c_realloc "nmyn ndogn nhasnn nnfleasnnn" n
nmyn ndogn nhasnn nnfleasnnn (28 chars)
my dog has fleas (16 chars)

Look things over and let me know if you have questions.

Answer 5

There are four main problems with your function implementation.

The first one is that the function accepts the pointer to the source string by value. That is the parameter cad is initialized by the value of the pointer used as an argument. As a result changing the variable cad does not influence on the original pointer.

The second one is that you are not checking whether a call of realloc was successful. As a result the function can invoke undefined behavior.

The third one is that it is inefficient to reallocate the string each time when a new character is appended.

And at last the fourth one is that the result dynamically allocated array does not contain a string because you forgot to append the terminating zero character '\0'.

If you want to change within the function a value of the original pointer you should either to return from the function the result pointer obtained in the function and assign it to the original pointer in the caller. Or you should pass the original pointer to the function by reference. In C passing by reference means passing an object (that can be a pointer) indirectly through a pointer to it.

Here is a demonstrative program that shows the function implementation when the original pointer is accepted by the function by reference.

The function also returns a pointer to the result string that can be checked in the caller whether the reallocation of dynamic memory within the function was successful.

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

char * remove_char( char **s, char c )
{
    char * result = *s;
    
    if ( c != '\0' )
    {
        char *dsn = *s;
        const char *src = *s;
        
        do
        {
            if ( *src != c )
            {
                if ( dsn != src )
                {
                    *dsn = *src;
                }
                ++dsn;
            }
        } while ( *src++ );
        
        char *tmp = realloc( *s, ( dsn - *s ) * sizeof( char ) );
        
        if( tmp != NULL ) *s = tmp;
        
        result = tmp;
    }
    
    return result;
}

int main(void) 
{
    char *s = malloc( 12 );
    
    strcpy( s, "H#e#l#l#o!" );
    
    puts( s );
    
    if ( remove_char( &s, '#' ) ) puts( s );
    
    free( s );
    
    return 0;
}

The program output is

H#e#l#l#o!
Hello!

Another approach is to write a function that does not change the source string but creates dynamically a new string that contains the source string excluding the specified character. Such a function is more flexible because you can call it with string literals. If the source string also was dynamically allocated then the caller of the function after a successful call it can just free the source string.

Here is a demonstrative program.

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

char * remove_copy( const char *s, char c )
{
    size_t src_len = strlen( s );
    size_t dsn_len = src_len;
    
    if ( c != '\0' )
    {
        for ( const char *p = s; ( p = strchr( p, c ) ) != NULL; ++p )
        {
            --dsn_len;
        }
    }
    
    char *result = malloc( ( dsn_len + 1 ) * sizeof( char ) );
    
    if ( result != NULL )
    {
            const char *src_s = s;
            char *dsn_s = result;
            
            if ( dsn_len != src_len )
            {
                for ( const char *p = src_s; 
                      ( p = strchr( src_s, c ) ) != NULL; 
                      src_s = p + 1 )
                {
                    if ( p - src_s != 0 )
                    {
                        memcpy( dsn_s, src_s, p - src_s );
                        dsn_s += p - src_s;
                    }
                }
            }
            
            strcpy( dsn_s, src_s );
    }

    return result;
}

int main(void) 
{
    char s[] = "H#e#l#l#o!";
    
    puts( s );
    
    char *p = remove_copy( s, '#' );
    if ( p != NULL ) puts( p );
    
    free( p );
    
    return 0;
}

The program output is the same as shown for the preceding demonstrative program that is

H#e#l#l#o!
Hello!