[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/9] cutils: Fix qemu_strtosz() & friends to
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/9] cutils: Fix qemu_strtosz() & friends to reject non-finite sizes |
Date: |
Tue, 20 Nov 2018 21:31:29 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 11/20/18 3:25 AM, David Hildenbrand wrote:
>> qemu_strtosz() & friends reject NaNs, but happily accept inifities.
>
> s/inifities/infinities/
>
>> They shouldn't. Fix that.
>>
>> The fix makes use of qemu_strtod_finite(). To avoid ugly casts,
>> change the @end parameter of qemu_strtosz() & friends from char **
>> to const char **.
>>
>> Also, add two test cases, testing that "inf" and "NaN" are properly
>> rejected.
>>
>> Signed-off-by: David Hildenbrand <address@hidden>
>> ---
>> include/qemu/cutils.h | 6 +++---
>> monitor.c | 2 +-
>> tests/test-cutils.c | 24 +++++++++++++++++-------
>> util/cutils.c | 16 +++++++---------
>> 4 files changed, 28 insertions(+), 20 deletions(-)
>>
>
>> +++ b/util/cutils.c
>> @@ -206,20 +206,18 @@ static int64_t suffix_mul(char suffix, int64_t unit)
>> * in *end, if not NULL. Return -ERANGE on overflow, Return -EINVAL on
>
> Pre-existing, but since you're touching this area: the second 'Return'
> is unusual capitalization for being mid-sentence. You could even
> s/Return/of/
"of"?
>
>> * other error.
>> */
>> -static int do_strtosz(const char *nptr, char **end,
>> +static int do_strtosz(const char *nptr, const char **end,
>> const char default_suffix, int64_t unit,
>> uint64_t *result)
>> {
>> int retval;
>> - char *endptr;
>> + const char *endptr;
>> unsigned char c;
>> int mul_required = 0;
>> double val, mul, integral, fraction;
>> - errno = 0;
>> - val = strtod(nptr, &endptr);
>> - if (isnan(val) || endptr == nptr || errno != 0) {
>> - retval = -EINVAL;
>> + retval = qemu_strtod_finite(nptr, &endptr, &val);
>> + if (retval) {
>> goto out;
>
> Here, retval can be -EINVAL (for failure to parse, or encountering
> "inf" or "NaN") or -ERANGE (overflow, underflow)...
>
>> }
>> fraction = modf(val, &integral);
>> @@ -259,17 +257,17 @@ out:
>
> out:
> if (end) {
> *end = endptr;
> } else if (*endptr) {
> retval = -EINVAL;
> }
>
>> return retval;
>
> ...if the failure was -EINVAL due to trailing garbage or empty string,
> nothing changes. If the failure was -EINVAL due to "inf", and the user
> passed in 'end', then 'end' now points to the beginning of "inf"
> instead of the end (probably okay). If the failure was -EINVAL due to
> "inf" and the user gave NULL for 'end', then we slam retval back to
> -EINVAL (no change). If the failure was -ERANGE, then there is no
> trailing garbage, so *endptr had better be NULL, and we still fail
> with -ERANGE. Any other way to reach the out label is unchanged from
> earlier logic.
>
> It's some hairy code to think about, but I can't find anything wrong
> with it. Typo fixes are minor, so
>
> Reviewed-by: Eric Blake <address@hidden>
Thanks for your analysis, Eric.
With the typo fixes:
Reviewed-by: Markus Armbruster <address@hidden>
- [Qemu-devel] [PATCH v2 4/9] qapi: Use qemu_strtod_finite() in qobject-input-visitor, (continued)
- [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
- Re: [Qemu-devel] [PATCH v2 5/9] test-string-input-visitor: Add more tests, Eric Blake, 2018/11/20
- Re: [Qemu-devel] [PATCH v2 5/9] test-string-input-visitor: Add more tests, Eric Blake, 2018/11/20
- Re: [Qemu-devel] [PATCH v2 5/9] test-string-input-visitor: Add more tests, Eric Blake, 2018/11/20
- Re: [Qemu-devel] [PATCH v2 5/9] test-string-input-visitor: Add more tests, Markus Armbruster, 2018/11/20
- Re: [Qemu-devel] [PATCH v2 5/9] test-string-input-visitor: Add more tests, David Hildenbrand, 2018/11/21
- Re: [Qemu-devel] [PATCH v2 5/9] test-string-input-visitor: Add more tests, Markus Armbruster, 2018/11/21
[Qemu-devel] [PATCH v2 2/9] cutils: Fix qemu_strtosz() & friends to reject non-finite sizes, David Hildenbrand, 2018/11/20
[Qemu-devel] [PATCH v2 7/9] test-string-input-visitor: Use virtual walk, David Hildenbrand, 2018/11/20
[Qemu-devel] [PATCH v2 6/9] qapi: Rewrite string-input-visitor, David Hildenbrand, 2018/11/20