[Libguestfs] [PATCH] Fix lossy conversion of Content-Length

[Pino Toscano]

On Thursday, 9 January 2020 17:41:58 CET Adrian Ambrożewicz wrote:
> W dniu 1/9/2020 o 14:07, Richard W.M. Jones pisze:
> > On Wed, Jan 08, 2020 at 07:19:56AM -0600, Eric Blake wrote:
> >> On 1/7/20 4:13 AM, Adrian Ambrożewicz wrote:
> >>> Actual variable holding content length is int64_t, but it was assigned
> >>> by explicit cast to size_t. On 32-bit systems it's a lossy conversion,
> >>> so it was replaced by casting to int64_t instead.
> >>>
> >>> Signed-off-by: Adrian Ambrożewicz <adrian.ambrozewicz at linux.intel.com>
> >>> ---
> >>>   plugins/curl/curl.c | 2 +-
> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/plugins/curl/curl.c b/plugins/curl/curl.c
> >>> index 031bd32..fe1330e 100644
> >>> --- a/plugins/curl/curl.c
> >>> +++ b/plugins/curl/curl.c
> >>> @@ -389,7 +389,7 @@ curl_open (int readonly)
> >>>       goto err;
> >>>     }
> >>>
> >>> -  h->exportsize = (size_t) d;
> >>> +  h->exportsize = (int64_t) d;
> >>
> >> Why is a cast needed at all?
> >> This is C, an implicit conversion
> >> works just as well.
> > 
> > Present since the initial commit, but not present in the qemu
> > block/curl.c driver which is where most of this code was derived from,
> > so I suspect I simply added it "because reasons".
> > 
> > But even more to the point, why on earth does the
> > CURLINFO_CONTENT_LENGTH_DOWNLOAD API return a double!  Well it seems
> > as if they replaced that API with:
> > 
> > https://curl.haxx.se/libcurl/c/CURLINFO_CONTENT_LENGTH_DOWNLOAD_T.html
> > 
> > which is what we really ought to use instead.  This was added in Curl
> > 7.55.0, released 2 and a half years ago, so I think we ought to use
> > that instead.  Adrian could you write a patch to change over to that
> > API?
> > 
> > Rich.
> I see that you've already ACK-ed such change by Pino so I assume my 
> patch is OK? Or should I remove casting altogether?

My patch used the better API with curl >= 7.55.0. In older versions the
issue still exists, and removing the cast should work.

Can you please provide a v2 of your patch with that change?

Thanks,
-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20200109/fd4ceca1/attachment.sig>