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
