what does it mean to have these kind of errors in C?

Question

this is my solution

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

char* deletemultiple(const char* str) {
    if (str == NULL) {
        return NULL; 
    }
    size_t length = strlen(str); 
    if (length == 0) {
        return str; 
    }
    length = length + 1u; 
    char* s = malloc(length); 
    if (s == NULL) {
        return NULL; 
    }  
    size_t j = 0; 
    for (size_t i = 0; s[i] != 0; i++) {
        if (str[i] != str[i+1]) {
            s[j++] = str[i]; 
        }
    }
    s = realloc(s, j+1); 
    if (s == NULL) {
        return NULL; 
    }

    return s; 
}



 int main(void) {
    char str[] = ""; 
    char* s = deletemultiple(str); 

    free(s); 
    return 0; 
}

it's a program that delete multiple characters (i.e adjacent characters) and return a new allocated string without multiple adjacent characters. This solution works only for strings with length != 0; but if string is "" (i.e an empty string), when I free the memory, I have an error that blocks the program. (i.e A breakpoint instruction (__debugbreak() statement or a similar call) was executed ).

Moreover, I have 2 warnings: one warning reminds me that "realloc might return a null pointer", but I already know that and in fact I've used an if-block to check if it's either null or not.

and the other warning is about "reading invalid data from s, and it's related to this for-loop block:

 for (size_t i = 0; s[i] != 0; i++) {
        if (str[i] != str[i+1]) {
            s[j++] = str[i]; 
        }
    }

can somebody explains what does these errors/warnings mean, and how to solve them? in similar exercises, if I'll have these errors again, what should I do?

You don't null terminate the copied string in the `deletemultiple()` function. You need `s[j] = '\0';` after the loop. This is likely what the compiler is warning you about. — Jonathan Leffler, Jul 20 '22 at 13:25
If length is zero, the function returns the string. So you are trying to free a value that was not allocated with `malloc`. — William Pursell, Jul 20 '22 at 13:26
You say "if string is `""` (i.e void)" — that is not `void` in C terminology. It is an empty string, but that's different from `void` which is an absence of any value. — Jonathan Leffler, Jul 20 '22 at 13:28
The realloc is pointless. If you want to allocate the minimal length needed, you should walk the array first and count. Note that `malloc(length)` is too short. You need `malloc(length + 1)` — William Pursell, Jul 20 '22 at 13:35
Re "*I already know that and in fact I've used an if-block to check if it's either null or not.*", Yeah, but you have a memory leak when it returns NULL because you did `s = realloc(s, ...)` instead of `tmp = realloc(s, ...)`. That's what the warning is about. — ikegami, Jul 20 '22 at 13:46

William Pursell · Answer 1 · 2022-07-20T13:34:17.407

If the length is zero, your function is returning its argument, so your code is the same as:

int main(void) {
    char str[] = ""; 
    char* s = str; 
    free(s);  /* ERROR: This is wrong!! */
    return 0; 
}

but you cannot free(str), because str was not allocated with malloc.

Just remove the special case check against length zero, and fix your bug so that you allocate length + 1 to leave space for the null terminatorr.

Vlad from Moscow · Accepted Answer · 2022-07-20T14:14:15.203

If you passed an empty string then this empty string is returned

char* deletemultiple(const char* str) {
    if (str == NULL) {
        return NULL; 
    }
    size_t length = strlen(str); 
    if (length == 0) {
        return str; 
    }
    //...

So you may not call the function free for such a pointer as you are doing

char* s = deletemultiple(str); 

free(s);

It means that the function interface is broken. You need to return a dynamically allocated empty string.

The dynamically allocated array s does not contain a string because you forgot to append it with the zero terminating character '\0'

If the memory reallocation was not successful

s = realloc(s, j+1); 
if (s == NULL) {
    return NULL; 
}

then the function produces a memory leak because the address of the previously allocated memory that will not be freed in this case will be lost due to reassigning the pointer s. You need to use an intermediate pointer as for example

char *tmp = realloc(s, j+1); 
if (tmp == NULL) {
    free( s );
    return NULL; 
}

The approach when the memory is allocated twice is unsafe. Also it is not required to check whether str is equal to NULL.

if (str == NULL) {
    return NULL; 
}

String functions support the convention according to which if the user passes a null pointer then functions have undefined behavior.

The function can look for example the following way as shown in the demonstration program below.

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

char * delete_multiple( const char *s ) 
{
    size_t n = strlen( s );

    for ( const char *p = s; *p++; )
    {
        if ( *p == *( p - 1 ) ) --n;
    }

    char *result = malloc( n + 1 );

    if ( result != NULL )
    {
        char *p = result;
        for ( *p = *s; *s++; )
        {
            if ( *s != *p ) *++p = *s;
        }
    }

    return result;
}

int main( void )
{
    char *p = delete_multiple( "" );

    printf( "\"%s\"\n", p );
    free( p );

    p = delete_multiple( "a" );

    printf( "\"%s\"\n", p );
    free( p );
    

    p = delete_multiple( "aa" );

    printf( "\"%s\"\n", p );
    free( p );
    
    p = delete_multiple( "aaa" );

    printf( "\"%s\"\n", p );
    free( p );

    p = delete_multiple( "abaca" );

    printf( "\"%s\"\n", p );
    free( p );
}

The program output is

""
"a"
"a"
"a"
"abaca"

what does it mean to have these kind of errors in C?

2 Answers2