Re: [Qemu-devel] [PATCH v2 2/9] cutils: Fix qemu_strtosz() & friends to reject non-finite sizes

[David Hildenbrand <david@redhat.com>]

On 20.11.18 21:41, Eric Blake wrote:
> On 11/20/18 2:31 PM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> 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 <david@redhat.com>
>>>> ---
>>>>    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"?
> 
> "or" (ouch - wrong time for my fingers to be slipping on the keyboard)

Shouldn't that be "and" and s/Return/Returns/


"Returns -ERANGE on overflow and -EINVAL on other errors".

I can include that fixup (whetever version you guys prefer)

> 
> 
>>> 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 <eblake@redhat.com>
>>
>> Thanks for your analysis, Eric.
>>
>> With the typo fixes:
> 
> Including the fix of my attempt at a typo fix :)
> 
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks!

-- 

Thanks,

David / dhildenb