Re: [PATCH RESEND v2] util/cutils: Expand do_strtosz parsing precision to 64 bits

[Tao Xu <tao3.xu@intel.com>]

On 12/17/2019 7:44 PM, Christophe de Dinechin wrote:
> 
> 
>> On 9 Dec 2019, at 09:30, Tao Xu <tao3.xu@intel.com> wrote:
>>
>> Parse input string both as a double and as a uint64_t, then use the
>> method which consumes more characters. Update the related test cases.
>>
>> Signed-off-by: Tao Xu <tao3.xu@intel.com>
>> ---
>>
>> Changes in v2:
>>     - Resend to use double small than DBL_MIN
>>     - Add more test case for double overflow and underflow.
>>     - Set mul as int64_t (Markus)
>>     - Restore endptr (Markus)
>> ---
>> tests/test-cutils.c    | 37 +++++++----------------
>> tests/test-keyval.c    | 47 +++++------------------------
>> tests/test-qemu-opts.c | 39 +++++-------------------
>> util/cutils.c          | 67 +++++++++++++++++++++++++++++++-----------
>> 4 files changed, 75 insertions(+), 115 deletions(-)
>>
[...]
>> +    /*
>> +     * Parse @nptr both as a double and as a uint64_t, then use the method
>> +     * which consumes more characters.
>> +     */
> 
> Why do ever need to parse as double if you have uint64?
> 

Because we want to keep do_strtosz Compatible with double input (such as 
1.5k).
>> +    retd = qemu_strtod_finite(nptr, &suffixd, &vald);
>> +    retu = qemu_strtou64(nptr, &suffixu, 0, &valu);
>> +    use_strtod = strlen(suffixd) < strlen(suffixu);
> 
> You could simply compare suffixd and suffixu:
> 
> use_strtod = suffixd > suffixu;
> 

Thank you for your suggestion.
>> +
>> +    if (use_strtod) {
>> +        endptr = suffixd;
>> +        retval = retd;
>> +    } else {
>> +        endptr = suffixu;
>> +        retval = retu;
>> +    }
>>
>> -    retval = qemu_strtod_finite(nptr, &endptr, &val);
>>      if (retval) {
>>          goto out;
>>      }
>> -    fraction = modf(val, &integral);
>> -    if (fraction != 0) {
>> -        mul_required = 1;
>> +    if (use_strtod) {
>> +        fraction = modf(vald, &integral);
>> +        if (fraction != 0) {
>> +            mul_required = 1;
>> +        }
>>      }
>>      c = *endptr;
>>      mul = suffix_mul(c, unit);
>> @@ -238,17 +258,30 @@ static int do_strtosz(const char *nptr, const char **end,
>>          retval = -EINVAL;
>>          goto out;
>>      }
>> -    /*
>> -     * Values near UINT64_MAX overflow to 2**64 when converting to double
>> -     * precision.  Compare against the maximum representable double precision
>> -     * value below 2**64, computed as "the next value after 2**64 (0x1p64) in
>> -     * the direction of 0".
>> -     */
>> -    if ((val * mul > nextafter(0x1p64, 0)) || val < 0) {
>> -        retval = -ERANGE;
>> -        goto out;
>> +
>> +    if (use_strtod) {
>> +        /*
>> +         * Values near UINT64_MAX overflow to 2**64 when converting to double
>> +         * precision. Compare against the maximum representable double precision
>> +         * value below 2**64, computed as "the next value after 2**64 (0x1p64)
>> +         * in the direction of 0".
>> +         */
>> +        if ((vald * mul > nextafter(0x1p64, 0)) || vald < 0) {
>> +            retval = -ERANGE;
>> +            goto out;
>> +        }
>> +        *result = vald * mul;
>> +    } else {
>> +        /* Reject negative input and overflow output */
>> +        while (qemu_isspace(*nptr)) {
>> +            nptr++;
>> +        }
>> +        if (*nptr == '-' || UINT64_MAX / mul < valu) {
>> +            retval = -ERANGE;
>> +            goto out;
>> +        }
>> +        *result = valu * mul;
>>      }
>> -    *result = val * mul;
>>      retval = 0;
>>
>> out:
>> -- 
>> 2.20.1
>>
>>
>