diff mbox series

channel_curl: Improve handling of tracking download progress

Message ID 20201211133410.83866-1-sava.jakovljev@teufel.de
State Changes Requested
Headers show
Series channel_curl: Improve handling of tracking download progress | expand

Commit Message

Sava Jakovljev Dec. 11, 2020, 1:34 p.m. UTC
Signed-off-by: Sava Jakovljev <sava.jakovljev@teufel.de>
---
When a server doesn't insert Content-Length header, there is no sense in 
enabling CURL option XFERINFUNCTION//PROGRESSFUNCTION. 
This change also solves a bug that currently exists, where an address of 
a local variable 'percent' is passed to libcurl, which in turn gives it as
an argument to a callback specified with CURLOPT_XFERINFOFUNCTION.
Also, for some reason if version is high enough, both XFERINFOFUNCTION and PROGRESSFUNCTION
are set. This change uses just one using preprocessor macros.
Download tracking is enabled only for get_file method and is disabled if total download size
cannot be determined.
This change also cleans up a code a little bit.

Best regards,
Sava Jakovljev

 corelib/channel_curl.c | 131 ++++++++++++++++++++++++++++++++---------
 1 file changed, 102 insertions(+), 29 deletions(-)

--
2.26.2
diff mbox series

Patch

diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c
index a3e785f..e6625ab 100644
--- a/corelib/channel_curl.c
+++ b/corelib/channel_curl.c
@@ -55,6 +55,13 @@  typedef struct {
 	output_data_t *outdata;
 } write_callback_t;

+typedef struct {
+	channel_curl_t *owner;
+	const char *url;
+	curl_off_t total_download_size;
+	uint8_t percent;
+} download_callback_data_t;
+

 /* Prototypes for "internal" functions */
 /* Note that they're not `static` so that they're callable from unit tests. */
@@ -414,15 +421,21 @@  static int channel_callback_xferinfo(void *p, curl_off_t dltotal, curl_off_t dln
 				     curl_off_t __attribute__((__unused__)) ultotal,
 				     curl_off_t __attribute__((__unused__)) ulnow)
 {
-	if ((dltotal <= 0) || (dlnow > dltotal)) {
+	download_callback_data_t *data = (download_callback_data_t*)p;
+
+	if (data->total_download_size != dltotal)
 		return 0;
-	}
-	double percent = 100.0 * (dlnow/1024.0) / (dltotal/1024.0);
-	double *last_percent = (double*)p;
-	if ((int)*last_percent == (int)percent) {
+	if ((dltotal <= 0) || (dlnow > dltotal))
 		return 0;
-	}
-	*last_percent = percent;
+
+	uint8_t percent = 100.0 * ((double)dlnow / dltotal) + 0.5;
+
+	if (data->percent == percent)
+		return 0;
+	else
+		data->percent = percent;
+
+	TRACE("Download %d% (%lu of %lu kB).", percent, dlnow, dltotal);
 	swupdate_download_update(percent, dltotal);
 	return 0;
 }
@@ -566,28 +579,6 @@  channel_op_res_t channel_set_options(channel_t *this, channel_data_t *channel_da
 		goto cleanup;
 	}

