Donald Duck Donald Duck - 1 year ago 66
C Question

Problems with free() function in C

I have a

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

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

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
is NULL). After some tests, I found out that the line that was causing the problem is
. I tried to replace it with
if(temp != NULL){free(temp);}
but it didn't change anything. I tried to declare
char temp[1000]
instead of
and take away the
line but it still does the same thing. If I take away the
line and still use
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
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
sometimes work and sometimes not and how can I get it to always work?

Answer Source

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


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

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download