cURL / Mailing Lists / curl-library / Single Mail

curl-library

[PATCH v2 1/2] Ensure progress.size_dl/progress.size_ul are always >= 0

From: Brandon Casey <drafnel_at_gmail.com>
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.html
Received on 2014-08-29