From patchwork Thu Jan 21 13:37:07 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sava Jakovljev X-Patchwork-Id: 1429849 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=googlegroups.com (client-ip=2a00:1450:4864:20::23b; helo=mail-lj1-x23b.google.com; envelope-from=swupdate+bncbaabbguhu2aamgqeqflo76y@googlegroups.com; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=googlegroups.com header.i=@googlegroups.com header.a=rsa-sha256 header.s=20161025 header.b=GVBAqEmo; dkim-atps=neutral Received: from mail-lj1-x23b.google.com (mail-lj1-x23b.google.com [IPv6:2a00:1450:4864:20::23b]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DM3Nd5wyxz9sW4 for ; Fri, 22 Jan 2021 00:37:35 +1100 (AEDT) Received: by mail-lj1-x23b.google.com with SMTP id z8sf890868lji.8 for ; Thu, 21 Jan 2021 05:37:35 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1611236251; cv=pass; d=google.com; s=arc-20160816; b=cOFBYQX3qUhC2HvcJy/rT0SjF9P/aB75ndbZcZeRzktOpFaxoc0++JiZiti8jMa9ba +XRnNlwqo7JG5PjDVG7zHALZSQPSkJkCWLpQP5YHPbi8b6n+q4c1S6hqxx5cYYyN+Pyf GKNjPqy5c1ksuTvR6iItA2JqYux+uBO7w13M9zRGkaoQ0PQIXDwXL//IWIEGnEXgT5sJ 7jo/HEkSbsyIlaXoHtWWxfna0RrkNj9r3X3JbC6zkTf9ETFu666iyKGhMpPREGkdbzGS re73AhJfRHLn+8OEnDM1a+FZ+aVacXZWXcqCUwjv1DspBwflKJetPm3xT42ELF+Tqh63 xtfw== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-unsubscribe:list-subscribe:list-archive:list-help:list-post :list-id:mailing-list:precedence:mime-version:message-id:date :subject:cc:to:from:sender:dkim-signature; bh=EAMrDqiSl1LeAj6YafV0N2AA3OTXf3jF8ZCgfXoYR5g=; b=tTpUqUXcrjClRIBdbjf5+PfliCCatZ0sPHyLs5VU7cfekYrxQcl1EWxnxUsftjN2aJ 5DcoUkjtvBnv+iKiHMhsXI872QKhfgRuIinG40LF7jU3cnzzMJ0HwhgXR9bPKJmoYdBW ZdwfmdXu5wb17wm/22befqiD2mGYMA2XUdUhLHk+hjgkWXIUQL+MS/3JAZY+cSQ33XZy IItJZwCmujDfka4kSghckfjpISRPxxz3364gZeH9K9C6RtpfEl8AAFutNj+KV+eBK378 ifo3K0SDlYkeeb0ErPJFcnpQPvHVliK0G2ZJGYq/yb1o1MUE/VIqSFIFuR3bkVEk/85/ DdEA== ARC-Authentication-Results: i=2; gmr-mx.google.com; dkim=pass header.i=@teufel.de header.s=hse1 header.b=dKLL2v5b; spf=pass (google.com: domain of sava.jakovljev@teufel.de designates 83.246.65.92 as permitted sender) smtp.mailfrom=sava.jakovljev@teufel.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlegroups.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:mime-version :x-original-sender:x-original-authentication-results:precedence :mailing-list:list-id:list-post:list-help:list-archive :list-subscribe:list-unsubscribe; bh=EAMrDqiSl1LeAj6YafV0N2AA3OTXf3jF8ZCgfXoYR5g=; b=GVBAqEmo1mIaiWwaYvPMuf2m6qhLjEsQmPS8oPnOB17jqiwjyVK8HXKEam14/4AvPX lCJzSPI0/a1jQNOF0U52AynWPxfKCpeOGiFCf9HcQDxdPgHFSs/QBo2HyADC/tFcGs0n +IgS4idFB8HgCV6o7D6pTCfYeo1ZrlxNzJSFl531CcJPRugUkBhWlhkwLfs6yhE+Rmsl E2v4ZiG1/Jr5rZ03us2cqCO0FMNEC44j+/HscCIDo2/ybWdpyBM+RKVQ1U3gTbPSZ3qq /SW3OpN40J3XuLEMXSTSx5maWRUwhLomY5zHdQEXgoEw4ZJfbBy+K/XIu+/lzFdPeptS ZxXg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=sender:x-gm-message-state:from:to:cc:subject:date:message-id :mime-version:x-original-sender:x-original-authentication-results :precedence:mailing-list:list-id:x-spam-checked-in-group:list-post :list-help:list-archive:list-subscribe:list-unsubscribe; bh=EAMrDqiSl1LeAj6YafV0N2AA3OTXf3jF8ZCgfXoYR5g=; b=lHd/6OT53ql0CTP6ytyCu+IlrFgjdkxBydwpCV6YYOjbci7f11FHE9G7A2qNSTh4xX dH7sK0LzYPLymAQiVKIR/Mb61tLCZougSmxp6KyhqzsD13sWvRpB4755k9lBgWOS7IfT MC62RRDc2BkyYWnrIoSogNDaNEYw2m8U4IrD3MqIAV8jG3oQCcr0+2Ndg1br2eSEr/ba P7xPne5e/Rmh0hu3tQ4ocBLPVBfWQnMRGbWNTdkpHlKx7227rB+VGWppMfA+TruZMpOQ euWVkKjsPSEb5rm47ivUHPo6rVvxBKqCEQPuGFyOEo19z73YPI6I5ZotN0sjVW6V9DH3 OggQ== Sender: swupdate@googlegroups.com X-Gm-Message-State: AOAM533g9CR2i1yfXldB63xiENHzUwMtJAphdZAqySGvu56jS9R7bBmg t8OaXMc3pfa8hY0d9CJ5W68= X-Google-Smtp-Source: ABdhPJy3lleR+ROKN+2nYX3zLFEzLZBHRIfiUX9z/Q3x0OTBhzYNx2pSTMe6J39OMkLFvCJilW9+6Q== X-Received: by 2002:a2e:720c:: with SMTP id n12mr7416050ljc.2.1611236251202; Thu, 21 Jan 2021 05:37:31 -0800 (PST) X-BeenThere: swupdate@googlegroups.com Received: by 2002:a05:6512:3993:: with SMTP id j19ls717684lfu.3.gmail; Thu, 21 Jan 2021 05:37:30 -0800 (PST) X-Received: by 2002:ac2:5294:: with SMTP id q20mr6304377lfm.251.1611236250136; Thu, 21 Jan 2021 05:37:30 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1611236250; cv=none; d=google.com; s=arc-20160816; b=wrecMiqo7dtC0uOHYMLUx49hEsItDtbl93Kiwy1lKBVgeI+T8vDzCaI0eSkr/Y4aD2 VK17QCDyUZ9p0kAx3YuseTKNdqZ0ifjNEYFOMggr9vr3zQG2l+02fnX0NX9EpDrHbVXM ZLcNCW7UEw3hbfZr1TRByOr9CRxukXhoxPxAotPUnPh/WVtV+btyaDSwkc20p5RUv/KU /65GbRES8OnsnZYFWwvyA5O6mFqETRfFUknuyZmLFx0phMAY4wQisfHE68cJ2x9t4/X2 AHwBbrnAxC3KyfrZiZBt773Y0m2tZtmOGA94x8MhMABx9gFYFif/be0jp7btegT9YV0b yPaQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=dkim-signature:content-transfer-encoding:mime-version:message-id :date:subject:cc:to:from; bh=jgEhwaL89bTCGzDdlUg3rrNcA2mUhx9+rhnHf3s+SwA=; b=X0PFCjbACOK20UFDzDJjxyYPlI0ejgyC+0AYU1kOMnUxMCLRTbsSF0Dqz7cf8DNP9J 3kTHixpwv5D+lOEW2i2pFa58d71ps42iPufe9gXk0dgaWwUcUqwo9leVkXL5ltqe7hln 929B8QTLay5ASlMewkHJn3L6+s8gSwZ3JIZ92sfXegVGgsjrl/Fp4WfpacDgVkTxliGN qHztBnsx3oLQKahrG5Xh5IKYSMxdQdDSjER5XsHgqH59oDCbr60+O4QmtErlwaEKz77D mAd5nxiZhD7BeOVsbatc7nLFeLRMCa30byXL+OiZp47FezjRRF9K0Ba2vML1MmePMKzH l9Ag== ARC-Authentication-Results: i=1; gmr-mx.google.com; dkim=pass header.i=@teufel.de header.s=hse1 header.b=dKLL2v5b; spf=pass (google.com: domain of sava.jakovljev@teufel.de designates 83.246.65.92 as permitted sender) smtp.mailfrom=sava.jakovljev@teufel.de Received: from mx-relay06-hz2.antispameurope.com (mx-relay06-hz2.antispameurope.com. [83.246.65.92]) by gmr-mx.google.com with ESMTPS id 7si237026lfp.13.2021.01.21.05.37.29 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Thu, 21 Jan 2021 05:37:29 -0800 (PST) Received-SPF: pass (google.com: domain of sava.jakovljev@teufel.de designates 83.246.65.92 as permitted sender) client-ip=83.246.65.92; Received: from unknown ([212.91.255.190]) by mx-relay06-hz2.antispameurope.com; Thu, 21 Jan 2021 14:37:28 +0100 From: Sava Jakovljev To: CC: Sava Jakovljev Subject: [swupdate] [PATCH v5] channel_curl: Improve tracking of download progress Date: Thu, 21 Jan 2021 14:37:07 +0100 Message-ID: <20210121133707.111032-1-sava.jakovljev@teufel.de> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 X-Originating-IP: [10.10.25.44] X-ClientProxiedBy: DNS-EX-02.teufel.local (10.10.0.81) To DNS-EX-02.teufel.local (10.10.0.81) X-C2ProcessedOrg: b93e13a0-e8da-4ba4-97b8-f14375b21c41 X-cloud-security-sender: sava.jakovljev@teufel.de X-cloud-security-recipient: swupdate@googlegroups.com X-cloud-security-Virusscan: CLEAN X-cloud-security-disclaimer: This E-Mail was scanned by E-Mailservice on mx-relay06-hz2.antispameurope.com with 1DE1B541398 X-cloud-security-connect: unknown[212.91.255.190], TLS=1, IP=212.91.255.190 X-cloud-security-Digest: 371aa934af18908b7614028f431f6bce X-cloud-security: scantime:1.615 X-Original-Sender: sava.jakovljev@teufel.de X-Original-Authentication-Results: gmr-mx.google.com; dkim=pass header.i=@teufel.de header.s=hse1 header.b=dKLL2v5b; spf=pass (google.com: domain of sava.jakovljev@teufel.de designates 83.246.65.92 as permitted sender) smtp.mailfrom=sava.jakovljev@teufel.de Precedence: list Mailing-list: list swupdate@googlegroups.com; contact swupdate+owners@googlegroups.com List-ID: X-Spam-Checked-In-Group: swupdate@googlegroups.com X-Google-Group-Id: 605343134186 List-Post: , List-Help: , List-Archive: , List-Unsubscribe: , * Compile xferinfo_legacy function only for version of libcurl not supporting newer XFERINFOFUNCTION option. * Use either XFERINFOFUNCTION or PROGRESSFUNCTION libcurl option depending on libcurl version - use XFERINFOFUNCTION if possible. * Introduce a structure containing information about file being downloaded. * Move setting progress tracking options into separate functions. * Improve callback for tracking download progress - correctly track percentage using valid variable and fix passing address of a temporary variable as callback argument. Signed-off-by: Sava Jakovljev --- When a server doesn't insert Content-Length header, there is no sense in enabling libcurl option XFERINFOFUNCTION/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 XFERINFOFUNCTION/PROGRESSFUNCTION. Also, this change will use XFERINFOFUNCTION if libcurl supports it, and PROGRESSFUNCTION otherwise, and not both of them will be set, as the situation is right now. Legacy xferinfo callback function is now compiled only when libcurl does not support XFERINFOFUNCTION. Secondly, download progress tracking is now enabled only for get_file HTTP request and if server has correctly reported total download file size. v2 of this patch deals with compiler warning and removes unused fields of introduced download_callback_data_t structure. Also, code is cleaned up. v3 of this patch deals with grammar errors in the git commit message, with no functional changes to the patch itself. v4 of this patch addresses review comments: xferinfo callback has been simplified, but download_callback_data_t structure has been kept, in order to enable possible future extensions. v5 of this patch addresses issued found by Coverty tool. Secondly, TRACE logs of download progress, in xferinfo function, are replaced with DEBUG logs. Best regards, Sava Jakovljev corelib/channel_curl.c | 124 +++++++++++++++++++++++++++++++---------- 1 file changed, 95 insertions(+), 29 deletions(-) diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c index a3e785f..f734be3 100644 --- a/corelib/channel_curl.c +++ b/corelib/channel_curl.c @@ -55,6 +55,11 @@ typedef struct { output_data_t *outdata; } write_callback_t; +typedef struct { + 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,25 +419,31 @@ 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)) { + if ((dltotal <= 0) || (dlnow > dltotal)) return 0; - } - double percent = 100.0 * (dlnow/1024.0) / (dltotal/1024.0); - double *last_percent = (double*)p; - if ((int)*last_percent == (int)percent) { + + uint8_t percent = 100.0 * ((double)dlnow / dltotal); + download_callback_data_t *data = (download_callback_data_t*)p; + + if (data->percent >= percent) return 0; - } - *last_percent = percent; + else + data->percent = percent; + + DEBUG("Downloaded %d%% (%lu of %lu kB).", percent, dlnow / 1024, dltotal / 1024); swupdate_download_update(percent, dltotal); + return 0; } +#if LIBCURL_VERSION_NUM < 0x072000 static int channel_callback_xferinfo_legacy(void *p, double dltotal, double dlnow, double ultotal, double ulnow) { return channel_callback_xferinfo(p, (curl_off_t)dltotal, (curl_off_t)dlnow, (curl_off_t)ultotal, (curl_off_t)ulnow); } +#endif static size_t channel_callback_headers(char *buffer, size_t size, size_t nitems, void *userdata) { @@ -566,28 +577,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 +720,73 @@ 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); + + curl_off_t size = -1; + 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; + + if (curl_easy_getinfo(this->handle, CURLINFO_CONTENT_LENGTH_DOWNLOAD_T, + &size) != CURLE_OK) + goto cleanup; + +cleanup: + if (curl_easy_setopt(this->handle, CURLOPT_NOBODY, 0L) != CURLE_OK) { + ERROR("Failed to properly clean-up handle."); + size = -1; + } + + return size; +} + +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->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 +1086,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); + if (curl_easy_setopt(channel_curl->handle, CURLOPT_CUSTOMREQUEST, "GET") != CURLE_OK) { ERROR("Set GET channel method option failed.");