Re: [Qemu-devel] [PATCH v2 1/9] cutils: Add qemu_strtod() and qemu_strtod_finite()

[Eric Blake]

On 11/20/18 3:25 AM, David Hildenbrand wrote:
> Let's provide a wrapper for strtod().
>
> Reviewed-by: Eric Blake <address@hidden>

This changed enough from v1 that I would have dropped R-b to ensure that 
reviewers notice the differences.

> Signed-off-by: David Hildenbrand <address@hidden>
> ---
>   include/qemu/cutils.h |  2 ++
>   util/cutils.c         | 65 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 67 insertions(+)

> + * If the conversion overflows, store +/-HUGE_VAL in @result, depending
> + * on the sign, and return -ERANGE.
> + *
> + * If the conversion underflows, store ±0.0 in @result, depending on the
> + * sign, and return -ERANGE.

The use of UTF-8 ± in one place but not both is odd.  I think we're at 
the point where UTF-8 comments are acceptable these days, rather than 
trying to keep our codebase ASCII-clean, so I don't care which way you 
resolve the inconsistency.

> +/**
> + * Convert string @nptr to a finite double.
> + *
> + * Works like qemu_strtod(), except that "NaN" and "inf" are rejected
> + * with -EINVAL and no conversion is performed.
> + */
> +int qemu_strtod_finite(const char *nptr, const char **endptr, double *result)
> +{
> +    double tmp;
> +    int ret;
> +
> +    ret = qemu_strtod(nptr, endptr, &tmp);
> +    if (ret) {
> +        return ret;

So, if we overflow, we are returning -ERANGE but with nothing stored 
into *result.  This is different from qemu_strtod(), where a return of 
-ERANGE guarantees that *result is one of 4 values (+/- 0.0/inf).  That 
seems awkward.

> +    } else if (!isfinite(tmp)) {
> +        if (endptr) {
> +            *endptr = nptr;
> +        }
> +        return -EINVAL;

Rewinding back to the start of "inf" is interesting, but matches your 
documentation.

> +    }
> +
> +    *result = tmp;
> +    return ret;
> +}
> +

I think you still need to fix -ERANGE handling before I can give R-b.

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