Re: [PATCH 03/46] qdev: Smooth error checking of qdev_realize() & friends

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> 24.06.2020 19:43, Markus Armbruster wrote:
>> Convert
>>
>>      foo(..., &err);
>>      if (err) {
>>          ...
>>      }
>>
>> to
>>
>>      if (!foo(..., &err)) {
>>          ...
>>      }
>>
>> for qdev_realize(), qdev_realize_and_unref(), qbus_realize() and their
>> wrappers isa_realize_and_unref(), pci_realize_and_unref(),
>> sysbus_realize(), sysbus_realize_and_unref(), usb_realize_and_unref().
>> Coccinelle script:
>
> Please, also mention a command to run the script
>
>>
>>      @@
>>      identifier fun = {isa_realize_and_unref, pci_realize_and_unref, 
>> qbus_realize, qdev_realize, qdev_realize_and_unref, sysbus_realize, 
>> sysbus_realize_and_unref, usb_realize_and_unref};
>>      expression list args, args2;
>>      typedef Error;
>>      Error *err;
>>      identifier errp;
>>      @@
>>      -      fun(args, &err, args2);
>>      -      if (err) {
>>      +      if (!fun(args, errp, args2)) {
>>             ... when != err
>>      -          error_propagate(errp, err);
>>             ...
>>         }
>>
>>      @@
>>      identifier fun = {isa_realize_and_unref, pci_realize_and_unref, 
>> qbus_realize, qdev_realize, qdev_realize_and_unref, sysbus_realize, 
>> sysbus_realize_and_unref, usb_realize_and_unref};
>>      expression list args, args2;
>>      typedef Error;
>>      Error *err;
>>      @@
>>      -      fun(args, &err, args2);
>>      -      if (err) {
>>      +      if (!fun(args, &err, args2)) {
>>             ...
>>         }
>>
>> Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by
>> ARMSSE being used both as typedef and function-like macro there.
>> Convert manually.
>>
>> Eliminate error_propagate() that are now unnecessary.  Delete @err
>> that are now unused.  Clean up whitespace.
>>
>> Signed-off-by: Markus Armbruster<armbru@redhat.com>
>> ---
[...]
>>  65 files changed, 248 insertions(+), 768 deletions(-)
>
> Almost all of this diff-stat may be generated by more obvious script:
>
> @rule1@
> identifier fun = {qdev_realize, qdev_realize_and_unref, sysbus_realize};
> expression list args;
> typedef Error;
> Error *err;
> identifier errp;
> @@
>
> -      fun(args, &err);
> -      if (err)
> +      if (!fun(args, errp))
>        {

Funny: the placement of { makes a difference.

When I use

  -      if (err) {
  +      if (!fun(args, errp)) {

out of QEMU coding style habit, I get an extra line containing only
spaces in the output.  Easy enough to clean up with sed, but putting the
brace in the place where Coccinelle doesn't produce such litter is
better.

> -              error_propagate(errp, err);
>            return;
>        }
>
[...]