On Sat, Jun 03, 2023 at 11:17:27AM +0300, Michael Tokarev wrote:
> 02.06.2023 01:02, Eric Blake пишет:
> > 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 <ebl...@redhat.com>
> > CC: qemu-sta...@nongnu.org
>
> Trying to pick this one up for -stable. The implementation changes are good.
> But the testsuite changes are.. difficult. The thing is that testsuite
> changes
> (here and in the other -stable patch) applies on top of previous changes in
> there (in the same series), which, in turn, requires other previous code
> changes
> in the implementation to succeed.
Yeah, I did wonder if it is worth even trying to get this for stable.
The series is inter-tangled enough that it feels like an
all-or-nothing approach may be easiest - but all means 19 patches and
hundreds of lines of testsuite additions. My other argument when
first posting this to qemu-stable was to just declare that these have
been broken long enough that it is not a recent regression, and users
are unlikely to be supplying command-line or QMP strings to tickle
these bugs, so not backporting may be okay.
>
> For example, this patch changes test_qemu_strtoui_overflow() which was
> introduced
> in previous patch "Test more integer corner cases" from this series and
> further
> modified in "Test integral qemu_strto* value on failures" one. Picking them
> result
> in testsuite failing due to missing previous code changes.
>
> I tried to drop just the testsuite changes, but the result is that the
> testsuite
> fails with fixed code :)
>
> It's quite fun situation actually, like it fails no matter what you do, one
> way
> or another.
>
> I'll try to find the most stright-forward way from here. Good stuff.
Let me know how I can help in deciding what, if any, is worth backporting.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
