Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10 02/10] qapi: Use QNull for a more regular visit_type_null()

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 07/18/2017 08:41 AM, Markus Armbruster wrote:
>> Make visit_type_null() take an @obj argument like its buddies.  This
>> helps keeing the next commit simple.
>
> s/keeing/keep/

Will fix.

>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>
>> +++ b/qapi/qobject-output-visitor.c
>> @@ -187,7 +187,8 @@ static void qobject_output_type_any(Visitor *v, const 
>> char *name,
>>      qobject_output_add_obj(qov, name, *obj);
>>  }
>>  
>> -static void qobject_output_type_null(Visitor *v, const char *name, Error 
>> **errp)
>> +static void qobject_output_type_null(Visitor *v, const char *name,
>> +                                     QNull **obj, Error **errp)
>>  {
>>      QObjectOutputVisitor *qov = to_qov(v);
>>      qobject_output_add(qov, name, qnull());
>
> would it be any better to call qobject_output_add(qov, name,
> QINCREF(*obj)) here, to ensure we are indeed passing in an appropriate
> QNull reference?  But your patch is at least self-consistent in stating
> we ignore obj in output visitors.

A QAPI schema type is connected to a C type (QAPI-generated except for a
few built-in types) and a QObject type.

For QAPI schema type 'null', the QObject type is QNull.

The C type is also QNull, but this is arbitrary.  We could just as well
generate a C struct type for the purpose.  All we need is some non-null
pointer value.  I'm lazy, so I grabbed one we already have: qnull_.

qnull_ is kind of nice because "allocation" / "deallocation" merely
increment / decrement its reference count.

A separate struct could be made less efficient (allocate / deallocate an
instance of the struct) or more efficient (make it a singleton without a
reference count).

To actually answer your question: I didn't feel like coupling the value
qobject_output_type_null() creates to the value it receives, even though
both are actually _qnull.

Precedence for coupling: qobject_output_type_any().  But that one's
different, because the choice of QObject as C type for 'any' is
essential instead of arbitrary.

Does this make some sense?

> Reviewed-by: Eric Blake <address@hidden>

Thanks!