On Wed, 2014-02-26 at 19:36 +0100, Lukas Slebodnik wrote:
On (26/02/14 13:00), Simo Sorce wrote:
>On Wed, 2014-02-26 at 18:33 +0100, Lukas Slebodnik wrote:
>> ehlo,
>>
>> Reported by: cppcheck
>>
>> void free_fun(struct info *info)
>> free(info->name);
>> free(info);
>> info = NULL;
>> ^^^^^^^^^^^
>> Assignment to function parameter has no effect outside the function.
>>
>> Patch is attached.
>
>Although the assignment is useless I do not like the solution.
>
>I think the solution is not to remove the assignment but to change the
>function prototype to take a pointer to the pointer we want to free so
>that at the end of the function we can assign NULL to the original
>variable we have been called to free.
>Otherwise the caller has a dangling pointer.
>
>So the solution for me is:
>void free_fun(struct info **_info) {
> struct info *info = *_info;
> free(info->name);
> free(info);
> *info = NULL;
>}
>
>Simo.
>
I do not agree with you.
Free functions have parameter (void *) which will be freed.
e.g.:
void free(void *ptr);
int talloc_free (void *ptr)
/**
* Free a string allocated by a krb5 function.
*
* @param [in] context Library context
* @param [in] val String to be freed
*
* @version First introduced in 1.10
*/
void KRB5_CALLCONV
krb5_free_string(krb5_context context, char *val);
If you do not want to have dangling pointer, you need to assign NULL yourself
or use macro.
gssapi OTOH uses th convention that you have to pass a pointer and then
NULLs the one you passed int, and I find it much less prone to mistakes.
#define talloc_zfree(ptr) \
do { talloc_free(discard_const(ptr)); ptr = NULL; } while(0)
Note that it is common agreement even upstream (I think) that this is an
unfortunate side-effect of not having thought about doing this right
beofre the API was finalized.
Having to define macros is not a good idea in general, it adds
unnecessary churn.
I do not want to introduce new "semantic" to free
functions.
They are not new in any way, a lot of APIs use the semantic I recommend
and it is a better one, in general.
In many cases, you cannot change API of free function, because they
are used
as a callback.
If there is a technical issue that prevents changing a specific function
then that's fine, but I prefer the way I shown as a general solution, I
started using it quite a while ago in all projects I am active in.
Simo.
--
Simo Sorce * Red Hat, Inc * New York