[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
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
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
--- 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
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
--- 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
alpm_pkg_compute_optional returns a generated list that needs to be
free'd.
Signed-off-by: Andrew Gregory
_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
Signed-off-by: Andrew Gregory
Signed-off-by: Andrew Gregory
Front-ends should be able to free memory that alpm hands them.
Signed-off-by: Andrew Gregory
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
--- 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
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
---
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