Re: [Qemu-devel] [PATCH v8 18/35] qapi: Drop unused error argument for list and implicit struct

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2270 bytes --]

On 01/05/2016 07:05 AM, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Dec 21, 2015 at 6:08 PM, Eric Blake <eblake@redhat.com> wrote:
>> No backend was setting an error when ending the visit of a list
>> or implicit struct.  Make the callers a bit easier to follow by
>> making this a part of the contract, and removing the errp
>> argument - callers can then unconditionally end an object as
>> part of cleanup without having to think about whether a second
>> error is dominated by a first, because there is no second error.
>>
>> A later patch will then tackle the larger task of splitting
>> visit_end_struct(), which can indeed set an error.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>
> 
> This patch makes visit_end_list() possibly abort, while before it
> would pass the error to upper.

Not so. The only added use of &error_abort is...

> I assume that's what you are going to
> change next.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 

>> +++ b/qapi/qmp-input-visitor.c
>> @@ -153,10 +153,6 @@ static void qmp_input_start_implicit_struct(Visitor *v, void **obj,
>>      }
>>  }
>>
>> -static void qmp_input_end_implicit_struct(Visitor *v, Error **errp)
>> -{
>> -}
>> -
>>  static void qmp_input_start_list(Visitor *v, const char *name, Error **errp)
>>  {
>>      QmpInputVisitor *qiv = to_qiv(v);
>> @@ -201,11 +197,11 @@ static GenericList *qmp_input_next_list(Visitor *v, GenericList **list,
>>      return entry;
>>  }
>>
>> -static void qmp_input_end_list(Visitor *v, Error **errp)
>> +static void qmp_input_end_list(Visitor *v)
>>  {
>>      QmpInputVisitor *qiv = to_qiv(v);
>>
>> -    qmp_input_pop(qiv, errp);
>> +    qmp_input_pop(qiv, &error_abort);

...here; but the only time that qmp_input_pop() sets error is if it is
paired with a qmp_input_push() from a struct, not a list.  It is a true
programming bug if you can mismatch push(struct)/pop(list) or
push(list)/pop(struct), which deserves to abort, and which is not
triggerable in the current code base.

But you are correct that later patches then further clean up
qmp-input-visitor.c.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]