Re: [PATCH v2 09/19] test-cutils: Add coverage of qemu_strtod

[Eric Blake]

On Mon, May 22, 2023 at 12:56:31PM +0200, Hanna Czenczek wrote:
> > > > +static void test_qemu_strtod_erange_junk(void)
> > > > +{
> > > > +    const char *str;
> > > > +    const char *endptr;
> > > > +    int err;
> > > > +    double res;
> > > > +
> > > > +    /* EINVAL has priority over ERANGE */
> > > By being placed here, this comment confused me a bit, because the first 
> > > case
> > > does return ERANGE.  So I’d prefer it above the second case, where we
> > > actually expect EINVAL, but understand that’s a personal preference.  
> > > (Same
> > > for the _finite_ variant)
> > The test is what happens when both conditions apply.  For
> > qemu_strtod("1e-999junk", &endptr), only ERANGE applies (because
> > "junk" is returned in endptr); it is not until
> > qemu_strtod("1e-999junk", NULL) where EINVAL is also possible
> > (trailing junk takes precedence over underflow).
> 
> Yep; it’s just that because the comment is directly above one test case, I
> assumed it applied to just that case, and was looking for the EINVAL there. 
> Only then I realized that EINVAL won’t occur there, and the comment instead
> points out the difference between the two cases there are.
> 
> > For qemu_strtosz(),
> > I made it a bit more obvious by writing a helper function that shows
> > both errno values in a single line, rather than spreading out the
> > boilerplate over multiple lines.
> > 
> > Should I do a similar helper function for qemu_strtod[_finite] in v3?
> 
> I mean, from my perspective, all I can see is that it would make reviewing
> v3 more tedious…

Okay, v3 will NOT include a helper function for strtoi or strtod (but
the helper already in place for strtosz remains).

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