This looks good, but take a look at my one thought below in the patch. On Nov 23, 2007 6:19 PM, Chantry Xavier wrote:

* 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 --- 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; Can the above be replaced by a call to MALLOC?

+ } 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