Strdup () memory leak even after freeing ()

Question

I never had to use strdup(stringp) with strsep(&stringp_copy, token) together until recently, and I think this was causing a memory leak.

( strdup() always strdup() just fine before.)

I fixed the leak and I think I understand how, but I just can’t understand why I need it.

Source code (debriefing):

 const char *message = "From: username\nMessage: basic message\n"; char *message_copy, *line, *field_name; int colon_position; message_copy = strdup(message); while(line = strsep(&message_copy, "\n")) { printf(line); char *colon = strchr(line, ':'); if (colon != NULL) { colon_position = colon - line; strncpy(field_name, line, colon_position); printf("%s\n", field_name); } } free(message_copy); 

New code that does not leak:

 const char *message = "From: username\nMessage: basic message\n"; char *message_copy, *freeable_message_copy, *line, *field_name; int colon_position; freeable_message_copy = message_copy = strdup(message); while(line = strsep(&message_copy, "\n")) { printf(line); char *colon = strchr(line, ':'); if (colon != NULL) { colon_position = colon - line; strncpy(field_name, line, colon_position); printf("%s\n", field_name); } } free(freeable_message_copy); 

How is the message_copy pointer rewritten in the first code? or that?

Answer 1

The strsep () function takes a pointer to the source string (message_copy) and modifies it to return a new pointer to the "next" token

 const char *message = "From: username\nMessage: basic message\n"; char *message_copy, *original_copy; //here you have allocated new memory, a duplicate of message message_copy = original_copy = strdup(message); 

Print a pointer here

 printf("copy %p, original %p\n", message_copy, original_copy); 

Note that when using strsep () you change message_copy,

 char* token; //here you modify message_copy while(token = strsep(&message_copy, "\n")) { printf("%s\n", token); } 

This illustrates the modified message_copy command, but original_copy does not change.

 printf("copy %p, original %p\n", message_copy, original_copy); 

Since message_copy does not indicate the original result of strdup (), this would be wrong,

 free(message_copy); 

But keep the original strdup () result, and this freeware works

 //original_copy points to the results of strdup free(original_copy); 

Answer 2

Since strsep() modifies the message_copy argument, you tried to free a pointer that was not returned by malloc() et al. This would cause complaints from some malloc() libraries and from valgrind . This is also undefined behavior, usually leading to crashes in a short order (but crashes in the code in an inconvenient place unrelated to the code that caused the damage).

In fact, your loop repeats until message_copy is set to NULL, so you release NULL, which is defined and safe, but it also does not work. It did not release the pointer allocated through strdup() .

Summary:

• Only free pointers returned by memory allocators.
• Do not point to the middle or end of the block returned by memory allocators.

Answer 3

Read the strsep man page here .

In short, the strsep function will change the source pointer to the character that is passed to the function, overwriting each separator event with \0 , and then, after the source character pointer has been updated to point to \0 .

The second version does not leak, since you created a temporary pointer pointing to the beginning of the original char pointer returned from strdup() , so the memory was correctly freed, since you called free() using the original char pointer instead of the updated, modified strsep() .

Answer 4

On the manual page

... This token ends by overwriting the delimiter with a zero byte ('\ 0') and *stringp is updated to point past the token ....