[Qemu-commits] [qemu/qemu] f214a2: iotests: Fix test 104 under NBD

[Richard Henderson]

  Branch: refs/heads/staging
  Home:   https://github.com/qemu/qemu
  Commit: f214a28a2c33cf5104f20403e26e65288cc878dc
      
https://github.com/qemu/qemu/commit/f214a28a2c33cf5104f20403e26e65288cc878dc
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/qemu-iotests/common.filter
    M tests/qemu-iotests/common.rc

  Log Message:
  -----------
  iotests: Fix test 104 under NBD

In the past, commit a231cb27 ("iotests: Fix 104 for NBD", v2.3.0)
added an additional filter to _filter_img_info to rewrite NBD URIs
into the expected output form.  This recently broke when we tweaked
tests to run in a per-format directory, which did not match the regex,
because _img_info itself is now already changing
SOCK_DIR=/tmp/tmpphjfbphd/raw-nbd-104 into
/tmp/tmpphjfbphd/IMGFMT-nbd-104 prior to _img_info_filter getting a
chance to further filter things.

While diagnosing the problem, I also noticed some filter lines
rendered completely useless by a typo when we switched from TCP to
Unix sockets for NBD (in shell, '\\+' is different from "\\+" (one
gives two backslash to the regex, matching the literal 2-byte sequence
<\+> after a single digit; the other gives one backslash to the regex,
as the metacharacter \+ to match one or more of <[0-9]>); since the
literal string <nbd://127.0.0.1:0\+> is not a valid URI, that regex
hasn't been matching anything for years so it is fine to just drop it
rather than fix the typo.

Fixes: f3923a72 ("iotests: Switch nbd tests to use Unix rather than TCP", 
v4.2.0)
Fixes: 5ba7db09 ("iotests: always use a unique sub-directory per test", v8.0.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20230519150216.2599189-1-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


  Commit: 7434102d58509d5bd8c24b0cf1da08d6d21695c5
      
https://github.com/qemu/qemu/commit/7434102d58509d5bd8c24b0cf1da08d6d21695c5
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M docs/interop/qcow2.txt

  Log Message:
  -----------
  qcow2: Explicit mention of padding bytes

Although we already covered the need for padding bytes with our
changes in commit 3ae3fcfa, commit 66fcbca5 (both v5.0.0) added one
byte and relied on the rest of the text for implicitly covering 7
padding bytes.  For consistency with other parts of the header (such
as the header extension format listing padding from n - m, or the
snapshot table entry listing variable padding), we might as well call
out the remaining 7 bytes as padding until such time (as any) as they
gain another meaning.

Signed-off-by: Eric Blake <eblake@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20230522184631.47211-1-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>


  Commit: ef598da85538892de2801701253acf5bde6e7981
      
https://github.com/qemu/qemu/commit/ef598da85538892de2801701253acf5bde6e7981
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c

  Log Message:
  -----------
  test-cutils: Avoid g_assert in unit tests

glib documentation[1] is clear: g_assert() should be avoided in unit
tests because it is ineffective if G_DISABLE_ASSERT is defined; unit
tests should stick to constructs based on g_assert_true() instead.
Note that since commit 262a69f428, we intentionally state that you
cannot define G_DISABLE_ASSERT while building qemu; but our code can
be copied to other projects without that restriction, so we should be
consistent.

For most of the replacements in this patch, using g_assert_cmpstr()
would be a regression in quality - although it would helpfully display
the string contents of both pointers on test failure, here, we really
do care about pointer equality, not just string content equality.  But
when a NULL pointer is expected, g_assert_null works fine.

[1] https://libsoup.org/glib/glib-Testing.html#g-assert

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230522190441.64278-2-eblake@redhat.com>


  Commit: ed5638426f9a7db28d081fe0a8953c33211460a7
      
https://github.com/qemu/qemu/commit/ed5638426f9a7db28d081fe0a8953c33211460a7
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c

  Log Message:
  -----------
  test-cutils: Use g_assert_cmpuint where appropriate

When debugging test failures, seeing unsigned values as large positive
values rather than negative values matters (assuming glib 2.78+; given
that I just fixed a bug in glib 2.76 [1] where g_assert_cmpuint
displays signed instead of unsigned values).  No impact when the test
is passing, but using a consistent style will matter more in upcoming
test additions.  Also, some tests are better with cmphex.

While at it, fix some spacing and minor typing issues spotted nearby.

[1] https://gitlab.gnome.org/GNOME/glib/-/issues/2997

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-3-eblake@redhat.com>


  Commit: f192c4eba870cd5dedb38421087bedbafa90dc56
      
https://github.com/qemu/qemu/commit/f192c4eba870cd5dedb38421087bedbafa90dc56
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c

  Log Message:
  -----------
  test-cutils: Test integral qemu_strto* value on failures

We are inconsistent on the contents of *value after a strto* parse
failure.  I found the following behaviors:

- parse_uint() and parse_uint_full(), which document that *value is
  slammed to 0 on all EINVAL failures and 0 or UINT_MAX on ERANGE
  failures, and has unit tests for that (note that parse_uint requires
  non-NULL endptr, and does not fail with EINVAL for trailing junk)

- qemu_strtosz(), which leaves *value untouched on all failures (both
  EINVAL and ERANGE), and has unit tests but not documentation for
  that

- qemu_strtoi() and other integral friends, which document *value on
  ERANGE failures but is unspecified on EINVAL (other than implicitly
  by comparison to libc strto*); there, *value is untouched for NULL
  string, slammed to 0 on no conversion, and left at the prefix value
  on NULL endptr; unit tests do not consistently check the value

- qemu_strtod(), which documents *value on ERANGE failures but is
  unspecified on EINVAL; there, *value is untouched for NULL string,
  slammed to 0.0 for no conversion, and left at the prefix value on
  NULL endptr; there are no unit tests (other than indirectly through
  qemu_strtosz)

- qemu_strtod_finite(), which documents *value on ERANGE failures but
  is unspecified on EINVAL; there, *value is left at the prefix for
  'inf' or 'nan' and untouched in all other cases; there are no unit
  tests (other than indirectly through qemu_strtosz)

Upcoming patches will change behaviors for consistency, but it's best
to first have more unit test coverage to see the impact of those
changes.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-4-eblake@redhat.com>


  Commit: db52484849b35f0e7e51fd5c753f605abfcd3425
      
https://github.com/qemu/qemu/commit/db52484849b35f0e7e51fd5c753f605abfcd3425
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c

  Log Message:
  -----------
  test-cutils: Test more integer corner cases

We have quite a few undertested and underdocumented integer parsing
corner cases.  To ensure that any changes we make in the code are
intentional rather than accidental semantic changes, it is time to add
more unit tests of existing behavior.

In particular, this demonstrates that parse_uint() and qemu_strtou64()
behave differently.  For "-0", it's hard to argue why parse_uint needs
to reject it (it's not a negative integer), but the documentation sort
of mentions it; but it is intentional that all other negative values
are treated as ERANGE with value 0 (compared to qemu_strtou64()
treating "-2" as success and UINT64_MAX-1, for example).

Also, when mixing overflow/underflow with a check for no trailing
junk, parse_uint_full favors ERANGE over EINVAL, while qemu_strto[iu]*
favor EINVAL.  This behavior is outside the C standard, so we can pick
whatever we want, but it would be nice to be consistent.

Note that C requires that "9223372036854775808" fail strtoll() with
ERANGE/INT64_MAX, but "-9223372036854775808" pass with INT64_MIN; we
weren't testing this.  For strtol(), the behavior depends on whether
long is 32- or 64-bits (the cutoff point either being the same as
strtoll() or at "-2147483648").  Meanwhile, C is clear that
"-18446744073709551615" pass stroull() (but not strtoll) with value 1,
even though we want it to fail parse_uint().  And although
qemu_strtoui() has no C counterpart, it makes more sense if we design
it like 32-bit strtoul() (that is, where "-4294967296" be an alternate
acceptable spelling for "1", but "-0xffffffff00000001" should be
treated as overflow and return 0xffffffff rather than 1).  We aren't
there yet, so some of the tests added in this patch have FIXME
comments.

However, note that C2x will (likely) be adding a SILENT semantic
change, where C17 strtol("0b1", &ep, 2) returns 0 with ep="b1", but
C2x will have it return 1 with ep="".  I did not feel like adding
testing for those corner cases, in part because the next version of C
is not standard and libc support for binary parsing is not yet
wide-spread (as of this patch, glibc.git still misparses bare "0b":
https://sourceware.org/bugzilla/show_bug.cgi?id=30371).

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20230522190441.64278-5-eblake@redhat.com>
[eblake: fix a few typos spotted by Hanna]
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>


  Commit: dd2cafa27ec26802701cb90f6640e283114c1caf
      
https://github.com/qemu/qemu/commit/dd2cafa27ec26802701cb90f6640e283114c1caf
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c
    M util/cutils.c

  Log Message:
  -----------
  cutils: Fix wraparound parsing in qemu_strtoui

While we were matching 32-bit strtol in qemu_strtoi, our use of a
64-bit parse was leaking through for some inaccurate answers in
qemu_strtoui in comparison to a 32-bit strtoul (see the unit test for
examples).  The comment for that function even described what we have
to do for a correct parse, but didn't implement it correctly: since
strtoull checks for overflow against the wrong values and then
negates, we have to temporarily undo negation before checking for
overflow against our desired value.

Our int wrappers would be a lot easier to write if libc had a
guaranteed 32-bit parser even on platforms with 64-bit long.

Whether we parse C2x binary strings like "0b1000" is currently up to
what libc does; our unit tests intentionally don't cover that at the
moment, though.

Fixes: 473a2a331e ("cutils: add qemu_strtoi & qemu_strtoui parsers for 
int/unsigned int types", v2.12.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
CC: qemu-stable@nongnu.org
Message-Id: <20230522190441.64278-6-eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>


  Commit: 7d6f4fe6c55836ee355789dd534e1ed469eb8978
      
https://github.com/qemu/qemu/commit/7d6f4fe6c55836ee355789dd534e1ed469eb8978
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M util/cutils.c

  Log Message:
  -----------
  cutils: Document differences between parse_uint and qemu_strtou64

These two functions are subtly different, and not just because of
swapped parameter order.  It took me adding better unit tests to
figure out why.  Document the differences to make it more obvious to
developers trying to pick which one to use, as well as to aid in
upcoming semantic changes.

While touching the documentation, adjust a mis-statement: parse_uint
does not return -EINVAL on invalid base, but assert()s, like all the
other qemu_strto* functions that take a base argument.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-7-eblake@redhat.com>


  Commit: 26be416be5dbe3d69b881f1a777eaf1595e96c82
      
https://github.com/qemu/qemu/commit/26be416be5dbe3d69b881f1a777eaf1595e96c82
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M audio/audio_legacy.c
    M block/gluster.c
    M block/nfs.c
    M blockdev.c
    M contrib/ivshmem-server/main.c
    M include/qemu/cutils.h
    M qapi/opts-visitor.c
    M tests/unit/test-cutils.c
    M ui/vnc.c
    M util/cutils.c
    M util/guest-random.c
    M util/qemu-sockets.c

  Log Message:
  -----------
  cutils: Adjust signature of parse_uint[_full]

It's already confusing that we have two very similar functions for
wrapping the parse of a 64-bit unsigned value, differing mainly on
whether they permit leading '-'.  Adjust the signature of parse_uint()
and parse_uint_full() to be like all of qemu_strto*(): put the result
parameter last, use the same types (uint64_t and unsigned long long
have the same width, but are not always the same type), and mark
endptr const (this latter change only affects the rare caller of
parse_uint).  Adjust all callers in the tree.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-8-eblake@redhat.com>


  Commit: cf82e720cd0ce45ddaa2f6076c001f0fb339c482
      
https://github.com/qemu/qemu/commit/cf82e720cd0ce45ddaa2f6076c001f0fb339c482
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c
    M util/cutils.c

  Log Message:
  -----------
  cutils: Allow NULL endptr in parse_uint()

All the qemu_strto*() functions permit a NULL endptr, just like their
libc counterparts, leaving parse_uint() as the oddball that caused
SEGFAULT on NULL and required the user to call parse_uint_full()
instead.  Relax things for consistency, even though the testsuite is
the only impacted caller.  Add one more unit test to ensure even
parse_uint_full(NULL, 0, &value) works.  This also fixes our code to
uniformly favor EINVAL over ERANGE when both apply.

Also fixes a doc mismatch @v vs. a parameter named value.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-9-eblake@redhat.com>


  Commit: b051b8d3408689e3e58070f6ce0262dbfdce8adc
      
https://github.com/qemu/qemu/commit/b051b8d3408689e3e58070f6ce0262dbfdce8adc
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c

  Log Message:
  -----------
  test-cutils: Add coverage of qemu_strtod

It's hard to tweak code for consistency if I can't prove what will or
won't break from those tweaks.  Time to add unit tests for
qemu_strtod() and qemu_strtod_finite().

Among other things, I wrote a check whether we have C99 semantics for
strtod("0x1") (which MUST parse hex numbers) rather than C89 (which
must stop parsing at 'x').  These days, I suspect that is okay; but if
it fails CI checks, knowing the difference will help us decide what we
want to do about it.  Note that C2x, while not final at the time of
this patch, has been considering whether to make strtol("0b1") parse
as 1 with no slop instead of the C17 parse of 0 with slop "b1"; that
decision may also bleed over to strtod().  But for now, I didn't think
it worth adding unit tests on that front (to strtol or strtod) as
things may still change.

Likewise, there are plenty more corner cases of strtod proper that I
don't explicitly test here, but there are enough unit tests added here
that it covers all the branches reached in our wrappers.  In
particular, it demonstrates the difference on when *value is left
uninitialized, which an upcoming patch will normalize.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-10-eblake@redhat.com>


  Commit: 74e8b1e769bfd084731e1404ed822a202e70a94d
      
https://github.com/qemu/qemu/commit/74e8b1e769bfd084731e1404ed822a202e70a94d
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c

  Log Message:
  -----------
  test-cutils: Prepare for upcoming semantic change in qemu_strtosz

A quick search for 'qemu_strtosz' in the code base shows that outside
of the testsuite, the ONLY place that passes a non-NULL pointer to
@endptr of any variant of a size parser is in hmp.c (the 'o' parser of
monitor_parse_arguments), and that particular caller warns of
"extraneous characters at the end of line" unless the trailing bytes
are purely whitespace.  Thus, it makes no semantic difference at the
high level whether we parse "1.5e1k" as "1" + ".5e1" + "k" (an attempt
to use scientific notation in strtod with a scaling suffix of 'k' with
no trailing junk, but which qemu_strtosz says should fail with
EINVAL), or as "1.5e" + "1k" (a valid size with scaling suffix of 'e'
for exabytes, followed by two junk bytes) - either way, any user
passing such a string will get an error message about a parse failure.

However, an upcoming patch to qemu_strtosz will fix other corner case
bugs in handling the fractional portion of a size, and in doing so, it
is easier to declare that qemu_strtosz() itself stops parsing at the
first 'e' rather than blindly consuming whatever strtod() will
recognize.  Once that is fixed, the difference will be visible at the
low level (getting a valid parse with trailing garbage when @endptr is
non-NULL, while continuing to get -EINVAL when @endptr is NULL); this
is easier to demonstrate by moving the affected strings from
test_qemu_strtosz_invalid() (which declares them as always -EINVAL) to
test_qemu_strtosz_trailing() (where @endptr affects behavior, for now
with FIXME comments).

Note that a similar argument could be made for having "0x1.5" or
"0x1M" parse as 0x1 with ".5" or "M" as trailing junk, instead of
blindly treating it as -EINVAL; however, as these cases do not suffer
from the same problems as floating point, they are not worth changing
at this time.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-11-eblake@redhat.com>


  Commit: 3c9ac700f7cdd06d778a6df44303adc86ebd3946
      
https://github.com/qemu/qemu/commit/3c9ac700f7cdd06d778a6df44303adc86ebd3946
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c

  Log Message:
  -----------
  test-cutils: Refactor qemu_strtosz tests for less boilerplate

No need to copy-and-paste lots of boilerplate per string tested, when
we can consolidate that behind helper functions.  Plus, this adds a
bit more coverage (we now test all strings both with and without
endptr, whereas before some tests skipped the NULL endptr case), which
exposed a SEGFAULT on qemu_strtosz(NULL, NULL, &val) that will be
fixed in an upcoming patch.

Note that duplicating boilerplate has one advantage lost here - a
failed test tells you which line number failed; but a helper function
does not show the call stack that reached the failure.  Since we call
the helper more than once within many of the "unit tests", even the
unit test name doesn't point out which call is failing.  But that only
matters when tests fail (they normally pass); at which point I'm
debugging the failures under gdb anyways, so I'm not too worried about
it.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-12-eblake@redhat.com>


  Commit: ed4394f06b58238f8b665118bf122c471bd37a6c
      
https://github.com/qemu/qemu/commit/ed4394f06b58238f8b665118bf122c471bd37a6c
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c
    M util/cutils.c

  Log Message:
  -----------
  cutils: Allow NULL str in qemu_strtosz

All the other qemu_strto* and parse_uint allow a NULL str.  Having
qemu_strtosz not crash on qemu_strtosz(NULL, NULL, &value) is an easy
fix that adds some consistency between our string parsers.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-13-eblake@redhat.com>


  Commit: 3215c7cb293d3f700617ce2800c94501adb39629
      
https://github.com/qemu/qemu/commit/3215c7cb293d3f700617ce2800c94501adb39629
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M hw/core/numa.c

  Log Message:
  -----------
  numa: Check for qemu_strtosz_MiB error

As shown in the previous commit, qemu_strtosz_MiB sometimes leaves the
result value untouched (we have to audit further to learn that in that
case, the QAPI generator says that visit_type_NumaOptions() will have
zero-initialized it), and sometimes leaves it with the value of a
partial parse before -EINVAL occurs because of trailing garbage.
Rather than blindly treating any string the user may throw at us as
valid, we should check for parse failures.

Fixes: cc001888 ("numa: fixup parsed NumaNodeOptions earlier", v2.11.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-14-eblake@redhat.com>


  Commit: b0a1830fc5d4916ce7eb7133ebf4e33289be8742
      
https://github.com/qemu/qemu/commit/b0a1830fc5d4916ce7eb7133ebf4e33289be8742
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c

  Log Message:
  -----------
  test-cutils: Add more coverage to qemu_strtosz

Add some more strings that the user might send our way.  In
particular, some of these additions include FIXME comments showing
where our parser doesn't quite behave the way we want.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-15-eblake@redhat.com>


  Commit: f7bec30fa8a29cc49e070b44c4fc66ba08cd5079
      
https://github.com/qemu/qemu/commit/f7bec30fa8a29cc49e070b44c4fc66ba08cd5079
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c
    M util/cutils.c

  Log Message:
  -----------
  cutils: Set value in all qemu_strtosz* error paths

Making callers determine whether or not *value was populated on error
is not nice for usability.  Pre-patch, we have unit tests that check
that *result is left unchanged on most EINVAL errors and set to 0 on
many ERANGE errors.  This is subtly different from libc strtoumax()
behavior which returns UINT64_MAX on ERANGE errors, as well as
different from our parse_uint() which slams to 0 on EINVAL on the
grounds that we want our functions to be harder to mis-use than
strtoumax().

Let's audit callers:

- hw/core/numa.c:parse_numa() fixed in the previous patch to check for
  errors

- migration/migration-hmp-cmds.c:hmp_migrate_set_parameter(),
  monitor/hmp.c:monitor_parse_arguments(),
  qapi/opts-visitor.c:opts_type_size(),
  qapi/qobject-input-visitor.c:qobject_input_type_size_keyval(),
  qemu-img.c:cvtnum_full(), qemu-io-cmds.c:cvtnum(),
  target/i386/cpu.c:x86_cpu_parse_featurestr(), and
  util/qemu-option.c:parse_option_size() appear to reject all failures
  (although some with distinct messages for ERANGE as opposed to
  EINVAL), so it doesn't matter what is in the value parameter on
  error.

- All remaining callers are in the testsuite, where we can tweak our
  expectations to match our new desired behavior.

Advancing to the end of the string parsed on overflow (ERANGE), while
still returning 0, makes sense (UINT64_MAX as a size is unlikely to be
useful); likewise, our size parsing code is complex enough that it's
easier to always return 0 when endptr is NULL but trailing garbage was
found, rather than trying to return the value of the prefix actually
parsed (no current caller cared about the value of the prefix).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-16-eblake@redhat.com>


  Commit: 9f7e421a7a34ae60a279ce6005983856aa8f6ada
      
https://github.com/qemu/qemu/commit/9f7e421a7a34ae60a279ce6005983856aa8f6ada
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c
    M util/cutils.c

  Log Message:
  -----------
  cutils: Set value in all integral qemu_strto* error paths

Our goal in writing qemu_strtoi() and friends is to have an interface
harder to abuse than libc's strtol().  Leaving the return value
uninitialized on some but not all error paths does not lend itself
well to this goal; and our documentation wasn't helpful on what to
expect.

Note that the previous patch changed all qemu_strtosz() EINVAL error
paths to slam value to 0 rather than stay uninitialized, even when the
EINVAL eror occurs because of trailing junk.  But for the remaining
integral qemu_strto*, it's easier to return the parsed value than to
force things back to zero, in part because of how check_strtox_error
works; in part because people expect that from libc strto* (while
there is no libc strtosz to compare to), and in part because doing so
creates less churn in the testsuite.

Here, the list of affected callers is much longer ('git grep
"qemu_strto[ui]" "*.c" "**/*.c" | grep -v tests/ |wc -l' outputs 107,
although a few of those are the implementation in in cutils.c), so
touching as little as possible is the wisest course of action.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-17-eblake@redhat.com>


  Commit: f7e089f493c1287ee20b1f26ba08d55a858e6855
      
https://github.com/qemu/qemu/commit/f7e089f493c1287ee20b1f26ba08d55a858e6855
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/qemu-iotests/049.out
    M tests/qemu-iotests/178.out.qcow2
    M tests/qemu-iotests/178.out.raw
    M tests/unit/test-cutils.c
    M util/cutils.c

  Log Message:
  -----------
  cutils: Use parse_uint in qemu_strtosz for negative rejection

Rather than open-coding two different ways to check for an unwanted
negative sign, reuse the same code in both functions.  That way, if we
decide down the road to accept "-0" instead of rejecting it, we have
fewer places to change.  Also, it means we now get ERANGE instead of
EINVAL for negative values in qemu_strtosz, which is reasonable for
what it represents.  This in turn changes the expected output of a
couple of iotests.

The change is not quite complete: negative fractional scaled values
can trip us up.  This will be fixed in a later patch addressing other
issues with fractional scaled values.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-18-eblake@redhat.com>


  Commit: 7266a51166c43d85cb3301f504d4493b1f8057dc
      
https://github.com/qemu/qemu/commit/7266a51166c43d85cb3301f504d4493b1f8057dc
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c
    M util/cutils.c

  Log Message:
  -----------
  cutils: Improve qemu_strtod* error paths

Previous patches changed all integral qemu_strto*() error paths to
guarantee that *value is never left uninitialized.  Do likewise for
qemu_strtod.  Also, tighten qemu_strtod_finite() to never return a
non-finite value (prior to this patch, we were rejecting "inf" with
-EINVAL and unspecified result 0.0, but failing "9e999" with -ERANGE
and HUGE_VAL - which is infinite on IEEE machines - despite our
function claiming to recognize only finite values).

Auditing callers, we have no external callers of qemu_strtod, and
among the callers of qemu_strtod_finite:

- qapi/qobject-input-visitor.c:qobject_input_type_number_keyval() and
  qapi/string-input-visitor.c:parse_type_number() which reject all
  errors (does not matter what we store)

- utils/cutils.c:do_strtosz() incorrectly assumes that *endptr points
  to '.' on all failures (that is, it is not distinguishing between
  EINVAL and ERANGE; and therefore still does the WRONG THING for
  "9.9e999".  The change here does not entirely fix that (a later
  patch will tackle this more systematically), but at least it fixes
  the read-out-of-bounds first diagnosed in
  https://gitlab.com/qemu-project/qemu/-/issues/1629

- our testsuite, which we can update to match what we document

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
CC: qemu-stable@nongnu.org
Message-Id: <20230522190441.64278-19-eblake@redhat.com>


  Commit: 58516caac47c4bc7ed3ad6a8e2f565404a563dd3
      
https://github.com/qemu/qemu/commit/58516caac47c4bc7ed3ad6a8e2f565404a563dd3
  Author: Eric Blake <eblake@redhat.com>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M tests/unit/test-cutils.c
    M util/cutils.c

  Log Message:
  -----------
  cutils: Improve qemu_strtosz handling of fractions

We have several limitations and bugs worth fixing; they are
inter-related enough that it is not worth splitting this patch into
smaller pieces:

* ".5k" should work to specify 512, just as "0.5k" does
* "1.9999k" and "1." + "9"*50 + "k" should both produce the same
  result of 2048 after rounding
* "1." + "0"*350 + "1B" should not be treated the same as "1.0B";
  underflow in the fraction should not be lost
* "7.99e99" and "7.99e999" look similar, but our code was doing a
  read-out-of-bounds on the latter because it was not expecting ERANGE
  due to overflow. While we document that scientific notation is not
  supported, and the previous patch actually fixed
  qemu_strtod_finite() to no longer return ERANGE overflows, it is
  easier to pre-filter than to try and determine after the fact if
  strtod() consumed more than we wanted.  Note that this is a
  low-level semantic change (when endptr is not NULL, we can now
  successfully parse with a scale of 'E' and then report trailing
  junk, instead of failing outright with EINVAL); but an earlier
  commit already argued that this is not a high-level semantic change
  since the only caller passing in a non-NULL endptr also checks that
  the tail is whitespace-only.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1629
Fixes: cf923b78 ("utils: Improve qemu_strtosz() to have 64 bits of precision", 
6.0.0)
Fixes: 7625a1ed ("utils: Use fixed-point arithmetic in qemu_strtosz", 6.0.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-20-eblake@redhat.com>
[eblake: tweak function comment for accuracy]


  Commit: 13856a56ccbc5f17b2da93bb69e656b0d7b0bcb8
      
https://github.com/qemu/qemu/commit/13856a56ccbc5f17b2da93bb69e656b0d7b0bcb8
  Author: Richard Henderson <richard.henderson@linaro.org>
  Date:   2023-06-01 (Thu, 01 Jun 2023)

  Changed paths:
    M audio/audio_legacy.c
    M block/gluster.c
    M block/nfs.c
    M blockdev.c
    M contrib/ivshmem-server/main.c
    M docs/interop/qcow2.txt
    M hw/core/numa.c
    M include/qemu/cutils.h
    M qapi/opts-visitor.c
    M tests/qemu-iotests/049.out
    M tests/qemu-iotests/178.out.qcow2
    M tests/qemu-iotests/178.out.raw
    M tests/qemu-iotests/common.filter
    M tests/qemu-iotests/common.rc
    M tests/unit/test-cutils.c
    M ui/vnc.c
    M util/cutils.c
    M util/guest-random.c
    M util/qemu-sockets.c

  Log Message:
  -----------
  Merge tag 'pull-nbd-2023-06-01' of https://repo.or.cz/qemu/ericb into staging

nbd and misc patches for 2023-06-01

- Eric Blake: Fix iotest 104 for NBD
- Eric Blake: Improve qcow2 spec on padding bytes
- Eric Blake: Fix read-beyond-bounds bug in qemu_strtosz

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAmR5FSsACgkQp6FrSiUn
# Q2ocnAf/WcshUe+gwsQF7ROeaiw2QY2m2n9nB/gQHBcqtol81HwBeCjVlZTNdcyz
# AaMxrjbTEXM3ER56ClLuAyptiCFGWaASFWhMvRhPc7LdUGXNvRqNcP5RdASWdCMU
# pnSGYXNRg+D0WA8Gs8KKG+BwRs7JdgO12MrkLP3SlA7Lx7nZF9a2lFLCoZECYtgf
# 9OgRX+Bu3KyJ8uMO331DOLGiEGsmohtxEqzAFKN7fYOZ1k8StGT+UHiksb70Ujnn
# kt53hFjXSGej3JlGLJO1J9dNwZ/qiebwxNYOx1lybKRzrS9MFYT1spmZB2WcwNJx
# Yh5pO/hsTh8K58MpUTY/5XfrrOvmeQ==
# =6Bex
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 01 Jun 2023 03:01:15 PM PDT
# gpg:                using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>" [full]
# gpg:                 aka "Eric Blake (Free Software Programmer) 
<ebb9@byu.net>" [full]
# gpg:                 aka "[jpeg image of size 6874]" [full]

* tag 'pull-nbd-2023-06-01' of https://repo.or.cz/qemu/ericb: (21 commits)
  cutils: Improve qemu_strtosz handling of fractions
  cutils: Improve qemu_strtod* error paths
  cutils: Use parse_uint in qemu_strtosz for negative rejection
  cutils: Set value in all integral qemu_strto* error paths
  cutils: Set value in all qemu_strtosz* error paths
  test-cutils: Add more coverage to qemu_strtosz
  numa: Check for qemu_strtosz_MiB error
  cutils: Allow NULL str in qemu_strtosz
  test-cutils: Refactor qemu_strtosz tests for less boilerplate
  test-cutils: Prepare for upcoming semantic change in qemu_strtosz
  test-cutils: Add coverage of qemu_strtod
  cutils: Allow NULL endptr in parse_uint()
  cutils: Adjust signature of parse_uint[_full]
  cutils: Document differences between parse_uint and qemu_strtou64
  cutils: Fix wraparound parsing in qemu_strtoui
  test-cutils: Test more integer corner cases
  test-cutils: Test integral qemu_strto* value on failures
  test-cutils: Use g_assert_cmpuint where appropriate
  test-cutils: Avoid g_assert in unit tests
  qcow2: Explicit mention of padding bytes
  ...

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Compare: https://github.com/qemu/qemu/compare/9eb400cdd7b0...13856a56ccbc