[pacman-dev] [PATCH] Fix several memleaks, mostly related to errors handling.

Chantry Xavier shiningxc at gmail.com
Fri Nov 23 18:19:58 EST 2007


* The frontend calls alpm_trans_prepare(&data), and in case of errors,
receive the missing dependencies / conflicts / etc in the data pointer.
It apparently needs to free this structure totally with :
alpm_list_free_inner(data, free)
alpm_list_free(data)

So I added alpm_list_free_inner(data, free) in
pacman/{sync.c,remove.c,add,c}

* in _alpm_sync_prepare, the deps and asked lists were not freed in case
of errors (unresolvable conflicts).
Besides the code for handling this case was duplicated.

* in _alpm_remove_commit, free was used instead of alpm_list_free for
newfiles.

* newline fix in pacman/sync.c

Signed-off-by: Chantry Xavier <shiningxc at gmail.com>
---
 lib/libalpm/remove.c |    2 +-
 lib/libalpm/sync.c   |   42 ++++++++++++++++++------------------------
 src/pacman/add.c     |    1 +
 src/pacman/remove.c  |    1 +
 src/pacman/sync.c    |    3 ++-
 5 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c
index 592cf39..e1f19ec 100644
--- a/lib/libalpm/remove.c
+++ b/lib/libalpm/remove.c
@@ -315,7 +315,7 @@ int _alpm_remove_commit(pmtrans_t *trans, pmdb_t *db)
 						(pkg_count - alpm_list_count(targ) + 1));
 				position++;
 			}
-			free(newfiles);
+			alpm_list_free(newfiles);
 		}
 
 		/* set progress to 100% after we finish unlinking files */
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index f03a78b..b49bbfb 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -496,12 +496,13 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
 		if(deps) {
 			int errorout = 0;
 			alpm_list_t *asked = NULL;
+			pmconflict_t *conflict = NULL;
 
 			for(i = deps; i && !errorout; i = i->next) {
-				pmconflict_t *conflict = i->data;
 				pmsyncpkg_t *sync;
 				pmpkg_t *found = NULL;
 
+				conflict = i->data;
 				_alpm_log(PM_LOG_DEBUG, "package '%s' conflicts with '%s'\n",
 						conflict->package1, conflict->package2);
 				/* check if the conflicting package is about to be removed/replaced.
@@ -614,42 +615,35 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync
 							/* abort */
 							_alpm_log(PM_LOG_ERROR, _("unresolvable package conflicts detected\n"));
 							errorout = 1;
-							if(data) {
-								if((conflict = malloc(sizeof(pmconflict_t))) == NULL) {
-									_alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %zd bytes\n"), sizeof(pmconflict_t));
-									FREELIST(*data);
-									pm_errno = PM_ERR_MEMORY;
-									ret = -1;
-									goto cleanup;
-								}
-								*conflict = *(pmconflict_t *)i->data;
-								*data = alpm_list_add(*data, conflict);
-							}
 						}
 					}
 				} else {
 					_alpm_log(PM_LOG_ERROR, _("unresolvable package conflicts detected\n"));
 					errorout = 1;
-					if(data) {
-						if((conflict = malloc(sizeof(pmconflict_t))) == NULL) {
-							_alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %zd bytes\n"), sizeof(pmconflict_t));
-							FREELIST(*data);
-							pm_errno = PM_ERR_MEMORY;
-							ret = -1;
-							goto cleanup;
-						}
-						*conflict = *(pmconflict_t *)i->data;
-						*data = alpm_list_add(*data, conflict);
-					}
 				}
 			}
 			if(errorout) {
+				/* The last conflict was unresolvable, so we duplicate it and add it to *data */
 				pm_errno = PM_ERR_CONFLICTING_DEPS;
+				if(data) {
+					pmconflict_t *lastconflict = conflict;
+					if((conflict = malloc(sizeof(pmconflict_t))) == NULL) {
+						_alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %zd bytes\n"),
+								sizeof(pmconflict_t));
+						FREELIST(*data);
+						pm_errno = PM_ERR_MEMORY;
+					} else {
+						*conflict = *lastconflict;
+						*data = alpm_list_add(*data, conflict);
+					}
+				}
+				FREELIST(asked);
+				FREELIST(deps);
 				ret = -1;
 				goto cleanup;
 			}
-			FREELIST(deps);
 			FREELIST(asked);
+			FREELIST(deps);
 		}
 		EVENT(trans, PM_TRANS_EVT_INTERCONFLICTS_DONE, NULL, NULL);
 	}
diff --git a/src/pacman/add.c b/src/pacman/add.c
index 7d18749..e04707f 100644
--- a/src/pacman/add.c
+++ b/src/pacman/add.c
@@ -175,6 +175,7 @@ int pacman_add(alpm_list_t *targets)
 				break;
 		}
 		add_cleanup();
+		alpm_list_free_inner(data, free);
 		alpm_list_free(data);
 		return(1);
 	}
diff --git a/src/pacman/remove.c b/src/pacman/remove.c
index e1e4f04..1028d9e 100644
--- a/src/pacman/remove.c
+++ b/src/pacman/remove.c
@@ -133,6 +133,7 @@ int pacman_remove(alpm_list_t *targets)
 							depstring);
 					free(depstring);
 				}
+				alpm_list_free_inner(data, free);
 				alpm_list_free(data);
 				break;
 			default:
diff --git a/src/pacman/sync.c b/src/pacman/sync.c
index 41d18a9..889c682 100644
--- a/src/pacman/sync.c
+++ b/src/pacman/sync.c
@@ -621,7 +621,7 @@ int sync_trans(alpm_list_t *targets, int sync_only)
 			case PM_ERR_CONFLICTING_DEPS:
 			  for(i = data; i; i = alpm_list_next(i)) {
 					pmconflict_t *conflict = alpm_list_getdata(i);
-					printf(_(":: %s: conflicts with %s"),
+					printf(_(":: %s: conflicts with %s\n"),
 							alpm_conflict_get_package1(conflict), alpm_conflict_get_package2(conflict));
 				}
 				break;
@@ -706,6 +706,7 @@ int sync_trans(alpm_list_t *targets, int sync_only)
 	/* Step 4: release transaction resources */
 cleanup:
 	if(data) {
+		alpm_list_free_inner(data, free);
 		alpm_list_free(data);
 	}
 	if(alpm_trans_release() == -1) {
-- 
1.5.3.6





More information about the pacman-dev mailing list