diff mbox

[pacman-dev] dload: ensure callback is always initialized once

Message ID 20171216174111.4641-2-andrew.gregory.8@gmail.com
State Accepted, archived
Headers show

Commit Message

Andrew Gregory Dec. 16, 2017, 5:41 p.m. UTC 
Frontends rely on an initialization call for setup between downloads.
Checking for intialization after checking for a completed download can
skip initialization in cases where files are small enough to be
downloaded all at once (FS#56408).  Relying on previous download size
can result in multiple initializations if there are multiple
non-transfer events prior to the download starting (fS#56468).

Introduce a new cb_initialized variable to the payload struct and use it
to ensure that the callback is initialized exactly once prior to any
actual events.

Fixes FS#56408, FS#56468

Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com>
---
 lib/libalpm/dload.c | 9 +++++----
 lib/libalpm/dload.h | 1 +
 2 files changed, 6 insertions(+), 4 deletions(-)

Comments

Allan McRae Jan. 6, 2018, 2:59 a.m. UTC | #1 
On 17/12/17 03:41, Andrew Gregory wrote:
> Frontends rely on an initialization call for setup between downloads.
> Checking for intialization after checking for a completed download can
> skip initialization in cases where files are small enough to be
> downloaded all at once (FS#56408).  Relying on previous download size
> can result in multiple initializations if there are multiple
> non-transfer events prior to the download starting (fS#56468).
> 
> Introduce a new cb_initialized variable to the payload struct and use it
> to ensure that the callback is initialized exactly once prior to any
> actual events.
> 
> Fixes FS#56408, FS#56468
> 
> Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com>

Great!  Probably fixes a much older version of FS#56468 too (I think we
blamed that on ftp...).

A
diff mbox

Patch

diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 875b689c..44db5f88 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -132,13 +132,13 @@  static int dload_progress_cb(void *file, curl_off_t dltotal, curl_off_t dlnow,
 	 * 0, -1: download initialized
 	 * 0, 0: non-download event
 	 * x {x>0}, x: download complete
 	 * x {x>0, x<y}, y {y > 0}: download progress, expected total is known */
-	if(current_size == total_size) {
-		payload->handle->dlcb(payload->remote_name, dlnow, dltotal);
-	} else if(!payload->prevprogress) {
+	if(!payload->cb_initialized) {
 		payload->handle->dlcb(payload->remote_name, 0, -1);
-	} else if(payload->prevprogress == current_size) {
+		payload->cb_initialized = 1;
+	}
+	if(payload->prevprogress == current_size) {
 		payload->handle->dlcb(payload->remote_name, 0, 0);
 	} else {
 	/* do NOT include initial_size since it wasn't part of the package's
 	 * download_size (nor included in the total download size callback) */
@@ -731,7 +731,8 @@  void _alpm_dload_payload_reset_for_retry(struct dload_payload *payload)
 	FREE(payload->fileurl);
 	payload->initial_size += payload->prevprogress;
 	payload->prevprogress = 0;
 	payload->unlink_on_fail = 0;
+	payload->cb_initialized = 0;
 }
 
 /* vim: set noet: */
diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h
index 6ca775a7..ac948528 100644
--- a/lib/libalpm/dload.h
+++ b/lib/libalpm/dload.h
@@ -40,8 +40,9 @@  struct dload_payload {
 	int allow_resume;
 	int errors_ok;
 	int unlink_on_fail;
 	int trust_remote_name;
+	int cb_initialized;
 #ifdef HAVE_LIBCURL
 	CURLcode curlerr;       /* last error produced by curl */
 #endif
 };