[pacman-dev] [PATCH 1/8] avoid double freeing unresolvable packages
If an unresolvable package was loaded from a file it would be free'd again when freeing trans->add. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/trans.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index ada8100..4930e94 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -246,12 +246,19 @@ int SYMEXPORT alpm_trans_release(alpm_handle_t *handle) void _alpm_trans_free(alpm_trans_t *trans) { + alpm_list_t *i; + if(trans == NULL) { return; } - alpm_list_free_inner(trans->unresolvable, - (alpm_list_fn_free)_alpm_pkg_free_trans); + /* only free packages not in trans->add to avoid + * double free'ing packages loaded from files. */ + for(i = trans->unresolvable; i; i = i->next) { + if(!alpm_list_find_ptr(trans->add, i->data)) { + _alpm_pkg_free_trans(i->data); + } + } alpm_list_free(trans->unresolvable); alpm_list_free_inner(trans->add, (alpm_list_fn_free)_alpm_pkg_free_trans); alpm_list_free(trans->add); -- 1.8.5.2
If the user opted not to remove the unresolvable packages from the transaction, the list wasn't saved to the transaction to be free'd in trans_release. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/sync.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 4ae01ac..b6061db 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -370,7 +370,6 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) { alpm_list_t *i, *j; alpm_list_t *deps = NULL; - alpm_list_t *unresolvable = NULL; size_t from_sync = 0; int ret = 0; alpm_trans_t *trans = handle->trans; @@ -427,7 +426,11 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) alpm_pkg_t *pkg = i->data; if(_alpm_resolvedeps(handle, localpkgs, pkg, trans->add, &resolved, remove, data) == -1) { - unresolvable = alpm_list_add(unresolvable, pkg); + /* Unresolvable packages will be removed from the target list; set + * these aside in the transaction as a list we won't operate on. If we + * free them before the end of the transaction, we may kill pointers + * the frontend holds to package objects. */ + trans->unresolvable = alpm_list_add(trans->unresolvable, pkg); } /* Else, [resolved] now additionally contains [pkg] and all of its dependencies not already on the list */ @@ -437,10 +440,10 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) /* If there were unresolvable top-level packages, prompt the user to see if they'd like to ignore them rather than failing the sync */ - if(unresolvable != NULL) { + if(trans->unresolvable != NULL) { int remove_unresolvable = 0; alpm_errno_t saved_err = handle->pm_errno; - QUESTION(handle, ALPM_QUESTION_REMOVE_PKGS, unresolvable, + QUESTION(handle, ALPM_QUESTION_REMOVE_PKGS, trans->unresolvable, NULL, NULL, &remove_unresolvable); if(remove_unresolvable) { /* User wants to remove the unresolvable packages from the @@ -470,12 +473,6 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) } } - /* Unresolvable packages will be removed from the target list; set these - * aside in the transaction as a list we won't operate on. If we free them - * before the end of the transaction, we may kill pointers the frontend - * holds to package objects. */ - trans->unresolvable = unresolvable; - alpm_list_free(trans->add); trans->add = resolved; -- 1.8.5.2
On 07/01/14 02:52, Andrew Gregory wrote:
If the user opted not to remove the unresolvable packages from the transaction, the list wasn't saved to the transaction to be free'd in trans_release.
It is not removed from trans->add. See below.
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/sync.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 4ae01ac..b6061db 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -370,7 +370,6 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) { alpm_list_t *i, *j; alpm_list_t *deps = NULL; - alpm_list_t *unresolvable = NULL; size_t from_sync = 0; int ret = 0; alpm_trans_t *trans = handle->trans; @@ -427,7 +426,11 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) alpm_pkg_t *pkg = i->data; if(_alpm_resolvedeps(handle, localpkgs, pkg, trans->add, &resolved, remove, data) == -1) { - unresolvable = alpm_list_add(unresolvable, pkg); + /* Unresolvable packages will be removed from the target list; set + * these aside in the transaction as a list we won't operate on. If we + * free them before the end of the transaction, we may kill pointers + * the frontend holds to package objects. */ + trans->unresolvable = alpm_list_add(trans->unresolvable, pkg); } /* Else, [resolved] now additionally contains [pkg] and all of its dependencies not already on the list */ @@ -437,10 +440,10 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data)
/* If there were unresolvable top-level packages, prompt the user to see if they'd like to ignore them rather than failing the sync */ - if(unresolvable != NULL) { + if(trans->unresolvable != NULL) { int remove_unresolvable = 0; alpm_errno_t saved_err = handle->pm_errno; - QUESTION(handle, ALPM_QUESTION_REMOVE_PKGS, unresolvable, + QUESTION(handle, ALPM_QUESTION_REMOVE_PKGS, trans->unresolvable, NULL, NULL, &remove_unresolvable); if(remove_unresolvable) { /* User wants to remove the unresolvable packages from the @@ -470,12 +473,6 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) } }
- /* Unresolvable packages will be removed from the target list; set these - * aside in the transaction as a list we won't operate on. If we free them - * before the end of the transaction, we may kill pointers the frontend - * holds to package objects. */ - trans->unresolvable = unresolvable; - alpm_list_free(trans->add); trans->add = resolved;
Here... when the unresolved packages are added to trans->unresolvable, the trans->add list is updated to remove them. What am I missing here? Allan
If the user opted not to remove the unresolvable packages from the transaction, the list was neither free'd nor saved to the transaction to be free'd in trans_release. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- This supersedes patch 1/8 (avoid double freeing unresovable packages) as well, which was only necessary because the previous version made it possible for packages to be in both trans->add and trans-unresolvable. lib/libalpm/sync.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index fb13324..c6c50a4 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -460,6 +460,7 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) /* pm_errno was set by resolvedeps, callback may have overwrote it */ handle->pm_errno = ALPM_ERR_UNSATISFIED_DEPS; alpm_list_free(resolved); + alpm_list_free(unresolvable); ret = -1; goto cleanup; } -- 1.8.5.3
On 07/02/14 11:49, Andrew Gregory wrote:
If the user opted not to remove the unresolvable packages from the transaction, the list was neither free'd nor saved to the transaction to be free'd in trans_release.
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
This supersedes patch 1/8 (avoid double freeing unresovable packages) as well, which was only necessary because the previous version made it possible for packages to be in both trans->add and trans-unresolvable.
Much better! And I note all tests pass with --valgrind now. Allan
If the db directory did not exist when local_db_populate was called, the pkgcache wouldn't be initialized, causing pkghash_add_pkg to fail. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/be_local.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 6029e12..671eed1 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -459,6 +459,7 @@ static int local_db_populate(alpm_db_t *db) /* no database existing yet is not an error */ db->status &= ~DB_STATUS_EXISTS; db->status |= DB_STATUS_MISSING; + db->pkgcache = _alpm_pkghash_create(0); return 0; } RET_ERR(db->handle, ALPM_ERR_DB_OPEN, -1); -- 1.8.5.2
alpm_pkg_compute_optional returns a generated list that needs to be free'd. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/package.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pacman/package.c b/src/pacman/package.c index 8ccdc1b..be01f63 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -235,6 +235,7 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra) printf("\n"); FREELIST(requiredby); + FREELIST(optionalfor); alpm_list_free(validation); } -- 1.8.5.2
_alpm_resolvedeps resets pm_errno to 0 by calling alpm_checkdeps. Whenever the last call succeeded, pm_errno was not properly set, preventing pacman from properly handling the error and leaking additional memory. We know pm_errno should be ALPM_ERR_UNSATISFIED_DEPS if resolvedeps has failed, so just set it manually. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/sync.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index b6061db..8b8a6ad 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -442,7 +442,6 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) see if they'd like to ignore them rather than failing the sync */ if(trans->unresolvable != NULL) { int remove_unresolvable = 0; - alpm_errno_t saved_err = handle->pm_errno; QUESTION(handle, ALPM_QUESTION_REMOVE_PKGS, trans->unresolvable, NULL, NULL, &remove_unresolvable); if(remove_unresolvable) { @@ -458,7 +457,7 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) } } else { /* pm_errno was set by resolvedeps, callback may have overwrote it */ - handle->pm_errno = saved_err; + handle->pm_errno = ALPM_ERR_UNSATISFIED_DEPS; alpm_list_free(resolved); ret = -1; goto cleanup; -- 1.8.5.2
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/deps.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 4eb779f..1f6ad11 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -833,8 +833,9 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, } ret = -1; } + alpm_list_free(targ); + targ = NULL; } - alpm_list_free(targ); alpm_list_free(deps); if(ret != 0) { -- 1.8.5.2
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/util.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 58b0cec..1982c89 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -589,9 +589,10 @@ static int table_display(const alpm_list_t *header, const alpm_list_t *i, *first; size_t *widths = NULL, totalcols, totalwidth; int *has_data = NULL; + int ret = 0; if(rows == NULL) { - return 0; + return ret; } /* we want the first row. if no headers are provided, use the first @@ -605,10 +606,12 @@ static int table_display(const alpm_list_t *header, if(totalwidth > cols) { pm_printf(ALPM_LOG_WARNING, _("insufficient columns available for table display\n")); - return -1; + ret = -1; + goto cleanup; } if(!totalwidth || !widths || !has_data) { - return -1; + ret = -1; + goto cleanup; } if(header) { @@ -620,9 +623,10 @@ static int table_display(const alpm_list_t *header, table_print_line(i->data, padding, totalcols, widths, has_data); } +cleanup: free(widths); free(has_data); - return 0; + return ret; } void list_display(const char *title, const alpm_list_t *list, -- 1.8.5.2
Front-ends should be able to free memory that alpm hands them. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/alpm.h | 4 ++++ lib/libalpm/conflict.c | 6 +++--- lib/libalpm/conflict.h | 3 --- lib/libalpm/deps.c | 8 ++++---- lib/libalpm/deps.h | 1 - lib/libalpm/remove.c | 6 +++--- lib/libalpm/sync.c | 14 +++++++------- src/pacman/remove.c | 3 ++- src/pacman/sync.c | 13 ++++++++----- 9 files changed, 31 insertions(+), 27 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 5628527..28ae851 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -1351,6 +1351,10 @@ enum alpm_caps { const char *alpm_version(void); enum alpm_caps alpm_capabilities(void); +void alpm_fileconflict_free(alpm_fileconflict_t *conflict); +void alpm_depmiss_free(alpm_depmissing_t *miss); +void alpm_conflict_free(alpm_conflict_t *conflict); + /* End of alpm_api */ /** @} */ diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 5a6e2be..4a1d77e 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -62,7 +62,7 @@ static alpm_conflict_t *conflict_new(alpm_pkg_t *pkg1, alpm_pkg_t *pkg2, /** * @brief Free a conflict and its members. */ -void _alpm_conflict_free(alpm_conflict_t *conflict) +void SYMEXPORT alpm_conflict_free(alpm_conflict_t *conflict) { FREE(conflict->package2); FREE(conflict->package1); @@ -135,7 +135,7 @@ static int add_conflict(alpm_handle_t *handle, alpm_list_t **baddeps, pkg1->name, pkg2->name, conflict_str); free(conflict_str); } else { - _alpm_conflict_free(conflict); + alpm_conflict_free(conflict); } return 0; } @@ -290,7 +290,7 @@ error: /** * @brief Frees a conflict and its members. */ -void _alpm_fileconflict_free(alpm_fileconflict_t *conflict) +void SYMEXPORT alpm_fileconflict_free(alpm_fileconflict_t *conflict) { FREE(conflict->ctarget); FREE(conflict->file); diff --git a/lib/libalpm/conflict.h b/lib/libalpm/conflict.h index f7a667d..182eb0e 100644 --- a/lib/libalpm/conflict.h +++ b/lib/libalpm/conflict.h @@ -25,14 +25,11 @@ #include "package.h" alpm_conflict_t *_alpm_conflict_dup(const alpm_conflict_t *conflict); -void _alpm_conflict_free(alpm_conflict_t *conflict); alpm_list_t *_alpm_innerconflicts(alpm_handle_t *handle, alpm_list_t *packages); alpm_list_t *_alpm_outerconflicts(alpm_db_t *db, alpm_list_t *packages); alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, alpm_list_t *upgrade, alpm_list_t *remove); -void _alpm_fileconflict_free(alpm_fileconflict_t *conflict); - #endif /* _ALPM_CONFLICT_H */ /* vim: set ts=2 sw=2 noet: */ diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 1f6ad11..7ee3ee4 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -57,7 +57,7 @@ static alpm_depmissing_t *depmiss_new(const char *target, alpm_depend_t *dep, return miss; } -void _alpm_depmiss_free(alpm_depmissing_t *miss) +void SYMEXPORT alpm_depmiss_free(alpm_depmissing_t *miss) { _alpm_dep_free(miss->depend); FREE(miss->target); @@ -804,7 +804,7 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, /* check if one of the packages in the [*packages] list already satisfies * this dependency */ if(find_dep_satisfier(*packages, missdep)) { - _alpm_depmiss_free(miss); + alpm_depmiss_free(miss); continue; } /* check if one of the packages in the [preferred] list already satisfies @@ -818,9 +818,9 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, _alpm_log(handle, ALPM_LOG_DEBUG, "pulling dependency %s (needed by %s)\n", spkg->name, pkg->name); - _alpm_depmiss_free(miss); + alpm_depmiss_free(miss); } else if(resolvedep(handle, missdep, (targ = alpm_list_add(NULL, handle->db_local)), rem, 0)) { - _alpm_depmiss_free(miss); + alpm_depmiss_free(miss); } else { handle->pm_errno = ALPM_ERR_UNSATISFIED_DEPS; char *missdepstring = alpm_dep_compute_string(missdep); diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 546521f..0d41d49 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -29,7 +29,6 @@ void _alpm_dep_free(alpm_depend_t *dep); alpm_depend_t *_alpm_dep_dup(const alpm_depend_t *dep); -void _alpm_depmiss_free(alpm_depmissing_t *miss); alpm_list_t *_alpm_sortbydeps(alpm_handle_t *handle, alpm_list_t *targets, alpm_list_t *ignore, int reverse); int _alpm_recursedeps(alpm_db_t *db, alpm_list_t **targs, int include_explicit); diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 899952b..04831fa 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -116,7 +116,7 @@ static int remove_prepare_cascade(alpm_handle_t *handle, alpm_list_t *lp) _("could not find %s in database -- skipping\n"), miss->target); } } - alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free); + alpm_list_free_inner(lp, (alpm_list_fn_free)alpm_depmiss_free); alpm_list_free(lp); lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(handle->db_local), trans->remove, NULL, 1); @@ -153,7 +153,7 @@ static void remove_prepare_keep_needed(alpm_handle_t *handle, alpm_list_t *lp) _alpm_pkg_free(pkg); } } - alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free); + alpm_list_free_inner(lp, (alpm_list_fn_free)alpm_depmiss_free); alpm_list_free(lp); lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(handle->db_local), trans->remove, NULL, 1); @@ -232,7 +232,7 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data) if(data) { *data = lp; } else { - alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free); + alpm_list_free_inner(lp, (alpm_list_fn_free)alpm_depmiss_free); alpm_list_free(lp); } RET_ERR(handle, ALPM_ERR_UNSATISFIED_DEPS, -1); diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 8b8a6ad..1bedbd1 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -451,7 +451,7 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) dependency-reordered list below */ handle->pm_errno = 0; if(data) { - alpm_list_free_inner(*data, (alpm_list_fn_free)_alpm_depmiss_free); + alpm_list_free_inner(*data, (alpm_list_fn_free)alpm_depmiss_free); alpm_list_free(*data); *data = NULL; } @@ -521,7 +521,7 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) *data = alpm_list_add(*data, newconflict); } } - alpm_list_free_inner(deps, (alpm_list_fn_free)_alpm_conflict_free); + alpm_list_free_inner(deps, (alpm_list_fn_free)alpm_conflict_free); alpm_list_free(deps); _alpm_dep_free(dep1); _alpm_dep_free(dep2); @@ -539,7 +539,7 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) trans->unresolvable = alpm_list_add(trans->unresolvable, rsync); } - alpm_list_free_inner(deps, (alpm_list_fn_free)_alpm_conflict_free); + alpm_list_free_inner(deps, (alpm_list_fn_free)alpm_conflict_free); alpm_list_free(deps); deps = NULL; @@ -585,13 +585,13 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) *data = alpm_list_add(*data, newconflict); } } - alpm_list_free_inner(deps, (alpm_list_fn_free)_alpm_conflict_free); + alpm_list_free_inner(deps, (alpm_list_fn_free)alpm_conflict_free); alpm_list_free(deps); goto cleanup; } } EVENT(handle, ALPM_EVENT_INTERCONFLICTS_DONE, NULL, NULL); - alpm_list_free_inner(deps, (alpm_list_fn_free)_alpm_conflict_free); + alpm_list_free_inner(deps, (alpm_list_fn_free)alpm_conflict_free); alpm_list_free(deps); } @@ -621,7 +621,7 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) if(data) { *data = deps; } else { - alpm_list_free_inner(deps, (alpm_list_fn_free)_alpm_depmiss_free); + alpm_list_free_inner(deps, (alpm_list_fn_free)alpm_depmiss_free); alpm_list_free(deps); } goto cleanup; @@ -1267,7 +1267,7 @@ int _alpm_sync_commit(alpm_handle_t *handle, alpm_list_t **data) if(data) { *data = conflict; } else { - alpm_list_free_inner(conflict, (alpm_list_fn_free)_alpm_fileconflict_free); + alpm_list_free_inner(conflict, (alpm_list_fn_free)alpm_fileconflict_free); alpm_list_free(conflict); } RET_ERR(handle, ALPM_ERR_FILE_CONFLICTS, -1); diff --git a/src/pacman/remove.c b/src/pacman/remove.c index aefe454..368c2e8 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -121,12 +121,13 @@ int pacman_remove(alpm_list_t *targets) char *depstring = alpm_dep_compute_string(miss->depend); colon_printf(_("%s: requires %s\n"), miss->target, depstring); free(depstring); + alpm_depmiss_free(miss); } break; default: break; } - FREELIST(data); + alpm_list_free(data); retval = 1; goto cleanup; } diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 5ed43ed..e1df860 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -763,8 +763,9 @@ int sync_prepare_execute(void) switch(err) { case ALPM_ERR_PKG_INVALID_ARCH: for(i = data; i; i = alpm_list_next(i)) { - const char *pkg = i->data; + char *pkg = i->data; colon_printf(_("package %s does not have a valid architecture\n"), pkg); + free(pkg); } break; case ALPM_ERR_UNSATISFIED_DEPS: @@ -773,6 +774,7 @@ int sync_prepare_execute(void) char *depstring = alpm_dep_compute_string(miss->depend); colon_printf(_("%s: requires %s\n"), miss->target, depstring); free(depstring); + alpm_depmiss_free(miss); } break; case ALPM_ERR_CONFLICTING_DEPS: @@ -788,6 +790,7 @@ int sync_prepare_execute(void) conflict->package1, conflict->package2, reason); free(reason); } + alpm_conflict_free(conflict); } break; default: @@ -847,6 +850,7 @@ int sync_prepare_execute(void) conflict->target, conflict->file); break; } + alpm_fileconflict_free(conflict); } break; case ALPM_ERR_PKG_INVALID: @@ -854,8 +858,9 @@ int sync_prepare_execute(void) case ALPM_ERR_PKG_INVALID_SIG: case ALPM_ERR_DLT_INVALID: for(i = data; i; i = alpm_list_next(i)) { - const char *filename = i->data; + char *filename = i->data; printf(_("%s is invalid or corrupted\n"), filename); + free(filename); } break; default: @@ -869,9 +874,7 @@ int sync_prepare_execute(void) /* Step 4: release transaction resources */ cleanup: - if(data) { - FREELIST(data); - } + alpm_list_free(data); if(trans_release() == -1) { retval = 1; } -- 1.8.5.2
On 07/01/14 02:52, Andrew Gregory wrote:
Front-ends should be able to free memory that alpm hands them.
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/alpm.h | 4 ++++ lib/libalpm/conflict.c | 6 +++--- lib/libalpm/conflict.h | 3 --- lib/libalpm/deps.c | 8 ++++---- lib/libalpm/deps.h | 1 - lib/libalpm/remove.c | 6 +++--- lib/libalpm/sync.c | 14 +++++++------- src/pacman/remove.c | 3 ++- src/pacman/sync.c | 13 ++++++++----- 9 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 5628527..28ae851 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -1351,6 +1351,10 @@ enum alpm_caps { const char *alpm_version(void); enum alpm_caps alpm_capabilities(void);
+void alpm_fileconflict_free(alpm_fileconflict_t *conflict); +void alpm_depmiss_free(alpm_depmissing_t *miss);
alpm_depmissing_free() - no point save three letters I was looking for a better position in the head to group these, but did not really see anything. Unless someone else has a good idea, I guess they will be fine there. Rest of patch was fine.
Front-ends should be able to free memory that alpm hands them. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- renamed alpm_depmiss_free -> alpm_depmissing_free lib/libalpm/alpm.h | 4 ++++ lib/libalpm/conflict.c | 6 +++--- lib/libalpm/conflict.h | 3 --- lib/libalpm/deps.c | 8 ++++---- lib/libalpm/deps.h | 1 - lib/libalpm/remove.c | 7 ++++--- lib/libalpm/sync.c | 16 +++++++++------- src/pacman/remove.c | 3 ++- src/pacman/sync.c | 13 ++++++++----- 9 files changed, 34 insertions(+), 27 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index d67d66a..6a2a1fe 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -1351,6 +1351,10 @@ enum alpm_caps { const char *alpm_version(void); enum alpm_caps alpm_capabilities(void); +void alpm_fileconflict_free(alpm_fileconflict_t *conflict); +void alpm_depmissing_free(alpm_depmissing_t *miss); +void alpm_conflict_free(alpm_conflict_t *conflict); + /* End of alpm_api */ /** @} */ diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index 369530b..2611aef 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -62,7 +62,7 @@ static alpm_conflict_t *conflict_new(alpm_pkg_t *pkg1, alpm_pkg_t *pkg2, /** * @brief Free a conflict and its members. */ -void _alpm_conflict_free(alpm_conflict_t *conflict) +void SYMEXPORT alpm_conflict_free(alpm_conflict_t *conflict) { FREE(conflict->package2); FREE(conflict->package1); @@ -135,7 +135,7 @@ static int add_conflict(alpm_handle_t *handle, alpm_list_t **baddeps, pkg1->name, pkg2->name, conflict_str); free(conflict_str); } else { - _alpm_conflict_free(conflict); + alpm_conflict_free(conflict); } return 0; } @@ -290,7 +290,7 @@ error: /** * @brief Frees a conflict and its members. */ -void _alpm_fileconflict_free(alpm_fileconflict_t *conflict) +void SYMEXPORT alpm_fileconflict_free(alpm_fileconflict_t *conflict) { FREE(conflict->ctarget); FREE(conflict->file); diff --git a/lib/libalpm/conflict.h b/lib/libalpm/conflict.h index 8cf36ec..886e927 100644 --- a/lib/libalpm/conflict.h +++ b/lib/libalpm/conflict.h @@ -25,14 +25,11 @@ #include "package.h" alpm_conflict_t *_alpm_conflict_dup(const alpm_conflict_t *conflict); -void _alpm_conflict_free(alpm_conflict_t *conflict); alpm_list_t *_alpm_innerconflicts(alpm_handle_t *handle, alpm_list_t *packages); alpm_list_t *_alpm_outerconflicts(alpm_db_t *db, alpm_list_t *packages); alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, alpm_list_t *upgrade, alpm_list_t *remove); -void _alpm_fileconflict_free(alpm_fileconflict_t *conflict); - #endif /* _ALPM_CONFLICT_H */ /* vim: set noet: */ diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index c7dbe1e..b3de1b0 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -57,7 +57,7 @@ static alpm_depmissing_t *depmiss_new(const char *target, alpm_depend_t *dep, return miss; } -void _alpm_depmiss_free(alpm_depmissing_t *miss) +void SYMEXPORT alpm_depmissing_free(alpm_depmissing_t *miss) { _alpm_dep_free(miss->depend); FREE(miss->target); @@ -804,7 +804,7 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, /* check if one of the packages in the [*packages] list already satisfies * this dependency */ if(find_dep_satisfier(*packages, missdep)) { - _alpm_depmiss_free(miss); + alpm_depmissing_free(miss); continue; } /* check if one of the packages in the [preferred] list already satisfies @@ -818,9 +818,9 @@ int _alpm_resolvedeps(alpm_handle_t *handle, alpm_list_t *localpkgs, _alpm_log(handle, ALPM_LOG_DEBUG, "pulling dependency %s (needed by %s)\n", spkg->name, pkg->name); - _alpm_depmiss_free(miss); + alpm_depmissing_free(miss); } else if(resolvedep(handle, missdep, (targ = alpm_list_add(NULL, handle->db_local)), rem, 0)) { - _alpm_depmiss_free(miss); + alpm_depmissing_free(miss); } else { handle->pm_errno = ALPM_ERR_UNSATISFIED_DEPS; char *missdepstring = alpm_dep_compute_string(missdep); diff --git a/lib/libalpm/deps.h b/lib/libalpm/deps.h index 8a8ff46..e36bbb3 100644 --- a/lib/libalpm/deps.h +++ b/lib/libalpm/deps.h @@ -29,7 +29,6 @@ void _alpm_dep_free(alpm_depend_t *dep); alpm_depend_t *_alpm_dep_dup(const alpm_depend_t *dep); -void _alpm_depmiss_free(alpm_depmissing_t *miss); alpm_list_t *_alpm_sortbydeps(alpm_handle_t *handle, alpm_list_t *targets, alpm_list_t *ignore, int reverse); int _alpm_recursedeps(alpm_db_t *db, alpm_list_t **targs, int include_explicit); diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 0afca24..6fb82ee 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -116,7 +116,7 @@ static int remove_prepare_cascade(alpm_handle_t *handle, alpm_list_t *lp) _("could not find %s in database -- skipping\n"), miss->target); } } - alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free); + alpm_list_free_inner(lp, (alpm_list_fn_free)alpm_depmissing_free); alpm_list_free(lp); lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(handle->db_local), trans->remove, NULL, 1); @@ -153,7 +153,7 @@ static void remove_prepare_keep_needed(alpm_handle_t *handle, alpm_list_t *lp) _alpm_pkg_free(pkg); } } - alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free); + alpm_list_free_inner(lp, (alpm_list_fn_free)alpm_depmissing_free); alpm_list_free(lp); lp = alpm_checkdeps(handle, _alpm_db_get_pkgcache(handle->db_local), trans->remove, NULL, 1); @@ -232,7 +232,8 @@ int _alpm_remove_prepare(alpm_handle_t *handle, alpm_list_t **data) if(data) { *data = lp; } else { - alpm_list_free_inner(lp, (alpm_list_fn_free)_alpm_depmiss_free); + alpm_list_free_inner(lp, + (alpm_list_fn_free)alpm_depmissing_free); alpm_list_free(lp); } RET_ERR(handle, ALPM_ERR_UNSATISFIED_DEPS, -1); diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index cab4c04..c6c50a4 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -451,7 +451,8 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) dependency-reordered list below */ handle->pm_errno = 0; if(data) { - alpm_list_free_inner(*data, (alpm_list_fn_free)_alpm_depmiss_free); + alpm_list_free_inner(*data, + (alpm_list_fn_free)alpm_depmissing_free); alpm_list_free(*data); *data = NULL; } @@ -528,7 +529,7 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) *data = alpm_list_add(*data, newconflict); } } - alpm_list_free_inner(deps, (alpm_list_fn_free)_alpm_conflict_free); + alpm_list_free_inner(deps, (alpm_list_fn_free)alpm_conflict_free); alpm_list_free(deps); _alpm_dep_free(dep1); _alpm_dep_free(dep2); @@ -546,7 +547,7 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) trans->unresolvable = alpm_list_add(trans->unresolvable, rsync); } - alpm_list_free_inner(deps, (alpm_list_fn_free)_alpm_conflict_free); + alpm_list_free_inner(deps, (alpm_list_fn_free)alpm_conflict_free); alpm_list_free(deps); deps = NULL; @@ -592,13 +593,13 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) *data = alpm_list_add(*data, newconflict); } } - alpm_list_free_inner(deps, (alpm_list_fn_free)_alpm_conflict_free); + alpm_list_free_inner(deps, (alpm_list_fn_free)alpm_conflict_free); alpm_list_free(deps); goto cleanup; } } EVENT(handle, ALPM_EVENT_INTERCONFLICTS_DONE, NULL, NULL); - alpm_list_free_inner(deps, (alpm_list_fn_free)_alpm_conflict_free); + alpm_list_free_inner(deps, (alpm_list_fn_free)alpm_conflict_free); alpm_list_free(deps); } @@ -628,7 +629,8 @@ int _alpm_sync_prepare(alpm_handle_t *handle, alpm_list_t **data) if(data) { *data = deps; } else { - alpm_list_free_inner(deps, (alpm_list_fn_free)_alpm_depmiss_free); + alpm_list_free_inner(deps, + (alpm_list_fn_free)alpm_depmissing_free); alpm_list_free(deps); } goto cleanup; @@ -1274,7 +1276,7 @@ int _alpm_sync_commit(alpm_handle_t *handle, alpm_list_t **data) if(data) { *data = conflict; } else { - alpm_list_free_inner(conflict, (alpm_list_fn_free)_alpm_fileconflict_free); + alpm_list_free_inner(conflict, (alpm_list_fn_free)alpm_fileconflict_free); alpm_list_free(conflict); } RET_ERR(handle, ALPM_ERR_FILE_CONFLICTS, -1); diff --git a/src/pacman/remove.c b/src/pacman/remove.c index 56b9333..00bd3b3 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -121,12 +121,13 @@ int pacman_remove(alpm_list_t *targets) char *depstring = alpm_dep_compute_string(miss->depend); colon_printf(_("%s: requires %s\n"), miss->target, depstring); free(depstring); + alpm_depmissing_free(miss); } break; default: break; } - FREELIST(data); + alpm_list_free(data); retval = 1; goto cleanup; } diff --git a/src/pacman/sync.c b/src/pacman/sync.c index f6ee57a..796dd0b 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -763,8 +763,9 @@ int sync_prepare_execute(void) switch(err) { case ALPM_ERR_PKG_INVALID_ARCH: for(i = data; i; i = alpm_list_next(i)) { - const char *pkg = i->data; + char *pkg = i->data; colon_printf(_("package %s does not have a valid architecture\n"), pkg); + free(pkg); } break; case ALPM_ERR_UNSATISFIED_DEPS: @@ -773,6 +774,7 @@ int sync_prepare_execute(void) char *depstring = alpm_dep_compute_string(miss->depend); colon_printf(_("%s: requires %s\n"), miss->target, depstring); free(depstring); + alpm_depmissing_free(miss); } break; case ALPM_ERR_CONFLICTING_DEPS: @@ -788,6 +790,7 @@ int sync_prepare_execute(void) conflict->package1, conflict->package2, reason); free(reason); } + alpm_conflict_free(conflict); } break; default: @@ -847,6 +850,7 @@ int sync_prepare_execute(void) conflict->target, conflict->file); break; } + alpm_fileconflict_free(conflict); } break; case ALPM_ERR_PKG_INVALID: @@ -854,8 +858,9 @@ int sync_prepare_execute(void) case ALPM_ERR_PKG_INVALID_SIG: case ALPM_ERR_DLT_INVALID: for(i = data; i; i = alpm_list_next(i)) { - const char *filename = i->data; + char *filename = i->data; printf(_("%s is invalid or corrupted\n"), filename); + free(filename); } break; default: @@ -869,9 +874,7 @@ int sync_prepare_execute(void) /* Step 4: release transaction resources */ cleanup: - if(data) { - FREELIST(data); - } + alpm_list_free(data); if(trans_release() == -1) { retval = 1; } -- 1.8.5.3
On 07/01/14 02:52, Andrew Gregory wrote:
If an unresolvable package was loaded from a file it would be free'd again when freeing trans->add.
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
This patch I do not understand. Where does anything get added to trans->unresolvable that is not removed from trans->add?
lib/libalpm/trans.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index ada8100..4930e94 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -246,12 +246,19 @@ int SYMEXPORT alpm_trans_release(alpm_handle_t *handle)
void _alpm_trans_free(alpm_trans_t *trans) { + alpm_list_t *i; + if(trans == NULL) { return; }
- alpm_list_free_inner(trans->unresolvable, - (alpm_list_fn_free)_alpm_pkg_free_trans); + /* only free packages not in trans->add to avoid + * double free'ing packages loaded from files. */ + for(i = trans->unresolvable; i; i = i->next) { + if(!alpm_list_find_ptr(trans->add, i->data)) { + _alpm_pkg_free_trans(i->data); + } + } alpm_list_free(trans->unresolvable); alpm_list_free_inner(trans->add, (alpm_list_fn_free)_alpm_pkg_free_trans); alpm_list_free(trans->add);
participants (2)
-
Allan McRae
-
Andrew Gregory