How to properly free char ** table in C

Question

I need your advice on this piece of code: the table field parameters [0], options [1], etc., do not seem to be correctly released. thank you for your responses

int main() { .... char **options; options = generate_fields(user_input); for(i = 0; i < sizeof(options) / sizeof(options[0]); i++) { free(options[i]); options[i] = NULL; } free(options); } char ** generate_fields(char *) { char ** options = malloc(256*sizeof(char *)); ... return options; } 

Answer 1

The problem is this:

 for(i = 0; i < sizeof(options) / sizeof(options[0]); i++) 

options is a pointer type, not an array type, so sizeof(options) will always be the same (usually 4 bytes on a 32-bit machine or 8 bytes on a 64-bit machine), therefore sizeof(options)/sizeof(options[0]) will almost always be 1.

The key should always be in free memory the same way you malloc 'ed. So, if you malloc two-dimensional array, and then malloc series of one-dimensional arrays, you need to do the reverse when freeing it:

 char ** generate_fields(char *) { char ** options = malloc(256*sizeof(char *)); for(int i = 0; i < 256; i++) options[i] = malloc(some_size); return options; } void free_fields(char ** options) { for(int i = 0; i < 256; i++) free(options[i]); free(options); } 

Please note that if the size (256 in this case) is not a constant, you need to track it yourself, because otherwise you have no way to find out how many times the loop is released.

Answer 2

You should have the same amount of free as you have malloc s.

In your code, you are allocating an array of pointers, but you are not allocating any memory for the individual elements of the array that you are pointing to. But your release code is written as if you did.

Answer 3

I am going to add an answer to Adam here because it probably won't fit in the comment. Adam is absolutely right. I suspect your generate_fields function, however, may actually receive input from the user, I'm not sure. In any case, there are two ways to approach this:

 char ** generate_fields(char *, int num_fields, int size_of_field) { char ** options = malloc(num_fields*sizeof(char *)); for(int i = 0; i < num_fields; i++) options[i] = malloc(size_of_field); return options; } 

and the corresponding free function, which I will leave for brevity. You can see what happens - we pass the number of fields and the size of the field. Change this as necessary. Another option is to generate fields that return to the routine that he called from the size of the array. I would do something like this:

 int generate_fields(char** options) { int num_fields = 0; // somewhere here we get num_fields options = malloc(num_fields*sizeof(char *)); for(int i = 0; i < num_fields; i++) options[i] = malloc(size_of_field); return num_fields; } 

And you call from the main, like this:

 int main() { int sizeofarray = 0; char** fields; sizeofarray = generate_fields(fields); 

Or, if you do not like these notations, you can always stick to what you had:

 char** generate_fields(int* size) 

As a function prototype (return the parameters this time and make size= somewhere in the code and call from the main one, like this:

 int sizeofarray = 0; char** options; options = generate_fields(&sizeofarray); 

Hope that gives you a few more ideas, Adam, feel free to edit all / all of this in your answer, if necessary, anyway from your answer.