From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
qemu-block@nongnu.org, tao3.xu@intel.com, qemu-devel@nongnu.org,
armbru@redhat.com, Max Reitz <mreitz@redhat.com>
Subject: Re: [PATCH 1/3] utils: Improve qemu_strtosz() to have 64 bits of precision
Date: Fri, 5 Feb 2021 14:10:08 +0000 [thread overview]
Message-ID: <20210205141008.GO908621@redhat.com> (raw)
In-Reply-To: <f2463f98-90a9-380d-06dd-9e410c32cfe3@redhat.com>
On Fri, Feb 05, 2021 at 08:06:53AM -0600, Eric Blake wrote:
> On 2/5/21 4:06 AM, Vladimir Sementsov-Ogievskiy wrote:
>
> >>> - /*
> >>> - * Values near UINT64_MAX overflow to 2**64 when converting to
> >>> double
> >>> - * precision. Compare against the maximum representable double
> >>> precision
> >>> - * value below 2**64, computed as "the next value after 2**64
> >>> (0x1p64) in
> >>> - * the direction of 0".
> >>> - */
> >>> - if ((val * mul > nextafter(0x1p64, 0)) || val < 0) {
> >>> + if (val > UINT64_MAX / mul) {
> >>
> >> Hmm, do we care about:
> >> 15.9999999999999999999999999999E
> >> where the fractional portion becomes large enough to actually bump our
> >> sum below to 16E which indeed overflows? Then again, we rejected a
> >> fraction of 1.0 above, and 0.9999999999999999999999999999 parses to 1.0
> >> due to rounding.
> >> Maybe it's just worth a good comment why the overflow check here works
> >> without consulting fraction.
> >
> > worth a good comment, because I don't follow :)
> >
> > If mul is big enough and fraction=0.5, why val*mul + fraction*mul will
> > not overflow?
>
> When mul is a power of 2, we know that fraction*mul does not change the
> number of significant bits, but merely moves the exponent, so starting
> with fraction < 1.0, we know fraction*mul < mul. But when @unit is
> 1000, there is indeed a rare possibility that the multiplication will
> cause an inexact answer that will trigger rounding, so we could end up
> with fraction*mul == mul. So I'm not yet 100% confident that there is
> no possible combination where we can't cause an overflow to result in
> val*mul + (uint64_t)(fraction*mul) resulting in 0 instead of UINT64_MAX,
> and I think I will have to tighten this code up for v2.
>
>
> >
> > Also, if we find '.' in the number, why not just reparse the whole
> > number with qemu_strtod_finite? And don't care about 1.0?
>
> Reparsing the whole number loses precision. Since we already have a
> 64-bit precise integer, why throw it away?
Yep, it isn't acceptable to throw away precision of the non-fractional
part of the input IMHO.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
next prev parent reply other threads:[~2021-02-05 14:11 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-04 19:07 [PATCH 0/3] Improve do_strtosz precision Eric Blake
2021-02-04 19:07 ` [PATCH 1/3] utils: Improve qemu_strtosz() to have 64 bits of precision Eric Blake
2021-02-04 20:12 ` Eric Blake
2021-02-05 10:06 ` Vladimir Sementsov-Ogievskiy
2021-02-05 10:18 ` Vladimir Sementsov-Ogievskiy
2021-02-05 14:06 ` Eric Blake
2021-02-05 14:10 ` Daniel P. Berrangé [this message]
2021-02-05 10:07 ` Vladimir Sementsov-Ogievskiy
2021-02-05 14:12 ` Eric Blake
2021-02-05 10:28 ` Richard W.M. Jones
2021-02-05 14:15 ` Eric Blake
2021-02-05 11:02 ` Daniel P. Berrangé
2021-02-05 14:27 ` Eric Blake
2021-02-05 14:36 ` Daniel P. Berrangé
2021-02-05 11:34 ` Daniel P. Berrangé
2021-02-05 14:36 ` Eric Blake
2021-02-04 19:07 ` [PATCH 2/3] utils: Deprecate hex-with-suffix sizes Eric Blake
2021-02-05 10:25 ` Vladimir Sementsov-Ogievskiy
2021-02-05 10:31 ` Richard W.M. Jones
2021-02-05 13:38 ` Eric Blake
2021-02-05 11:13 ` Daniel P. Berrangé
2021-02-05 13:40 ` Eric Blake
2021-02-05 14:02 ` Daniel P. Berrangé
2021-02-04 19:07 ` [PATCH 3/3] utils: Deprecate inexact fractional suffix sizes Eric Blake
2021-02-04 20:02 ` Eric Blake
2021-02-05 10:34 ` Richard W.M. Jones
2021-02-05 14:19 ` Eric Blake
2021-02-05 10:38 ` Vladimir Sementsov-Ogievskiy
2021-02-05 11:10 ` Daniel P. Berrangé
2021-02-05 14:28 ` Eric Blake
2021-02-05 14:40 ` Daniel P. Berrangé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210205141008.GO908621@redhat.com \
--to=berrange@redhat.com \
--cc=armbru@redhat.com \
--cc=eblake@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=tao3.xu@intel.com \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.