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 <mysatyre@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