-	double percent = -INFINITY;
-	if ((curl_easy_setopt(channel_curl->handle, CURLOPT_PROGRESSFUNCTION,
-			      channel_callback_xferinfo_legacy) != CURLE_OK) ||
-	    (curl_easy_setopt(channel_curl->handle, CURLOPT_PROGRESSDATA,
-				  &percent) != CURLE_OK)) {
-		result = CHANNEL_EINIT;
-		goto cleanup;
-	}
-#if LIBCURL_VERSION_NUM >= 0x072000
-	if ((curl_easy_setopt(channel_curl->handle, CURLOPT_XFERINFOFUNCTION,
-			      channel_callback_xferinfo) != CURLE_OK) ||
-	    (curl_easy_setopt(channel_curl->handle, CURLOPT_XFERINFODATA,
-				  &percent) != CURLE_OK)) {
-		result = CHANNEL_EINIT;
-		goto cleanup;
-	}
-#endif
-	if (curl_easy_setopt(channel_curl->handle, CURLOPT_NOPROGRESS, 0L) != CURLE_OK) {
-		result = CHANNEL_EINIT;
-		goto cleanup;
-	}
-
 	if (channel_data->headers) {
 		/*
 		 * Setup supply request and receive reply HTTP headers.
@@ -731,6 +722,78 @@  cleanup:
 	return result;
 }

+static curl_off_t channel_get_total_download_size(channel_curl_t *this,
+		const char *url)
+{
+	assert(this != NULL);
+	assert(url  != NULL);
+
+	if (curl_easy_setopt(this->handle, CURLOPT_URL, url) != CURLE_OK ||
+		curl_easy_setopt(this->handle, CURLOPT_NOBODY, 1L) != CURLE_OK ||
+		curl_easy_perform(this->handle) != CURLE_OK)
+		goto cleanup;
+
+	curl_off_t size = -1;
+	if (curl_easy_getinfo(this->handle, CURLINFO_CONTENT_LENGTH_DOWNLOAD_T,
+			&size) != CURLE_OK)
+		goto cleanup;
+
+cleanup:
+	curl_easy_setopt(this->handle, CURLOPT_NOBODY, 0L);
+	return size;
+}
+
+static channel_op_res_t channel_disable_download_progress_tracking(
+		channel_curl_t *this)
+{
+	curl_easy_setopt(this->handle, CURLOPT_NOPROGRESS, 1L) != CURLE_OK ?
+		CHANNEL_EINIT : CHANNEL_OK;
+}
+
+static channel_op_res_t channel_enable_download_progress_tracking(
+		channel_curl_t *this,
+		const char *url,
+		download_callback_data_t *download_data)
+{
+	assert(url != NULL);
+	assert(download_data != NULL);
+
+	download_data->url = url;
+	download_data->owner = this;
+	download_data->percent = 0;
+
+	channel_op_res_t result = CHANNEL_OK;
+	if ((download_data->total_download_size = channel_get_total_download_size(
+				this, url)) <= 0) {
+		result = CHANNEL_EINIT;
+		goto cleanup;
+	}
+
+#if LIBCURL_VERSION_NUM >= 0x072000
+	if ((curl_easy_setopt(this->handle, CURLOPT_XFERINFOFUNCTION,
+			      channel_callback_xferinfo) != CURLE_OK) ||
+	    (curl_easy_setopt(this->handle, CURLOPT_XFERINFODATA,
+				  download_data) != CURLE_OK)) {
+		result = CHANNEL_EINIT;
+		goto cleanup;
+	}
+#else
+	if ((curl_easy_setopt(this->handle, CURLOPT_PROGRESSFUNCTION,
+			      channel_callback_xferinfo_legacy) != CURLE_OK) ||
+	    (curl_easy_setopt(this->handle, CURLOPT_PROGRESSDATA,
+				  download_data) != CURLE_OK)) {
+		result = CHANNEL_EINIT;
+		goto cleanup;
+	}
+#endif
+	if (curl_easy_setopt(this->handle, CURLOPT_NOPROGRESS, 0L) != CURLE_OK) {
+		result = CHANNEL_EINIT;
+		goto cleanup;
+	}
+cleanup:
+	return result;
+}
+
 static size_t put_read_callback(void *ptr, size_t size, size_t nmemb, void *data)
 {
 	channel_data_t *channel_data = (channel_data_t *)data;
@@ -1030,6 +1093,16 @@  channel_op_res_t channel_get_file(channel_t *this, void *data)
 		goto cleanup_header;
 	}

+	download_callback_data_t download_data;
+	if (channel_enable_download_progress_tracking(channel_curl,
+				channel_data->url,
+				&download_data) == CHANNEL_EINIT) {
+		WARN("Failed to get total download size for URL %s.",
+				channel_data->url);
+	} else
+		INFO("Total download size is %lu kB.",
+				download_data.total_download_size / 1024.0);
+
 	if (curl_easy_setopt(channel_curl->handle, CURLOPT_CUSTOMREQUEST, "GET") !=
 	    CURLE_OK) {
 		ERROR("Set GET channel method option failed.");