On Fri, Oct 14, 2016 at 09:26:46PM +0200, mar77i wrote: > In FS#43434, Downloads which fail and are restarted on a different server > will resume and may display a negative download speed. The payload's progress > in libalpm was not properly reset which ultimately caused terminal noise > because the line width calculation assumes positive download speeds. > > This patch fixes the incomplete reset of the payload by mimicing what > be_sync.c:alpm_db_update() does over in sync.c:download_single_file(). > dload.c:_alpm_dload_payload_reset() bundles and extends over the current > behavior by updating initial_size and prevprogress for this case. > This makes pacman reset the progress properly in the next invocation of the > callback and display positive download speeds. > > Fixes FS#43434. > > Signed-off-by: Martin Kühne <mysat...@gmail.com> > --- > lib/libalpm/be_sync.c | 4 ++-- > lib/libalpm/dload.c | 15 +++++++++++---- > lib/libalpm/dload.h | 2 +- > lib/libalpm/sync.c | 4 +--- > 4 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c > index a836277..2425359 100644 > --- a/lib/libalpm/be_sync.c > +++ b/lib/libalpm/be_sync.c > @@ -244,7 +244,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) > payload.unlink_on_fail = 1; > > ret = _alpm_download(&payload, syncpath, NULL, &final_db_url); > - _alpm_dload_payload_reset(&payload); > + _alpm_dload_payload_reset(&payload, 0); > updated = (updated || ret == 0); > > if(ret != -1 && updated && (level & ALPM_SIG_DATABASE)) { > @@ -300,7 +300,7 @@ int SYMEXPORT alpm_db_update(int force, alpm_db_t *db) > sig_ret = _alpm_download(&payload, syncpath, NULL, > NULL); > /* errors_ok suppresses error messages, but not the > return code */ > sig_ret = payload.errors_ok ? 0 : sig_ret; > - _alpm_dload_payload_reset(&payload); > + _alpm_dload_payload_reset(&payload, 0);
This is kind of opaque at a glance. What does 0 mean? Potential modifications: 1) Add an inline comment: _alpm_dload_payload_reset(&payload, 0 /* do not allow resume */); 2) Separate this out into 2 wrappers: _alpm_dload_payload_reset_for_retry(&payload); _alpm_dload_payload_reset(&payload); _alpm_dload_payload_reset_internal(&payload, 0|1); I don't feel too strongly about this, though, since this sort of opaqueness is evident elsewhere in the codebase. > } > > if(ret != -1 && sig_ret != -1) { > diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c > index dc57c92..55ff41f 100644 > --- a/lib/libalpm/dload.c > +++ b/lib/libalpm/dload.c > @@ -697,7 +697,7 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, > const char *url) > > /* download the file */ > ret = _alpm_download(&payload, cachedir, &final_file, > &final_pkg_url); > - _alpm_dload_payload_reset(&payload); > + _alpm_dload_payload_reset(&payload, 0); > if(ret == -1) { > _alpm_log(handle, ALPM_LOG_WARNING, _("failed to > download %s\n"), url); > free(final_file); > @@ -738,7 +738,7 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, > const char *url) > FREE(sig_final_file); > } > free(sig_filepath); > - _alpm_dload_payload_reset(&payload); > + _alpm_dload_payload_reset(&payload, 0); > } > > /* we should be able to find the file the second time around */ > @@ -750,15 +750,22 @@ char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t > *handle, const char *url) > return filepath; > } > > -void _alpm_dload_payload_reset(struct dload_payload *payload) > +void _alpm_dload_payload_reset(struct dload_payload *payload, int > allow_resume) > { > ASSERT(payload, return); > > + FREE(payload->fileurl); > + if(allow_resume) { > + payload->initial_size += payload->prevprogress; > + payload->prevprogress = 0; > + payload->unlink_on_fail = 0; > + return; > + } > + > FREE(payload->remote_name); > FREE(payload->tempfile_name); > FREE(payload->destfile_name); > FREE(payload->content_disp_name); > - FREE(payload->fileurl); > memset(payload, '\0', sizeof(*payload)); > } > > diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h > index 427c486..c9c94b8 100644 > --- a/lib/libalpm/dload.h > +++ b/lib/libalpm/dload.h > @@ -46,7 +46,7 @@ struct dload_payload { > #endif > }; > > -void _alpm_dload_payload_reset(struct dload_payload *payload); > +void _alpm_dload_payload_reset(struct dload_payload *payload, int > allow_resume); > > int _alpm_download(struct dload_payload *payload, const char *localpath, > char **final_file, const char **final_url); > diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c > index 00b68d0..81900df 100644 > --- a/lib/libalpm/sync.c > +++ b/lib/libalpm/sync.c > @@ -946,9 +946,7 @@ static int download_single_file(alpm_handle_t *handle, > struct dload_payload *pay > EVENT(handle, &event); > return 0; > } > - > - FREE(payload->fileurl); > - payload->unlink_on_fail = 0; > + _alpm_dload_payload_reset(payload, 1); > } > > event.type = ALPM_EVENT_PKGDOWNLOAD_FAILED; > -- > 2.10.0