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

[Markus Armbruster <armbru@redhat.com>]

Tao Xu <tao3.xu@intel.com> writes:

> On 12/19/2019 2:26 AM, Markus Armbruster wrote:
>> Tao Xu <tao3.xu@intel.com> writes:
>>
>>> On 12/18/2019 9:33 AM, Tao Xu wrote:
>>>> On 12/17/2019 6:25 PM, Markus Armbruster wrote:
>> [...]
>>>>> Also fun: for "0123", we use uint64_t 83, not double 123.0.  But for
>>>>> "0123.", we use 123.0, not 83.
>>>>>
>>>>> Do we really want to accept octal and hexadecimal integers?
>>>>>
>>>>
>>>> Thank you for reminding me. Octal and hexadecimal may bring more
>>>> confusion. I will use qemu_strtou64(nptr, &suffixu, 10, &valu) and
>>>> add test for input like "0123".
>>>>
>>>
>>> Hi Markus,
>>>
>>> After I use qemu_strtou64(nptr, &suffixu, 10, &valu), it cause another
>>> question. Because qemu_strtod_finite support hexadecimal input, so in
>>> this situation, it will parsed as double. It will also let large
>>> hexadecimal integers be rounded. So there may be two solution:
>>>
>>> 1: use qemu_strtou64(nptr, &suffixu, 0, &valu) and parse octal as
>>> decimal. This will keep hexadecimal valid as now.
>>>
>>> "0123" --> 123; "0x123" --> 291
>>
>> How would you make qemu_strtou64() parse octal as decimal?
>
> How about this solution, set @base as variable, if we detect
> hexadecimal, we use 0, then can prase decimal as u64, else we use 10,
> then can prase octal as decimal, because 0 prefix will be ignored in
> qemu_strtou64(nptr, &suffixu, 10, &valu);
>
>     const char *p = nptr;
>     while (qemu_isspace(*p)) {
>        p++;
>     }
>     if (*p == '0' && (qemu_toupper(*(p+1)) == 'X' ||) {
>         base = 0;
>     } else {
>         base = 10;
>     }
>
>     retd = qemu_strtod_finite(nptr, &suffixd, &vald);
>     retu = qemu_strtou64(nptr, &suffixu, base, &valu);
>     use_strtod = strlen(suffixd) < strlen(suffixu);
>
>     if (use_strtod) {
>         endptr = suffixd;
>         retval = retd;
>     } else {
>         endptr = suffixu;
>         retval = retu;
>     }

You skip whitespace, but neglect to skip the sign.

Peeking into the input to predict what qemu_strtou64() will do feels
unadvisable.

We could try both base 10 and 16, and use whatever consumes more
characters, but that's even more complicated.

This function's contract is *terrible*.  We've tried to improve on it,
but so far only managed to make it more complex and differently
terrible.

Do we really, really, really need full precision?

If no, let's flee this swamp without looking back.

If yes, what's the *simplest* solution that provides it?

>>> 2: use qemu_strtou64(nptr, &suffixu, 10, &valu) and reject octal and
>>> decimal.
>>>
>>> "0123" --> Error; "0x123" --> Error
>>
>> How would you reject the 0x prefix?
>>
> How about check the first&second character is '0' and 'x' and then
> return -EINVAL.