[pacman-dev] FREELIST macros / little memleak fix

[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=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)

Hm, actually while writing this mail, I realize this it not very problematic :P
In the worst case, we just need two little additional calls. But I still
don't find these very messy.

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)" ?


>From fdf689932ca1fa919b95533a2469f797f22970f1 Mon Sep 17 00:00:00 2001
From: Chantry Xavier <shiningxc at gmail.com>
Date: Wed, 21 Nov 2007 17:10:20 +0100
Subject: [PATCH] Fix a memleak in _alpm_sync_free.

An alpm_list_free call was missing.
Also make use of alpm_list_free_inner in both _alpm_sync_free and
_alpm_trans_free.

Signed-off-by: Chantry Xavier <shiningxc at gmail.com>
---
 lib/libalpm/sync.c  |    7 ++-----
 lib/libalpm/trans.c |   10 ++--------
 2 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index 16b1998..f03a78b 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -74,11 +74,8 @@ void _alpm_sync_free(pmsyncpkg_t *sync)
 
 	/* TODO wow this is ugly */
 	if(sync->type == PM_SYNC_TYPE_REPLACE) {
-		alpm_list_t *tmp;
-		for(tmp = sync->data; tmp; tmp = alpm_list_next(tmp)) {
-			_alpm_pkg_free(tmp->data);
-			tmp->data = NULL;
-		}
+		alpm_list_free_inner(sync->data, (alpm_list_fn_free)_alpm_pkg_free);
+		alpm_list_free(sync->data);
 		sync->data = NULL;
 	} else {
 		_alpm_pkg_free(sync->data);
diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c
index 19ae2b6..d998826 100644
--- a/lib/libalpm/trans.c
+++ b/lib/libalpm/trans.c
@@ -246,8 +246,6 @@ pmtrans_t *_alpm_trans_new()
 
 void _alpm_trans_free(pmtrans_t *trans)
 {
-	alpm_list_t *i;
-
 	ALPM_LOG_FUNC;
 
 	if(trans == NULL) {
@@ -256,13 +254,9 @@ void _alpm_trans_free(pmtrans_t *trans)
 
 	FREELIST(trans->targets);
 	if(trans->type == PM_TRANS_TYPE_SYNC) {
-		for(i = trans->packages; i; i = alpm_list_next(i)) {
-			_alpm_sync_free(i->data);
-		}
+		alpm_list_free_inner(trans->packages, (alpm_list_fn_free)_alpm_sync_free);
 	} else {
-		for(i = trans->packages; i; i = alpm_list_next(i)) {
-			_alpm_pkg_free(i->data);
-		}
+		alpm_list_free_inner(trans->packages, (alpm_list_fn_free)_alpm_pkg_free);
 	}
 	alpm_list_free(trans->packages);
 
-- 
1.5.3.6