curl-library
[PATCH v2 1/2] Ensure progress.size_dl/progress.size_ul are always >= 0
Date: Fri, 29 Aug 2014 14:48:03 -0700
Historically the default "unknown" value for progress.size_dl and
progress.size_ul has been zero, since these values are initialized
implicitly by the calloc that allocates the curl handle that these
variables are a part of. Users of curl that install progress
callbacks may expect these values to always be >= 0.
Currently it is possible for progress.size_dl and progress.size_ul
to by set to a value of -1, if Curl_pgrsSetDownloadSize() or
Curl_pgrsSetUploadSize() are passed a "size" of -1 (which a few
places currently do, and a following patch will add more). So
lets update Curl_pgrsSetDownloadSize() and Curl_pgrsSetUploadSize()
so they make sure that these variables always contain a value that
is >= 0.
Update test579.
Signed-off-by: Brandon Casey <drafnel_at_gmail.com>
--- On Thu, Aug 28, 2014 at 3:12 PM, Daniel Stenberg <daniel_at_haxx.se> wrote: > On Wed, 27 Aug 2014, Brandon Casey wrote: > >> Instead of passing in 0 to reset the download size and the upload size, >> let's pass in -1. This will make Curl_pgrsSetDownloadSize() and >> Curl_pgrsSetUploadSize() clear their respective "KNOWN" bits in >> progress.flags and will allow curl_easy_getinfo() to return -1 appropriately >> when CURLINFO_CONTENT_LENGTH_DOWNLOAD or CURLINFO_CONTENT_LENGTH_UPLOAD is >> requested. > > > Thanks what an awesome research and fix! Thanks. > I only have one little remark here: by also storing -1 in the struct fields > progress.size_dl and progress.size_ul this change alters what the progress > function callbacks send in their 2nd and 4th arguments - and that is > probably an ABI breakage users won't like. > > How about changing Curl_pgrsSetDownloadSize() and Curl_pgrsSetUploadSize() > to simply see the value as negative, clear the known flag but then store the > value as 0? That sounds sane. I was tempted to say that maybe progress callbacks would want to be passed -1 to indicate that the value was unknown and that this would be in-line with the CURLINFO_CONTENT_LENGTH_DOWNLOAD et al behavior, but the progress callbacks have never worked like this (I think), since the base curl handle is initialized using calloc and the progress.size_[du]l variables get initialized to zero implicitly by this. So the progress callbacks have always been passed a value that is >= 0 regardless of whether the underlying values are known. So I think it makes sense to ensure that these values are always >= 0. -Brandon lib/progress.c | 18 ++++++++++++------ tests/data/test579 | 11 +++++------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/lib/progress.c b/lib/progress.c index e6a8d82..3963e67 100644 --- a/lib/progress.c +++ b/lib/progress.c @@ -234,20 +234,26 @@ void Curl_pgrsSetUploadCounter(struct SessionHandle *data, curl_off_t size) void Curl_pgrsSetDownloadSize(struct SessionHandle *data, curl_off_t size) { - data->progress.size_dl = size; - if(size >= 0) + if(size >= 0) { + data->progress.size_dl = size; data->progress.flags |= PGRS_DL_SIZE_KNOWN; - else + } + else { + data->progress.size_dl = 0; data->progress.flags &= ~PGRS_DL_SIZE_KNOWN; + } } void Curl_pgrsSetUploadSize(struct SessionHandle *data, curl_off_t size) { - data->progress.size_ul = size; - if(size >= 0) + if(size >= 0) { + data->progress.size_ul = size; data->progress.flags |= PGRS_UL_SIZE_KNOWN; - else + } + else { + data->progress.size_ul = 0; data->progress.flags &= ~PGRS_UL_SIZE_KNOWN; + } } /* diff --git a/tests/data/test579 b/tests/data/test579 index adbb3dd..e352e3d 100644 --- a/tests/data/test579 +++ b/tests/data/test579 @@ -77,12 +77,11 @@ http://%HOSTIP:%HTTPPORT/579 log/ip579 <verify> <file name="log/ip579"> Progress callback called with UL 0 out of 0 -Progress callback called with UL 0 out of -1 -Progress callback called with UL 8 out of -1 -Progress callback called with UL 16 out of -1 -Progress callback called with UL 26 out of -1 -Progress callback called with UL 61 out of -1 -Progress callback called with UL 66 out of -1 +Progress callback called with UL 8 out of 0 +Progress callback called with UL 16 out of 0 +Progress callback called with UL 26 out of 0 +Progress callback called with UL 61 out of 0 +Progress callback called with UL 66 out of 0 </file> </verify> </testcase> -- 2.0.0.rc2 ------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.htmlReceived on 2014-08-29