[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 24/24] QemuOpts: Fix checking of sizes for overf
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 24/24] QemuOpts: Fix checking of sizes for overflow and trailing crap |
Date: |
Sat, 18 Feb 2017 11:46:21 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 02/14/2017 04:26 AM, Markus Armbruster wrote:
>> parse_option_size()'s checking for overflow and trailing crap is
>> wrong. Has always been that way. qemu_strtosz() gets it right, so
>> use that.
>>
>> This adds support for size suffixes 'P', 'E', and ignores case for all
>> suffixes, not just 'k'.
>
> Yay! Be liberal in what you accept! (I've always been annoyed that '-m
> 1g' complains, forcing me to us '-m 1G').
The liberality is okay when we measure bytes, because 'm' can only mean
"mega" there. It's problematic when we measure things like seconds,
where 'm' definitely should mean "milli".
Perhaps I should make do_strtosz() case-insensitive only when unit ==
1024, but not when unit == 1000.
> Okay, we're at the end of the series, and I've pointed out other
> possible improvements. As mentioned elsewhere, it might be nice to add
> a 25/24 to match coreutils, which allows 'm' and 'mib' to mean
> 1024*1024, vs. 'mb' to mean 1000*1000. Right now, all clients of
> qemu_strtosz() are stuck to one scale, vs. letting the user choose a
> scale by their choice of suffix. But it does not hold up this series if
> you don't like the idea.
I'd prefer not to do that now, because I'm busy on getting -blockdev
done before the soft freeze.
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> tests/test-qemu-opts.c | 21 +++++++++------------
>> util/qemu-option.c | 41 +++++++++++++----------------------------
>> 2 files changed, 22 insertions(+), 40 deletions(-)
>>
>
> Reviewed-by: Eric Blake <address@hidden>
Thanks!
[Qemu-devel] [PATCH 22/24] util/cutils: Return qemu_strtosz*() error and value separately, Markus Armbruster, 2017/02/14