[pacman-dev] FREELIST macros / little memleak fix

[Dan McGee]

On Nov 21, 2007 11:30 AM, Xavier <shiningxc at gmail.com> wrote:
> 1)
> I remember having already asked that, but it was probably just on irc, and
> anyway I forgot what the answer was (if there was one ;)).
> It's about commit a57b2f233 :
> http://projects.archlinux.org/git/?p=pacman.git;a=commitdiff;h=a57b2f233f28c275b0b171cb291415351f9ec87d
>
> What was wrong / messy about these macros :
>
> -/* TODO we should do away with these... they're messy*/
> -#define _FREELIST(p, f) do { alpm_list_free_inner(p, f); alpm_list_free(p); p = NULL; } while(0)
> -#define FREELIST(p)     _FREELIST(p, free)
> -#define FREELISTPTR(p)  do { alpm_list_free(p); p = NULL; } while(0)
> +#define FREELIST(p) do { alpm_list_free_inner(p, free); alpm_list_free(p); p = NULL; } while(0)

There were three macros that you didn't know what needed using without
actually looking at them, so instead I simplified it down to one
macro. FREELISTPTR saved zero coding while only adding unnecessary
abstraction.

Macros calling macros = suck, IMO.

> 2)
> If I mentioning this, it's because I found a little memleak in
> _alpm_sync_free: a missing alpm_list_free call. So maybe the above
> _FREELIST(p, f) macro could have helped.
>
> Also, for some reasons, both _alpm_sync_free and _alpm_trans_free did the job
> of alpm_list_free_inner manually. So I changed that.
>
> 3)
> A little problem : all _alpm_*_free don't match the alpm_list_fn_free
> prototype except one : _alpm_graph_free .
> All others don't take a void * argument, so they need to be casted to
> alpm_list_fn_free for being used with alpm_list_free_inner .
>
> So should "static void _alpm_graph_free(void *data)" be changed to
> "static void _alpm_graph_free(pmgraph_t *graph)" ?

Hmm, interesting. I like type-safe functions, but that (_alpm_fn_free)
cast seems so dirty. Not sure what to do here.

-Dan