free pointer to pointer in C

Question

I'm not familiar with C as much as with C++ but I found my self in need to debug C application and after even succeeded to do so, but I'm not sure if my fix indeed is correct.

Here is a code:

FcValueList **value = NULL;
value = (FcValueList **) malloc (SIZEOF_VOID_P * nobjs);

// other code...

if (value)
free (value);

The FcValueList is a structure and SIZEOF_VOID_P is a size of void pointer. The above code does not work because the value is "unable to read memory"

So I applied following fix and the code worked with app exiting with 0:

value = (FcValueList **) malloc (SIZEOF_VOID_P * nobjs);

// other code...

if (*value)
free (value);

Is my fix correct? and if not what would be correct then?

You don't need to typecast the return value of `malloc` in a C program. — Carl Norum, Feb 16 '15 at 22:08
This code doesn't make sense; if you need to store `FcValueList*`, you should use `sizeof(FcValueList*)`, not the size of a void pointer. — Oliver Charlesworth, Feb 16 '15 at 22:09
@Oliver SIZEOF_VOID_P is set by configure script, I would need to change a lot of code then. — codekiddy, Feb 16 '15 at 22:10
You should do, because it's semantically incorrect as it stands ;) — Oliver Charlesworth, Feb 16 '15 at 22:11
Chances are very high that `sizeof(void *)` is the same as `sizeof(FcValueList *)` on your platform. Don't sweat it too much. — Carl Norum, Feb 16 '15 at 22:12
@CarlNorum: That's not the only reason though. I'd argue that the above code is confusing to the reader/maintainer, because the types don't match, thus obscuring the intent. — Oliver Charlesworth, Feb 16 '15 at 22:13
@OliverCharlesworth - absolutely, and I agree. But sometimes you're working with a 3rd party project and it's not worth the effort. — Carl Norum, Feb 16 '15 at 22:14
[Don't cast the result of malloc (and friends)](http://stackoverflow.com/q/605845). Also, `SIZEOF_VOID_P` is painful. Use `sizeof *value` there instead. — Deduplicator, Feb 16 '15 at 22:15
Thank you guys for response, suggestion from Carl solved everything. — codekiddy, Feb 16 '15 at 22:16
Sometimes readability gets better if you write a typedef or a few. Here, e.g. ``typedef FcValueList* FcValueListPtr`` would result in less unusual code and probably less confusion. In combination with a better name for value: ``FcValueListPtr * valuePointers; valuePointers = malloc(sizeof(FcValueListPtr) * nobjs );`` — BitTickler, Feb 16 '15 at 22:19
@user2225104, I completely disagree with ever hiding a `*` inside a typedef like that. — Carl Norum, Feb 17 '15 at 01:13

score 6 · Accepted Answer · answered Feb 16 '15 at 22:10

6

No, your fix is not correct. Your original code is fine, but doesn't need the if statement. Passing a NULL pointer to free() is allowed.

Without more explanation, it's not really possible to give more context - your first code example is fine as shown.

answered Feb 16 '15 at 22:10

Carl Norum

219,201
40
422
469

Thank you so much! That worked, and I even learned something :) I can accept your answer in 7 minutes... – codekiddy Feb 16 '15 at 22:14

free pointer to pointer in C

1 Answers1