On Nov 21, 2007 11:30 AM, Xavier
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=a57b2f233f28c...
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