[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/9] cutils: Add qemu_strtod() and qemu_strto
From: |
David Hildenbrand |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/9] cutils: Add qemu_strtod() and qemu_strtod_finite() |
Date: |
Wed, 21 Nov 2018 11:35:53 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 |
On 20.11.18 21:07, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> 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.
Indeed, dropping it now ;)
>>
>>> 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.
>
> 217 out of 6455 git-controlled files contain non-ASCII characters. 53
> of them are binary, and don't count. In most text files, it's for
> spelling names of authors properly in comments. Ample precedence for
> UTF-8 in comments, I'd say.
>
> That said, I second Eric's call for consistency, with the slightest of
> preferrences for plain ASCII.
I'll just go with +/-. Thanks.
>
> I spotted UTF-8 in two error messages, which might still be unadvisable:
>
> hw/misc/tmp105.c: error_setg(errp, "value %" PRId64 ".%03" PRIu64 " °C
> is out of range",
> hw/misc/tmp421.c: error_setg(errp, "value %" PRId64 ".%03" PRIu64 " °C
> is out of range",
>
>>> +/**
>>> + * 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.
>
> Violates the contract's "like qemu_strtod()".
Right, I missed that. What about something like this:
int qemu_strtod_finite(const char *nptr, const char **endptr, double
*result)
{
double tmp;
int ret;
ret = qemu_strtod(nptr, endptr, &tmp);
if (!ret && !isfinite(tmp)) {
if (endptr) {
*endptr = nptr;
}
ret = -EINVAL;
}
if (ret != -EINVAL) {
*result = tmp;
}
return ret;
}
--
Thanks,
David / dhildenb
[Qemu-devel] [PATCH v2 3/9] qapi: Fix string-input-visitor to reject NaN and infinities, David Hildenbrand, 2018/11/20
[Qemu-devel] [PATCH v2 4/9] qapi: Use qemu_strtod_finite() in qobject-input-visitor, David Hildenbrand, 2018/11/20
[Qemu-devel] [PATCH v2 5/9] test-string-input-visitor: Add more tests, David Hildenbrand, 2018/11/20