[pacman-dev] [PATCH 2/2] ensure package download sizes are invalidated when an error occurs

[Nagy Gabor]

> Signed-off-by: Jonathan Conder <j at skurvy.no-ip.org>
> ---
>  lib/libalpm/sync.c |   16 +++++++---------
>  1 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index d9f4c28..9e140dc 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -855,9 +855,7 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
>  
>  		if(files) {
>  			EVENT(trans, PM_TRANS_EVT_RETRIEVE_START, current->treename, NULL);
> -			errors = _alpm_download_files(files, current->servers, cachedir);
> -
> -			if (errors) {
> +			if(_alpm_download_files(files, current->servers, cachedir)) {
>  				_alpm_log(PM_LOG_WARNING, _("failed to retrieve some files from %s\n"),
>  						current->treename);
>  				if(pm_errno == 0) {
> @@ -869,12 +867,6 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
>  		}
>  	}
>  
> -	for(j = trans->add; j; j = j->next) {
> -		pmpkg_t *pkg = j->data;
> -		pkg->infolevel &= ~INFRQ_DSIZE;
> -		pkg->download_size = 0;
> -	}
> -
>  	/* clear out value to let callback know we are done */
>  	if(handle->totaldlcb) {
>  		handle->totaldlcb(0);
> @@ -1003,6 +995,12 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
>  	ret = 0;
>  
>  error:
> +	for(j = trans->add; j; j = j->next) {
> +		pmpkg_t *pkg = j->data;
> +		pkg->infolevel &= ~INFRQ_DSIZE;
> +		pkg->download_size = 0;
> +	}
> +
>  	FREELIST(files);
>  	alpm_list_free(deltas);
>  	return(ret);


Khm, lol, when I wrote the last patch, I was too lazy and I simply
overlooked this part. I "thought" that this code-part sets download_size
to 0, but it _unsets_ it instead (see also my commit message).

The spirit of this patch is better, but it is still not perfect, because
of the -Sw (PM_TRANS_FLAG_DOWNLOADONLY) case. That return(0) can also be
changed to goto error, and I can see one more RET_ERR in the function.

I just note that in case of error-free -S commit operation, this
code-part is needless/overkill, because the target pkgcache packages are
replaced in the integrity-check part by the pmpkg_t of pkg.tar.xz file
(line 949), and then every download_size is either 0 or unset.

An other side note:
I have some bad feelings when I see many foo->download_size in this
codepart. We have a good access function for this. I know that atm all
download_sizes are filled in correctly at the end of sync_prepare, but
that may be changed later. But this direct access causes some nanosec
speedup, so I dunno. :-)

NG