Re: [PATCH v2 34/44] error: Eliminate error_propagate() with Coccinelle, part 1

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 7/2/20 10:49 AM, Markus Armbruster wrote:
>> When all we do with an Error we receive into a local variable is
>> propagating to somewhere else, we can just as well receive it there
>> right away.  Convert
>>
>>      if (!foo(..., &err)) {
>>          ...
>>          error_propagate(errp, err);
>>          ...
>>          return ...
>>      }
>>
>> to
>>
>>      if (!foo(..., errp)) {
>>          ...
>>          ...
>>          return ...
>>      }
>>
>
>>
>> Not exactly elegant, I'm afraid.
>
> True, but it is still reasonable enough to use.
>
>>
>> The "when != goto lbl;" is necessary to avoid transforming
>
> spelled "when != lbl:" above in rule1 and rule2

Perils of editing old explanations to fit new code...

>>
>>           if (fun(args, &err)) {
>>               goto out
>>           }
>>           ...
>>       out:
>>           error_propagate(errp, err);
>>
>> even though other paths to label out still need the error_propagate().
>> For an actual example, see sclp_realize().
>>
>> Without the "when strict", Coccinelle transforms vfio_msix_setup(),
>> incorrectly.  I don't know what exactly "when strict" does, only that
>> it helps here.
>>
>> The match of return is narrower than what I want, but I can't figure
>> out how to express "return where the operand doesn't use @err".  For
>> an example where it's too narrow, see vfio_intx_enable().
>>
>> Silently fails to convert hw/arm/armsse.c, because Coccinelle gets
>> confused by ARMSSE being used both as typedef and function-like macro
>> there.  Converted manually.
>>
>> Line breaks tidied up manually.  One nested declaration of @local_err
>> deleted manually.  Preexisting unwanted blank line dropped in
>> hw/riscv/sifive_e.c.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>>   114 files changed, 376 insertions(+), 884 deletions(-)
>
> Big, but sane.  Picks up the spots I noticed in 33/44, and then some ;)

Right :)

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!