On 15.11.18 17:22, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
>> On 11/15/18 8:04 AM, David Hildenbrand wrote:
>>> Let's provide a wrapper for strtod().
>>>
>>> Signed-off-by: David Hildenbrand <da...@redhat.com>
>>> ---
>>> include/qemu/cutils.h | 2 ++
>>> util/cutils.c | 38 ++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 40 insertions(+)
>>>
>>
>>
>>> +
>>> +/**
>>> + * Convert string @nptr to a finite double.
>>> + *
>>> + * Works like qemu_strtoul(), except it stores +/-HUGE_VAL on
>>> + * overflow/underflow. "NaN" or "inf" are rejcted with -EINVAL.
>>
>> s/rejcted/rejected/
>
> Also, just overflow. Floating-point underflow is when a computation's
> mathematical result is too close to zero to be represented without
> extraordinary rounding error.
Indeed, as the "man strod" states
"... would cause overflow, plus or minus HUGE_VAL (HUGE_VALF, HUGE_VALL)
is returned (according to the sign of the value)"
>
> Skip this paragraph unless you're ready to nerd out. IEEE 754 section
> 7.5 defines underflow to happen
>
> [...] either
>
> a) after rounding — when a non-zero result computed as though the
> exponent range were unbounded would lie strictly between ±b^emin,
> or
>
> b) before rounding — when a non-zero result computed as though both
> the exponent range and the precision were unbounded would lie
> strictly between ±b^emin.
>
> where b^emin is the smallest normal number.
>
> The "Works like qemu_strtoul()" is a bit lazy. I guess it works like
> qemu_strtoul() in the sense that it adds to strtod() what qemu_strtoul()
> adds to strtoul(). I consciously didn't take a similar shortcut in
> commit 4295f879bec: I documented both qemu_strtol() and qemu_strtoul()
> in longhand, and used "Works like" shorthand only where that's actually
> the case: qemu_strtoll() works like qemu_strtol(), and qemu_strtoull()
> works like qemu_strtoul(). I'd prefer longhand for qemu_strtod(). It
> costs us a few lines, but it results in a clearer contract.
/**
* Convert string @nptr to a double.
*
* This is a wrapper around strtod() that is harder to misuse.
* Semantics of @nptr and @endptr match strtod() with differences
* noted below.
*
* @nptr may be null, and no conversion is performed then.
*
* If no conversion is performed, store @nptr in *@endptr and return
* -EINVAL.
*
* If @endptr is null, and the string isn't fully converted, return
* -EINVAL. This is the case when the pointer that would be stored in
* a non-null @endptr points to a character other than '\0'.
*
* If the conversion overflows @result, store +/-HUGE_VAL, depending on
* the sign, in @result and return -ERANGE.
*
* Else store the converted value in @result, and return zero.
*/
>
>>> + */
>>> +int qemu_strtod_finite(const char *nptr, const char **endptr, double
>>> *result)
>>> +{
>>> + int ret = qemu_strtod(nptr, endptr, result);
>>
>> On overflow, result is set to HUGE_VAL (aka "inf") with ret set to
>> -ERANGE. (The C standard uses HUGE_VAL rather than directly requiring
>> infinity on overflow, in order to cater to museum platforms where the
>> largest representable double is still finite; but no one develops qemu
>> on a non-IEEE machine these days so we know that HUGE_VAL == INF).
>
> Aside: museum clauses like this one make the standard much harder to
> read than necessary. I wish they'll purge them from C2X.
>
>>> +
>>> + if (!ret && !isfinite(*result)) {
>>> + return -EINVAL;
>>> + }
>
> qemu_strtol() & friends leave *result alone when they return -EINVAL.
> This one doesn't. Unlikely to hurt anyone, but I'd prefer to keep them
> consistent.
Will use a temporary and also properly set endptr in case we return
-EINVAL; And add a fully-blown description as noted above :)
>
>> This check means that overflow ("1e9999") fails with -ERANGE, while
>> actual infinity ("inf") fails with -EINVAL, letting the user
>> distinguish between the two. Still, I wonder if assigning a
>> non-finite value into result on -ERANGE is the wisest course of
>> action. We'll just have to see in the next patches that use this.
>
> I guess it's about as "wise" as qemu_strtol() storing LONG_MAX on
> integer overflow.
>
> I'm fine with the semantics David picked, as long as they're spelled out
> in the function contract.
I think for now we're fine treating explicit "infinity" user input as
-EINVAL. We could return something like "HUGE_VAL - 1" along with
-ERANGE, but I guess for now this is overkill. Most callers will bail
out on -ERANGE either way. And if not, they have to make sure they can
deal with HUGE_VAL.
>
>> With the typo fix,
>>
>> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
--
Thanks,
David / dhildenb
