Re: [pacman-dev] [PATCH] libalpm: set ret in download files

[Morgan Adamiec]

On 30/11/2020 7:02 pm, Anatol Pomozov wrote:
> 
> "if(ret)" is semantically equivalent to "if(ret != 0)". Is the change
> really needed here?
> 

I don't particularity care what format is used, just as long as they're
all the same.

Comparing against 0 was already used twice in that file so just thought
I'd make that one do it too.

Re: [pacman-dev] [PATCH] libalpm: set ret in download files

[Anatol Pomozov]

Hi

On Mon, Nov 30, 2020 at 10:21 AM morganamilo  wrote:
>
> download_files never set ret on failiure, so even when downloading
> fails, the transaction goes on to commit and error out.
>
> :: Retrieving packages...
>  python-packaging-20.4-4-any.pkg.tar.zst failed to download
> error: failed retrieving file 'python-packaging-20.4-4-any.pkg.tar.zst' from 
> mirror.oldsql.cc : The requested URL returned error: 404
> warning: failed to retrieve some files
> (1/1) checking keys in keyring
> (1/1) checking package integrity
> error: failed to commit transaction (wrong or NULL argument passed)
> Errors occurred, no packages were upgraded.
>
> Also make the ret checking more consistent.
>
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 601f1d69..5d8652a5 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -769,7 +769,7 @@ static int download_files(alpm_handle_t *handle)
> }
>
> ret = find_dl_candidates(handle, );
> -   if(ret) {
> +   if(ret != 0) {
> goto finish;
> }

"if(ret)" is semantically equivalent to "if(ret != 0)". Is the change
really needed here?

>
> @@ -818,7 +818,9 @@ static int download_files(alpm_handle_t *handle)
>
> payloads = alpm_list_add(payloads, payload);
> }
> -   if(_alpm_download(handle, payloads, cachedir) == -1) {
> +
> +   ret = _alpm_download(handle, payloads, cachedir);
> +   if(ret != 0) {

This part LGTM.

[pacman-dev] [PATCH] libalpm: set ret in download files

[morganamilo]

download_files never set ret on failiure, so even when downloading
fails, the transaction goes on to commit and error out.

:: Retrieving packages...
 python-packaging-20.4-4-any.pkg.tar.zst failed to download
error: failed retrieving file 'python-packaging-20.4-4-any.pkg.tar.zst' from 
mirror.oldsql.cc : The requested URL returned error: 404
warning: failed to retrieve some files
(1/1) checking keys in keyring
(1/1) checking package integrity
error: failed to commit transaction (wrong or NULL argument passed)
Errors occurred, no packages were upgraded.

Also make the ret checking more consistent.

diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index 601f1d69..5d8652a5 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -769,7 +769,7 @@ static int download_files(alpm_handle_t *handle)
}
 
ret = find_dl_candidates(handle, );
-   if(ret) {
+   if(ret != 0) {
goto finish;
}
 
@@ -818,7 +818,9 @@ static int download_files(alpm_handle_t *handle)
 
payloads = alpm_list_add(payloads, payload);
}
-   if(_alpm_download(handle, payloads, cachedir) == -1) {
+
+   ret = _alpm_download(handle, payloads, cachedir);
+   if(ret != 0) {
event.type = ALPM_EVENT_PKG_RETRIEVE_FAILED;
EVENT(handle, );
_alpm_log(handle, ALPM_LOG_WARNING, _("failed to 
retrieve some files\n"));
-- 
2.29.2