David Hildenbrand <da...@redhat.com> writes:
> On 20.11.18 21:07, Markus Armbruster wrote:
>> Eric Blake <ebl...@redhat.com> writes:
>>
>>> On 11/20/18 3:25 AM, David Hildenbrand wrote:
>>>> Let's provide a wrapper for strtod().
>>>>
>>>> Reviewed-by: Eric Blake <ebl...@redhat.com>
>>>
>>> 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 <da...@redhat.com>
>>>> ---
>>>> 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;
> }
With these changes:
Reviewed-by: Markus Armbruster <arm...@redhat.com>
