[pacman-dev,2/5] libalpm: don't call dlcb when not set

Message ID 20201124123908.2096891-2-morganamilo@archlinux.org
State Accepted, archived
Headers show
Series [pacman-dev,1/5] libalpm: set parallel_downloads to 1 when creating the handle | expand

Commit Message

morganamilo Nov. 24, 2020, 12:39 p.m. UTC
Fixes FS#68728:

Comments

Allan McRae Nov. 26, 2020, 6:32 a.m. UTC | #1
On 24/11/20 10:39 pm, morganamilo wrote:
> Fixes FS#68728:
> 

Thanks - I had a quick skim elsewhere and looks like this covers the
issue fully.


> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 673e769f..d43e6d45 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -586,7 +586,7 @@ cleanup:
>  		unlink(payload->tempfile_name);
>  	}
>  
> -	if(!payload->signature) {
> +	if(handle->dlcb && !payload->signature) {
>  		alpm_download_event_completed_t cb_data = {0};
>  		cb_data.total = bytes_dl;
>  		cb_data.result = ret;
> @@ -719,7 +719,7 @@ static int curl_download_internal(alpm_handle_t *handle,
>  			struct dload_payload *payload = payloads->data;
>  
>  			if(curl_add_payload(handle, curlm, payload, localpath) == 0) {
> -				if(!payload->signature) {
> +				if(handle->dlcb && !payload->signature) {
>  					alpm_download_event_init_t cb_data = {.optional = payload->errors_ok};
>  					handle->dlcb(payload->remote_name, ALPM_DOWNLOAD_INIT, &cb_data);
>  				}
>
Anatol Pomozov Nov. 28, 2020, 3:57 a.m. UTC | #2
Hi

On Wed, Nov 25, 2020 at 10:32 PM Allan McRae <allan@archlinux.org> wrote:
>
> On 24/11/20 10:39 pm, morganamilo wrote:
> > Fixes FS#68728:
> >
>
> Thanks - I had a quick skim elsewhere and looks like this covers the
> issue fully.
>
>
> > diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> > index 673e769f..d43e6d45 100644
> > --- a/lib/libalpm/dload.c
> > +++ b/lib/libalpm/dload.c
> > @@ -586,7 +586,7 @@ cleanup:
> >               unlink(payload->tempfile_name);
> >       }
> >
> > -     if(!payload->signature) {
> > +     if(handle->dlcb && !payload->signature) {
> >               alpm_download_event_completed_t cb_data = {0};
> >               cb_data.total = bytes_dl;
> >               cb_data.result = ret;
> > @@ -719,7 +719,7 @@ static int curl_download_internal(alpm_handle_t *handle,
> >                       struct dload_payload *payload = payloads->data;
> >
> >                       if(curl_add_payload(handle, curlm, payload, localpath) == 0) {
> > -                             if(!payload->signature) {
> > +                             if(handle->dlcb && !payload->signature) {
> >                                       alpm_download_event_init_t cb_data = {.optional = payload->errors_ok};
> >                                       handle->dlcb(payload->remote_name, ALPM_DOWNLOAD_INIT, &cb_data);
> >                               }
> >

The patch LGTM.

And for the record, the third place where we use handle->dlcb
(lib/libalpm/dload.c around line 137) already has a guard:

        if(payload->handle->dlcb == NULL) {
                return 0;
        }

Patch

diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 673e769f..d43e6d45 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -586,7 +586,7 @@  cleanup:
 		unlink(payload->tempfile_name);
 	}
 
-	if(!payload->signature) {
+	if(handle->dlcb && !payload->signature) {
 		alpm_download_event_completed_t cb_data = {0};
 		cb_data.total = bytes_dl;
 		cb_data.result = ret;
@@ -719,7 +719,7 @@  static int curl_download_internal(alpm_handle_t *handle,
 			struct dload_payload *payload = payloads->data;
 
 			if(curl_add_payload(handle, curlm, payload, localpath) == 0) {
-				if(!payload->signature) {
+				if(handle->dlcb && !payload->signature) {
 					alpm_download_event_init_t cb_data = {.optional = payload->errors_ok};
 					handle->dlcb(payload->remote_name, ALPM_DOWNLOAD_INIT, &cb_data);
 				}