| Message ID | 20201130182126.902723-1-morganamilo@archlinux.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [pacman-dev] libalpm: set ret in download files | expand |
Hi On Mon, Nov 30, 2020 at 10:21 AM morganamilo <morganamilo@archlinux.org> 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, &files); > - 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.
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.
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, &files); - 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, &event); _alpm_log(handle, ALPM_LOG_WARNING, _("failed to retrieve some files\n"));