Re: [PATCH v3 04/19] test-cutils: Test more integer corner cases

From: Eric Blake <eblake@redhat.com>
To: Hanna Czenczek <hreitz@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [PATCH v3 04/19] test-cutils: Test more integer corner cases
Date: Tue, 23 May 2023 14:19:07 -0500	[thread overview]
Message-ID: <dl65ot7lh2hstviwk2yltgun2jgnz2vypvgoid6d3zstku52xf@trj4rhrs7qri> (raw)
In-Reply-To: <5720a801-a6b7-1b9e-efdb-bceb7e55c6ce@redhat.com>

On Tue, May 23, 2023 at 07:25:28PM +0200, Hanna Czenczek wrote:
> On 22.05.23 21:04, Eric Blake wrote:
> > 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.
> > 
> 
> > +    if (LONG_MAX == INT_MAX) {
> > +        str = "0xffffffff00000001"; /* ULLONG_MAX - UNIT_MAX + 1 (not 1) */
> 
> s/UNIT/UINT/

Yep, and this comment is new to v3.

> 
> > +        endptr = "somewhere";
> > +        res = 999;
> > +        err = qemu_strtol(str, &endptr, 0, &res);
> > +        g_assert_cmpint(err, ==, -ERANGE);
> > +        g_assert_cmpint(res, ==, LONG_MIN);
> > +        g_assert_true(endptr == str + strlen(str));
> > +    }
> 
> [...]
> 
> > @@ -1325,31 +1799,87 @@ static void test_qemu_strtoul_max(void)
> 
> [...]
> 
> >   static void test_qemu_strtoul_underflow(void)
> >   {
> > -    const char *str = "-99999999999999999999999999999999999999999999";
> > -    char f = 'X';
> > -    const char *endptr = &f;
> > -    unsigned long res = 999;
> > +    const char *str;
> > +    const char *endptr;
> > +    unsigned long res;
> >       int err;
> > 
> > +    /* 1 less than -ULONG_MAX */
> > +    str = ULONG_MAX == UINT_MAX ? "-4294967296" : "-18446744073709551616";
> > +    endptr = "somewhere";
> > +    res = 999;
> >       err = qemu_strtoul(str, &endptr, 0, &res);
> > +    g_assert_cmpint(err, ==, -ERANGE);
> > +    g_assert_cmpint(res, ==, ULONG_MAX);
> 
> Should be g_assert_cmpuint().

Looks like I missed one of the spots, even though I did get most of
them from your same comment on v2.

I can fix both things before applying, if it goes through my tree, or
trust that whoever else queues it can do likewise.

> 
> Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org