[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 0/3] Improve do_strtosz precision
From: |
Eric Blake |
Subject: |
[PATCH 0/3] Improve do_strtosz precision |
Date: |
Thu, 4 Feb 2021 13:07:05 -0600 |
Recently, commit 8b1170012b tweaked the maximum size the block layer
will allow, which in turn affects nbdkit's testsuite of edge-case
behaviors, where Rich noted [1] that our use of double meant rounding
errors that cause spurious failures in qemu-io (among other places).
So I decided to fix that. In the process, I was reminded that we have
attempted this before, most recently in Dec 2019 [2], where Markus
(rightly, IMHO) said that any approach that tries to reimplement
strtoull() or which compares the amount of data consumed between
strtod() and strtoull() adds more complexity than it solves.
So first, our analysis of what we absolutely need:
- Existing clients expect decimal scaling to work (1M is a lot easier
to type than 1048576)
- Existing clients expect hex to work (my initial attempt disabled it,
and promptly hung in iotests when things like 0x10000 got parsed as
zero rather than their intended byte value) (although our existing
testsuite coverage was only via iotests, rather than unit tests)
- Existing clients expect sane decimal fractions to work (1.5k is
easier to type than 1536), although our testsuite coverage of this
was less apparant
- Existing clients are surprised when something that looks like a byte
value is truncated or rounded due to an intermediate pass through
double (hence Rich's bug report)
[1] https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg00812.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg00852.html
My solution, instead of parsing twice and picking the longest, is to
always parse the integral portion with strtoull(), then special case
on what is left: if it is 'x' or 'X', the user wanted hex (and does
NOT want a fraction, and in fact we can optionally warn about the use
of scaling suffixes in this case); if it is '.', the user wanted the
convenience of a floating point adjustment which we can do via
strtod() on the suffix (and where we can optionally warn about
fractions that are not evenly divisible into bytes). I also enhanced
the testsuite (our coverage for hex constants was implicit via
iotests, and now explicit in the unit test; and our testsuite did not
flag the fact that we were previously accepting nonsense like '0x1.8k'
for 1536, or '1.5e1M' for 53477376).
We may decide that one or both of patch 2 and 3 goes too far, which is
why I split them out from the main enhancement.
Eric Blake (3):
utils: Improve qemu_strtosz() to have 64 bits of precision
utils: Deprecate hex-with-suffix sizes
utils: Deprecate inexact fractional suffix sizes
docs/system/deprecated.rst | 8 ++++
tests/test-cutils.c | 83 ++++++++++++++++++++++++++++---------
tests/test-keyval.c | 28 +++++++++----
tests/test-qemu-opts.c | 24 +++++++----
util/cutils.c | 85 +++++++++++++++++++++++++++++---------
tests/qemu-iotests/049.out | 7 +++-
6 files changed, 178 insertions(+), 57 deletions(-)
--
2.30.0