Adrian Ambrożewicz
2020-Jan-07 10:13 UTC
[Libguestfs] [PATCH] Fix lossy conversion of Content-Length
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;
nbdkit_debug ("content length: %" PRIi64, h->exportsize);
if (strncasecmp (url, "http://", strlen ("http://")) ==
0 ||
--
2.17.1
Eric Blake
2020-Jan-08 13:19 UTC
Re: [Libguestfs] [PATCH] Fix lossy conversion of Content-Length
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@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.> nbdkit_debug ("content length: %" PRIi64, h->exportsize); > > if (strncasecmp (url, "http://", strlen ("http://")) == 0 ||-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Adrian Ambrożewicz
2020-Jan-09 11:32 UTC
Re: [Libguestfs] [PATCH] Fix lossy conversion of Content-Length
W dniu 1/8/2020 o 14:19, Eric Blake pisze:> 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@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.I agree. It was just me being careful with scope of changes :) Should I send new patch? And another question - who is the maintainer and how should I proceed with patch after acceptance on mailing list?> >> nbdkit_debug ("content length: %" PRIi64, h->exportsize); >> >> if (strncasecmp (url, "http://", strlen ("http://")) == 0 || >
Richard W.M. Jones
2020-Jan-09 13:07 UTC
Re: [Libguestfs] [PATCH] Fix lossy conversion of Content-Length
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@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.> > nbdkit_debug ("content length: %" PRIi64, h->exportsize); > > > > if (strncasecmp (url, "http://", strlen ("http://")) == 0 || > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html