Re: [PATCH 2/3] utils: Deprecate hex-with-suffix sizes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/5/21 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
> 04.02.2021 22:07, Eric Blake wrote:
>> Supporting '0x20M' looks odd, particularly since we have an 'E' suffix
> 
> What about also deprecating 'E' suffix? (just my problem of reviewing
> previous patch)

No, we want to keep '1E' as a valid way to spell 1 exabyte.  That has
uses in the wild (admittedly corner-case, as that is a LOT of storage).
It is only '0x1E' where the use of 'E' as a hex digit has priority over
'E' as a suffix meaning exabyte.

> 
>> that is ambiguous between a hex digit and the extremely large exibyte
>> suffix, as well as a 'B' suffix for bytes.  In practice, people using
>> hex inputs are specifying values in bytes (and would have written
>> 0x2000000, or possibly relied on default_suffix in the case of
>> qemu_strtosz_MiB), and the use of scaling suffixes makes the most
>> sense for inputs in decimal (where the user would write 32M).  But
>> rather than outright dropping support for hex-with-suffix, let's
>> follow our deprecation policy.  Sadly, since qemu_strtosz() does not
>> have an Err** parameter, we pollute to stderr.
>>
>> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
>> ---

>> +++ b/util/cutils.c
>> @@ -264,7 +264,7 @@ static int do_strtosz(const char *nptr, const char
>> **end,
>>       int retval;
>>       const char *endptr;
>>       unsigned char c;
>> -    bool mul_required = false;
>> +    bool mul_required = false, hex = false;
>>       uint64_t val;
>>       int64_t mul;
>>       double fraction = 0.0;
>> @@ -309,6 +309,10 @@ static int do_strtosz(const char *nptr, const
>> char **end,
> 
> you forget to set hex to true in corresponding if(){...}
> 
>>       c = *endptr;
>>       mul = suffix_mul(c, unit);
>>       if (mul > 0) {
>> +        if (hex) {
>> +            fprintf(stderr, "Using a multiplier suffix on hex numbers "
>> +                    "is deprecated: %s\n", nptr);
>> +        }

D'oh.  Now I get to rerun my tests to see when the warning triggers.

>>           endptr++;
>>       } else {
>>           mul = suffix_mul(default_suffix, unit);
> 
> should we also deprecate hex where default_suffix is not 'B' ?

That's exactly what this patch is (supposed to be) doing.  If we parsed
a hex number, and there was an explicit suffix at all (which is
necessarily neither 'B' nor 'E', since those were already consumed while
parsing the hex number), issue a warning.

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux