[pacman-dev] [PATCH 2/8] _alpm_sync_prepare: fix unresolvable packages leak

[Allan McRae]

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 at 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