using realloc to write on a new string?-CodePudding

remove_multiple takes one parameter (a C string), and remove duplicates. it has to return the dynamically allocated string in the heap.

I've tried to:

create a new dynamically allocated pointer (s);
if the current character (of str) is not equal to the next character, just copy that in s, after having reallocated (s).

the problem is that I still have this warning on the realloc: "C6308 'realloc' might return null pointer: assigning null pointer to 's', which is passed as an argument to 'realloc', will cause the original memory block to be leaked", and when I try to debug the program I got a message, it says that I was trying to write outside the allocated memory.

here's my code:

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

char* remove_duplicates(const char* str) {
    char* s = malloc(sizeof(char)); 
    if (!s) {
        return NULL; 
    }
    for (size_t i = 0; str[i] != 0; i  ) {
        
        
        if (str[i] != str[i   1]) {
            s = realloc(s, 2 * sizeof(char));
            if (!s) {
                return NULL;
            }
            s[i] = str[i];
        }
    }

    return s; 
}



int main(void) {
    char str[] = "heyyyy"; 
    char* s = remove_duplicates(str); 
    printf("%s", s); 

    free(s); 
    return 0; 
}

error list:

Warning C6308 'realloc' might return null pointer: assigning null pointer to 's', which is passed as an argument to 'realloc', will cause the original memory block to be leaked.

CodePudding user response：

For starters your approach is not good.

The function is inefficient because within it there are too many memory reallocations.

As for the messages then this

s = realloc(s, 2 * sizeof(char));

statement is unsafe because the function realloc can return a null pointer. In this case the address of the previously allocated memory will be lost that results in a memory leak.

You need to use an intermediate pointer that will be assigned with the value returned by realloc.

In this statement

s = realloc(s, 2 * sizeof(char));

there are always allocated 2 bytes of memory.

Also there is used an invalid index in the left hand side expression

s[i] = str[i];
^^^^

For the pointer s you need to support its one index not i.

What you need is at first to determine how many there are non-duplicated adjacent characters and then only one time allocate an array of the required size.

Here is a demonstration program.

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

char * remove_duplicates( const char *s ) 
{
    size_t n = 0;
    
    for ( const char *p = s; *p; )
    {
          n;
        while ( *  p && p[0] == p[-1] );
    }
    
    char *result = malloc( n   1 );
    
    if ( result != NULL )
    {
        char *p = result;
        do
        {
            *p = *s;
            while ( *s   && s[0] == s[-1] );            
        } while ( *p   );
    }
    
    return result;
}

int main( void )
{
    const char *s = "1223334444";
    
    printf( "\"%s\"\n", s );
    
    char *p = remove_duplicates( s );
    
    if ( p )    printf( "\"%s\"\n", p );

    free( p );
}

The program output is

"1223334444"
"1234"

CodePudding user response：

You are normally not expected to realloc for each character. So the lazy way would be to initially allocate an array of the same size of the original array, and optionally try to shrink it in the end:

char* remove_duplicates(const char* str) {
    char* s = malloc(strlen(str)); // only allocate once with original size
    if (!s) {
        return NULL;
    }
    // keep track or current position, size and previous character
    char c, old = 0, *curr = s;
    size_t n = 0;
    while ((c = *str  ) != '\0') {
        // ignore duplicates
        if (c != old) {
            *curr   = c;
            old = c;
              n;
        }
    }
    // add the terminating NULL
    *curr = '\0';
    // optional shrink attempt
    s = realloc(s, n    1);
    return s;
}