[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/24] QemuOpts: Assert value string isn't null
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 02/24] QemuOpts: Assert value string isn't null |
Date: |
Thu, 16 Feb 2017 13:58: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:25 AM, Markus Armbruster wrote:
>> Plenty of code relies on QemuOpt member @str not being null, including
>> qemu_opts_print(), qemu_opts_to_qdict(), and callbacks passed to
>> qemu_opt_foreach().
>>
>
>>
>> Assert member @str isn't null, so that misuse is caught right away.
>>
>> Simplify parse_option_bool(), parse_option_number() and
>> parse_option_size() accordingly. Best viewed with whitespace changes
>> ignored.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> util/qemu-option.c | 89
>> ++++++++++++++++++++++++------------------------------
>> 1 file changed, 39 insertions(+), 50 deletions(-)
>>
>
>> @@ -180,39 +172,35 @@ void parse_option_size(const char *name, const char
>> *value,
>> char *postfix;
>> double sizef;
>>
>
>> + sizef = strtod(value, &postfix);
>> + if (sizef < 0 || sizef > UINT64_MAX) {
>> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
>> + "a non-negative number below 2^64");
>> + return;
>> + }
>> + switch (*postfix) {
>> + case 'T':
> ...
>> + default:
>> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
>> + error_append_hint(errp, "You may use k, M, G or T suffixes for "
>> + "kilobytes, megabytes, gigabytes and
>> terabytes.\n");
>> + return;
>> }
>
> Unrelated to this patch, but noticing it now: it looks like we blindly
> accept "qemu-system-x86_64 -nodefaults -m 1Mgarbage" as meaning the same
> as "... -m 1M". Looking back at 1/24, looks like you marked that as one
> of the buggy cases.
Correct.
> Good - I guess I'll get to comment more on it in a
> later patch.
>
> Reviewed-by: Eric Blake <address@hidden>
Thanks!
- [Qemu-devel] [PATCH 01/24] tests/test-qemu-opts: Cover qemu_opts_parse(), (continued)
- [Qemu-devel] [PATCH 01/24] tests/test-qemu-opts: Cover qemu_opts_parse(), Markus Armbruster, 2017/02/14
- [Qemu-devel] [PATCH 18/24] tests/test-cutils: Use qemu_strtosz() more often, Markus Armbruster, 2017/02/14
- [Qemu-devel] [PATCH 12/24] tests/test-cutils: Cover qemu_strtosz() with trailing crap, Markus Armbruster, 2017/02/14
- [Qemu-devel] [PATCH 03/24] tests/test-cutils: Add missing qemu_strtol()... endptr checks, Markus Armbruster, 2017/02/14
- [Qemu-devel] [PATCH 02/24] QemuOpts: Assert value string isn't null, Markus Armbruster, 2017/02/14
- [Qemu-devel] [PATCH 07/24] util/cutils: Clean up variable names around qemu_strtol(), Markus Armbruster, 2017/02/14
- [Qemu-devel] [PATCH 15/24] util/cutils: Rename qemu_strtosz() to qemu_strtosz_mebi(), Markus Armbruster, 2017/02/14
- [Qemu-devel] [PATCH 16/24] util/cutils: New qemu_strtosz(), Markus Armbruster, 2017/02/14