[Top][All Lists]

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

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

From: Eric Blake
Subject: Re: [PATCH 2/3] utils: Deprecate hex-with-suffix sizes
Date: Fri, 5 Feb 2021 07:38:12 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0

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@redhat.com>
>> ---

>> +++ 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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]