Free Memory in C (Can't fix memory leak)

Question

EDIT: I have changed my program according to suggestions people have made but I am unable to fix memory leaks. Also, I need to free them without using argc, so i need to somehow keep track of the array length so I marked the last element as null.

Currently I'm writing a C program that copies the command line arguments into a dynamically allocated array. My code looks like:

char **array;                                                                                                                                                                                     
int j;                                                                                                                                                                                        

array = malloc(sizeof(char*) * (argc + 1));                                                                                                                                                       
int i;                                                                                                                                                                                            
int index = 0;                                                                                                                                                                                    

for(i = 0; i < (argc); i++){                                                                                                                                                                      
    int length = strlen(*(argv + i));                                                                                                                                                             
    array[i] = malloc((length + 1) * sizeof(char));                                                                                                                                                                                                                                                                                                                                    
        // Cycle through all the chars and copy them in one by one                                                                                                                                
    for(j = 0; j <= length; j++){                                                                                                                                                                 
        array[i][j] = toupper(argv[i][j]);                                                                                                                                                        
    }                                                                                                                                                                                             
}      
array[i + 1] = NULL;                                                                                                                                                                                           

return array;

Later, I try to free the memory:

char** array_copy = array;
while(*array_copy != NULL){
    free(*array_copy++);
}
free(*array_copy) // free the null at the end
free(array);

However, I still get memory leaks. I'm not quite sure what I'm doing wrong. If anyone could give me tips that would be great.

Thank!

Why do you malloc `argc + 1` for array? You only need `argc` times. Also your delete `while` loop is broken. Always keep a count for the array size around. — pmr, Oct 14 '11 at 20:25

score 7 · Accepted Answer · answered Oct 14 '11 at 20:25

7

Your last line free(array) isn't freeing your original malloc, because you've incremented array as you are freeing its contents.

Also (as others point out):

your loop to free the array contents is checking for non-zero, but you don't ensure that the elements are zero to begin with.
You allocate argc+1 elements in array, when you only need argc.
*(argv + i) is the same as argv[i].

answered Oct 14 '11 at 20:25

Ned Batchelder

364,293
75
561
662

I have changed the code according to your (and other peoples') suggestions but I'm still leaking memory. The reason I use argc + 1 was to add an null element to keep track of the array end as for the assignment I was not allowed to use a loop with argc as a counter. Sorry for the confusion, I have edited my post to correspond with my current code. – Matt Oct 14 '11 at 20:44

score 2 · Answer 2 · answered Oct 14 '11 at 20:27

2

array has been completely ++'d off of the beginning. The free(array) is corrupting your heap, not freeing.

add

char ** array_iter = array;

and then change the loop to

while(*array_iter){
    free(*array_iter++);
}
free(array);

answered Oct 14 '11 at 20:27

Lou Franco

87,846
14
132
192

score 2 · Answer 3 · answered Oct 14 '11 at 20:30

2

By using the post-increment, you're setting your pointer equal to something other than what it was originally. You're no longer freeing the right memory.

answered Oct 14 '11 at 20:30

MGZero

5,812
5
29
46

score 0 · Answer 4 · answered Oct 14 '11 at 20:29

Your while loop isn't working. First only allocate argc times space for char*. Then allocate strlen + 1 for each string (null terminator) use strcpy to copy the strings and uppercase them afterwards (less error prone). Then have a free like this:

for(int i = 0; i < argc; ++i) free(array[i]);

Free Memory in C (Can't fix memory leak)

4 Answers4

Linked