From: "Richard W.M. Jones" <rjones@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
vsementsov@virtuozzo.com, berrange@redhat.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 10:28:05 +0000 [thread overview]
Message-ID: <20210205102805.GA30079@redhat.com> (raw)
In-Reply-To: <20210204190708.1306296-2-eblake@redhat.com>
On Thu, Feb 04, 2021 at 01:07:06PM -0600, Eric Blake wrote:
> We have multiple clients of qemu_strtosz (qemu-io, the opts visitor,
> the keyval visitor), and it gets annoying that edge-case testing is
> impacted by implicit rounding to 53 bits of precision due to parsing
> with strtod(). As an example posted by Rich Jones:
> $ nbdkit memory $(( 2**63 - 2**30 )) --run \
> 'build/qemu-io -f raw "$uri" -c "w -P 3 $(( 2**63 - 2**30 - 512 )) 512" '
> write failed: Input/output error
>
> because 9223372035781033472 got rounded to 0x7fffffffc0000000 which is
> out of bounds.
>
> It is also worth noting that our existing parser, by virtue of using
> strtod(), accepts decimal AND hex numbers, even though test-cutils
> previously lacked any coverage of the latter. We do have existing
> clients that expect a hex parse to work (for example, iotest 33 using
> qemu-io -c "write -P 0xa 0x200 0x400"), but strtod() parses "08" as 8
> rather than as an invalid octal number, so we know there are no
> clients that depend on octal. Our use of strtod() also means that
> "0x1.8k" would actually parse as 1536 (the fraction is 8/16), rather
> than 1843 (if the fraction were 8/10); but as this was not covered in
> the testsuite, I have no qualms forbidding hex fractions as invalid,
> so this patch declares that the use of fractions is only supported
> with decimal input, and enhances the testsuite to document that.
>
> Our previous use of strtod() meant that -1 parsed as a negative; now
> that we parse with strtoull(), negative values can wrap around module
^^ modulo
The patch looked fine to me although Vladimir found some problems
which I didn't spot. I have a question: What happens with leading or
trailing whitespace? Is that ignored, rejected or impossible?
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
next prev parent reply other threads:[~2021-02-05 10:31 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é
2021-02-05 10:07 ` Vladimir Sementsov-Ogievskiy
2021-02-05 14:12 ` Eric Blake
2021-02-05 10:28 ` Richard W.M. Jones [this message]
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=20210205102805.GA30079@redhat.com \
--to=rjones@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@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.