Let's provide a wrapper for strtod().

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 include/qemu/cutils.h |  2 ++
 util/cutils.c         | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 7071bfe2d4..756b41c193 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -146,6 +146,8 @@ int qemu_strtoi64(const char *nptr, const char **endptr, int base,
                   int64_t *result);
 int qemu_strtou64(const char *nptr, const char **endptr, int base,
                   uint64_t *result);
+int qemu_strtod(const char *nptr, const char **endptr, double *result);
+int qemu_strtod_finite(const char *nptr, const char **endptr, double *result);
 
 int parse_uint(const char *s, unsigned long long *value, char **endptr,
                int base);
diff --git a/util/cutils.c b/util/cutils.c
index 698bd315bd..7868a683e8 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -544,6 +544,44 @@ int qemu_strtou64(const char *nptr, const char **endptr, int base,
     return check_strtox_error(nptr, ep, endptr, errno);
 }
 
+/**
+ * Convert string @nptr to a double.
+ *
+ * Works like qemu_strtoul(), except it stores +/-HUGE_VAL on
+ * overflow/underflow.
+ */
+int qemu_strtod(const char *nptr, const char **endptr, double *result)
+{
+    char *ep;
+
+    if (!nptr) {
+        if (endptr) {
+            *endptr = nptr;
+        }
+        return -EINVAL;
+    }
+
+    errno = 0;
+    *result = strtod(nptr, &ep);
+    return check_strtox_error(nptr, ep, endptr, errno);
+}
+
+/**
+ * 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.
+ */
+int qemu_strtod_finite(const char *nptr, const char **endptr, double *result)
+{
+    int ret = qemu_strtod(nptr, endptr, result);
+
+    if (!ret && !isfinite(*result)) {
+        return -EINVAL;
+    }
+    return ret;
+}
+
 /**
  * Searches for the first occurrence of 'c' in 's', and returns a pointer
  * to the trailing null byte if none was found.
-- 
2.17.2

On 11/15/18 8:04 AM, David Hildenbrand wrote:
> Let's provide a wrapper for strtod().
> 
> Signed-off-by: David Hildenbrand <david@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/

> + */
> +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).

> +
> +    if (!ret && !isfinite(*result)) {
> +        return -EINVAL;
> +    }

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.

With the typo fix,

Reviewed-by: Eric Blake <eblake@redhat.com>

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

Eric Blake <eblake@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 <david@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.

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.

>> + */
>> +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.

> 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.

> With the typo fix,
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

On 15.11.18 17:22, Markus Armbruster wrote:
> Eric Blake <eblake@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 <david@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 <eblake@redhat.com>

Thanks!

-- 

Thanks,

David / dhildenb

On 11/15/18 11:25 AM, David Hildenbrand wrote:

>> 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.
> 

>> 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.

Fails to mention underflow. Maybe add:

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

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

On 15.11.18 19:02, Eric Blake wrote:
> If the conversion underflows, store ±0.0 in @result, depending on the 
> sign, and return -ERANGE.

Will do! Thanks!

-- 

Thanks,

David / dhildenb