CODEWITHSUNDEEP
csegmentation-faultmallocfree
VMills
VMills

Reputation: 183

malloc/free, appear to be getting multiple frees

I've written a function to test if a given path is a valid Maildir directory (standard Maildir has the three subfolders "cur" "new" and "tmp" ). Function takes in the supposed directory, checks for those subfolders, and returns appropriately.

I'm getting a segfault at the second free statement with the current code, and I similarly got an "invalid next size" error with code of slightly different organization. Even more confusing, it only segfaults on some directories, while successfully completing on others, with no discernible reason (though it is consistent on which ones it will segfault on). With the second free() commented out, all accurately-formatted directories complete successfully.

Obviously I'm double-freeing. My question is, why and how? If the first free is inside the conditional statement and we return immediately after freeing, we never get to the second free. If we get to the second free, that means we skipped the first one... right?

I realize in this context it's perfectly fine because the system will reclaim the memory at the end of the program, but I'm more interested in the reason this is happening than in just making the code work. What if I were looking at a different situation, functions called by functions called by functions etc. and memory could possibly be a concern? Don't I need that 2nd free to reclaim memory?

int is_valid_folder(char* maildir)
{
 struct stat *buf;
 buf = (struct stat *) malloc(sizeof(struct stat));

char* new = strdup(maildir);
char* cur = strdup(maildir);
char* tmp = strdup(maildir);
strcat (cur, "/cur"); strcat (new, "/new"); strcat (tmp, "/tmp");

if(stat(cur, buf) || stat(tmp, buf) || stat(new, buf))
{
printf("Problem stat-ing one of the cur/new/tmp folders\n");
printf("Error number %d\n", errno);
free(buf);
return 1;
}

free(buf);
return 0; //a valid folder path for this function
}

Upvotes: 1

Views: 219

Answers (3)

Vinicius Kamakura
Vinicius Kamakura

Reputation: 7778

I know you got it, but just as a tip... (too big for a comment)

Check the return value of strdup() for NULL and free() those pointers when you are done with them. If you don't memory will leak (it is leaking in your current code).

The strdup() function shall return a pointer to a new string, which is a duplicate of the string pointed to by s1. The returned pointer can be passed to free(). A null pointer is returned if the new string cannot be created.

Upvotes: 0

Aasmund Eldhuset
Aasmund Eldhuset

Reputation: 37950

You have several buffer overflows: strdup() probably allocates a char array that is just large enough to hold the maildir string, and the calls to strcat() will then overflow the arrays. (strcat(), as opposed to strdup(), does not create a new char array, so you must ensure yourself that the array you give it is large enough to hold the resulting string.)

By the way, valgrind is your friend when it comes to tracking down memory management bugs.

Upvotes: 5

James
James

Reputation: 9278

There's not enough space in the duplicate strings for the concatenation.

try:

char* new = (char*)calloc(strlen(maildir) + 5);

etc

Upvotes: 1

Related Questions