Re: [Qemu-devel] [PATCH] migration: don't use uninitialized variables

[Anthony Liguori]

On Mon, Jul 29, 2013 at 7:15 PM, Markus Armbruster <address@hidden> wrote:
> Luiz Capitulino <address@hidden> writes:
>
>> On Fri, 19 Jul 2013 04:57:51 -0600
>> Eric Blake <address@hidden> wrote:
>>
>>> On 07/18/2013 08:36 PM, Pawit Pornkitprasan wrote:
>>> > The qmp_migrate method uses the 'blk' and 'inc' parameter without
>>> > checking if they're valid or not (they may be uninitialized if
>>> > command is received via QMP)
>>> >
>>> > Signed-off-by: Pawit Pornkitprasan <address@hidden>
>>> > ---
>>> >  migration.c | 4 ++--
>>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> Reviewed-by: Eric Blake <address@hidden>
>>>
>>> However, wouldn't it be nice if we improved the qapi generator to
>>> guarantee a sane default value for optional parameters, even when
>>> has_value is false?
>>
>> We could do that for bool and pointers, but this wouldn't help
>> integers and enums. Also, even if we had default values, I guess
>> I'd enforce a common idiom for handling optionals as this is also
>> a good practice for preventing bugs.
>
> I disagree on pointers.
>
> "have_ptr && ptr->..." is a stupid idiom.  The common idiom for safe
> dereference is "ptr && ptr->...".
>
> "bool have_ptr, FOO *ptr" in a parameter list is unidiomatic C.
> Idiomatic is just "FOO *ptr".
>
> Conciseness is a virtue.
>
> For non-pointers, there's no special "don't have one" value, so we'd
> have to declare a default value in the schema to get rid of the
> have_FOO.

The problem is the protocol.  There are cases where the absence of a
value is different than having a default value for the parameter.

This was part of the discussion way back when this all was first
introduced.  Since everything was open coded and we had to preserve
the semantics, that was the only choice we had.

Regards.

Anthony Liguori