[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/16] qapi: introduce OptsVisitor
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH 04/16] qapi: introduce OptsVisitor |
Date: |
Wed, 06 Jun 2012 13:12:16 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:10.0.4) Gecko/20120422 Thunderbird/10.0.4 |
Thank you very much for the review! One question below (and maybe some
more later in response to other parts of the review):
On 06/05/12 23:12, Paolo Bonzini wrote:
> Il 22/05/2012 12:45, Laszlo Ersek ha scritto:
>> Optarg values can be of scalar types str / bool / int / size.
>
> Michael Roth recently added Visitor interfaces for uint*_t and int*_t,
> they look like they would simplify the patches very nicely. They do the
> range checking automatically and only need a type_int callback in the
> visitor.
>
> You can get the patch from http://patchwork.ozlabs.org/patch/150427/,
> feel free to include it at the beginning of your series.
The unsigned functions don't check if the int64_t value to be assigned is
negative. What happens in such a case is fully defined, but did Michael
really intend the wraparound to unsigned?
I thought it was a silent requirement for the int parser to return a
non-negative int (which is something I also implemented in
opts_type_int() [*]), but the signed visit_type_XXX() functions do check
for the negative limit. I'm confused.
[*] see the comment in opts-visitor.h:
/* Contrarily to qemu-option.c::parse_option_number(), OptsVisitor's "int"
* parser relies on strtoll() instead of strtoull(). Consequences:
* - string representations of negative numbers are rejected (parsed values
* continue to be non-negative),
* - values above INT64_MAX or LLONG_MAX are rejected.
*/
That's also why I dropped/omitted the "< 0" checks in net_init_nic() [v1
09/16], net_init_dump() [v1 10/16] and net_init_vde() [v1 13/16].
If you want I can go through all the integer fields I introduced in [v1
06/16] "qapi schema: add Netdev types" and classify each as strictly as
possible (and then remove the checks being obviated).
Thanks!
Laszlo