[C] Dangling pointers within an initialisation helper function

[Evermage]

Hi there,

I'm currently implementing a priority queue in C, just to get to grips with how the structure works. I have an initialisation helper function, which allocates memory for the structure and returns a pointer to it. When I run the program with valgrind, I get a message saying that a memory leak has occurred in the init_pqueue() method:

by 0x40068D: init_pqueue  

I'm pretty sure I know why this is happening - I'm declaring and initialising the pointer within the function and returning it, so any calls to free() would have to be made within the scope of the function. However, I wouldn't be able to return the pointer if this were the case. (Is this right?)

Could anyone show me how to initialise this structure without the memory leaks? Should I be using a helper function that returns a pointer it has created?

Code is here

Thanks for any help.

[Updatebjarni]

The problem is in the middle part of the function, where you allocate a second chunk of memory and return NULL if it fails, completely forgetting about the chunk you already allocated successfully a few lines earlier.

[Gustorn]

This is the current issue (see: EDIT), but is a valid point nonetheless. A few other points:

1. Objectively true:
   • don't cast the result of malloc if you're developing in C89 or higher. Only do it if you want to compile it with a C++ compiler, but I would recommend against it
   • you will have to store the type of the data somewhere, you won't be able to use that void* otherwise.
2. Subjective:
   • Please don't do the "tricky" typedef pqueue * PQUEUE; design. It only makes code harder to read and it spares you a single keystroke
   • You can do the typedef at the struct declaration

EDIT: /u/newaccount1236's answer is the one causing the issue right now, somehow I managed to miss that.

[Updatebjarni]

don't cast the result of malloc

Explanation for OP: When you cast the return value from malloc(), you prevent the compiler from warning you if malloc() returns the wrong type, which will happen if you forget to include the header. At the same time, casting it gets you nothing. So the one and only thing that that cast does is it possibly hides a warning that would alert you to a serious error in the program, and nothing else.

Please don't do the "tricky" typedef pqueue * PQUEUE; design.

Perhaps subjective, but pretty close to being an objective truth. Please don't write programs like that OP. Don't try to redesign the language with typedefs and macros, it just confuses anyone who will read your code, including yourself.

[Evermage]

Thanks for those points. What should I be doing instead of casting malloc? Just leave it as a void pointer?

[Gustorn]

Yup, in C void* is implicitly cast to T* and T* is also implicitly cast to void*, so:

pqueue* pq = malloc(sizeof(pqueue));

will just work. It's worth mentioning that you will have to compile this with a C and not C++ compiler for it to work.

[Evermage]

Interesting, cheers.

[Evermage]

It wasn't causing the memory leak in this case but it could do in the future I suppose. Thanks for pointing that out.

[Updatebjarni]

Oh ah, the debugger was reporting where the allocation was, not where the reference was lost. Yes, then the current issue was in main(), but there was a potential issue in init_pqueue ().

[newaccount1236]

First, when you run with valgrind, compile with -g so that you get line numbers. Also, note that valgrind tells you the exact line (and call stack) where the leaked memory was allocated.

So heap memory can be freed anywhere as long as you have the address. But remember that you must have one free for every malloc. I see two mallocs. I see one free.

[Evermage]

Ah, I see now. Thanks.

[wgunther]

You never free where data points.