Re: [PATCH 1/3] utils: Improve qemu_strtosz() to have 64 bits of precision

[Eric Blake <eblake@redhat.com>]

On 2/5/21 5:02 AM, Daniel P. Berrangé wrote:

>>  /*
>> - * Convert string to bytes, allowing either B/b for bytes, K/k for KB,
>> - * M/m for MB, G/g for GB or T/t for TB. End pointer will be returned
>> - * in *end, if not NULL. Return -ERANGE on overflow, and -EINVAL on
>> - * other error.
>> + * Convert size string to bytes.
>> + *
>> + * Allow either B/b for bytes, K/k for KB, M/m for MB, G/g for GB or
>> + * T/t for TB, with scaling based on @unit, and with @default_suffix
>> + * implied if no explicit suffix was given.
>> + *
>> + * The end pointer will be returned in *end, if not NULL.  If there is
>> + * no fraction, the input can be decimal or hexadecimal; if there is a
>> + * fraction, then the input must be decimal and there must be a suffix
>> + * (possibly by @default_suffix) larger than Byte, and the fractional
>> + * portion may suffer from precision loss or rounding.  The input must
>> + * be positive.
> 
> Even though the test suite gives some illustrations, I think we should
> document here the patterns we're intending to support. IIUC, we aim for
> 
> [quote]
> The size parsing supports the following syntaxes
> 
>  - 12345   - decimal, bytes
>  - 12345{bBkKmMgGtT} - decimal, scaled bytes

Yes.  Actually 12345{bBkKmMgGtTpPeE}, as long as it doesn't overflow 16E.

>  - 12345.678 - fractional decimal, bytes

No - this was already rejected prior to my patch, and my patch keeps it
as rejected (that's the whole mul_required bool, which checks whether
mul > 1).

>  - 12345.678{bBkKmMgGtT} - fractional decimal, scaled bytes

Close; we reject bB in this situation for the same reason that we reject
fractional decimal without suffix.  Also, patch 3/3 was questioning
whether the fractional portion must be exact or is permitted to round

>  - 0x7FEE  - hex, bytes

Yes, and with the additional note that 'E' and 'B' are treated as hex
digits, not scale suffixes, in this form.

> 
> The following are intentionally not supported
> 
>  - octal

Never has worked.

>  - fractional hex

worked by accident, dropping it in this patch is not deemed worth a
deprecation period.

>  - floating point exponents

worked by accident, dropping it in this patch is not deemed worth a
deprecation period.

> [/quote]

and with one more form:

patch 2/3
 - 0x123abc{kKmMgGtTpP} - hex, scaled bytes, with limited set of working
suffixes, and slated for future removal (this one is barely plausible
enough that we need the deprecation period to see who uses it)

At any rate, I like the idea of being more explicit in the function
description, so I will enhance v2 along these lines.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org