Re: [Qemu-devel] [PATCH 12/13] qapi: Replace uncommon use of the error API by the common one

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 05/02/2014 06:44 AM, Markus Armbruster wrote:
>> We commonly use the error API like this:
>> 
>
>> However, mixing the two techniques is confusing.  You can't use the
>> "accumulate" technique with functions designed for the "check
>> separately" technique.  You can use the "check separately" technique
>> with functions designed for the "accumulate" technique, but then
>> error_set() can't catch you setting an error more than once.
>
> Nice comparison of the two techniques.
>
>> 
>> Standardize on the "check separately" technique for now, because it's
>> overwhelmingly prevalent.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -121,23 +121,27 @@ static void balloon_stats_get_all(Object *obj, struct 
>> Visitor *v,
>
>>      visit_start_struct(v, NULL, NULL, "stats", 0, &err);
>>      if (err) {
>>          goto out_end;
>>      }
>> -        
>> -    for (i = 0; i < VIRTIO_BALLOON_S_NR; i++) {
>> +    for (i = 0; err && i < VIRTIO_BALLOON_S_NR; i++) {
>
> Oops; logic error makes this loop dead code.
>
> s/err/!err/

Oww!  Respin coming...  with all your nits fixed (you earned that).

>> @@ -502,7 +503,7 @@ fdecl.write(mcgen('''
>>  /* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
>>  
>>  /*
>> - * schema-defined QAPI visitor function
>> + * schema-defined QAPI visitor functions
>
> Unrelated typo fix; could go in separately via -trivial if you were so
> inclined, but I don't mind it going in here.
>
> If the logic error is the only fix,
> Reviewed-by: Eric Blake <address@hidden>

Thanks!