Donald Duck Donald Duck - 3 months ago 12
C Question

Problems with free() function in C

I have a

while
loop inside a
for
loop to handle strings. Here is basically the structure of my code:

char myString[1000];
//Initialize and maybe change myString
for(/*conditions*/){
while(/*conditions*/){
if(strchr(myString,' ') == NULL){
break;
}
char *temp = malloc(sizeof(char) * strlen(myString));
strcpy(temp,myString);
*strchr(temp,' ') = '\0';
strcat(myString," ");
strcat(myString,temp);
free(temp);
}
}


Sometimes, this code works just fine, but sometimes the process ends and returns 3 which means that there is an error (3 is the return value that I usually get when I try to use NULL where I shouldn't like for example
myPointer->example
where
myPointer
is NULL). After some tests, I found out that the line that was causing the problem is
free(temp);
. I tried to replace it with
if(temp != NULL){free(temp);}
but it didn't change anything. I tried to declare
temp
with
char temp[1000]
instead of
malloc
and take away the
free(temp);
line but it still does the same thing. If I take away the
free(temp);
line and still use
malloc
the problem is solved but instead there is a huge memory leak, so I can't do that. If there is an error or not depends on what is in the
myString
string, which means that if there is a certain value in there, there always is an error, and if there is another certain value, there never is an error, but I can't manage to find out what type of values work and which ones don't, it seems to be random.

Why does
free(temp);
sometimes work and sometimes not and how can I get it to always work?

Answer

The major problem is, you're allocating one element less than the required memory.

strlen() does not account for the terminating null, so you're one short of the required memory. Later, doing

    strcpy(temp,myString);

is actually out of bound access (to store the terminating null) which invokes undefined behavior. As a result, you get to see

Sometimes, this code works just fine, but sometimes the process ends and returns 3 which means that there is an error[....]

To resolve, you should modify the allocation statement like

 char *temp = malloc(strlen(myString) + 1); // +1 for terminating null,
                                            // sizeof(char) == 1, guaranteed by C standard.

That said, from the man page

The strchr() and strrchr() functions return a pointer to the matched character or NULL if the character is not found. [...]

and, for the highlighted scenario,

*strchr(temp,' ') = '\0';

attempts to dereference a null-pointer constant (NULL) which is invalid and again, invokes UB. Check for a valid return value before dereferecing the returned pointer

Comments