Re: [PATCH 09/46] error: Avoid error_propagate() after migrate_add_blocker()

[Vladimir Sementsov-Ogievskiy]

24.06.2020 19:43, Markus Armbruster wrote:
> When migrate_add_blocker(blocker, &errp) is followed by
> error_propagate(errp, err), we can often just as well do
> migrate_add_blocker(..., errp).
>
> Do that with this Coccinelle script:
>
>      @@
>      expression blocker, err, errp;
>      expression ret;
>      @@
>      -    ret = migrate_add_blocker(blocker, &err);
>      -    if (err) {
>      +    ret = migrate_add_blocker(blocker, errp);
>      +    if (ret < 0) {
>              ... when != err;
>      -        error_propagate(errp, err);
>              ...
>          }
>
>      @@
>      expression blocker, err, errp;
>      @@
>      -    migrate_add_blocker(blocker, &err);
>      -    if (err) {
>      +    if (migrate_add_blocker(blocker, errp) < 0) {
>              ... when != err;
>      -        error_propagate(errp, err);
>              ...
>          }
>
> Double-check @err is not used afterwards.  Dereferencing it would be
> use after free, but checking whether it's null would be legitimate.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   block/parallels.c            | 5 ++---
>   block/qcow.c                 | 6 ++----
>   block/vdi.c                  | 6 ++----
>   block/vhdx.c                 | 5 ++---
>   block/vmdk.c                 | 6 ++----
>   block/vpc.c                  | 5 ++---
>   block/vvfat.c                | 5 ++---
>   hw/display/virtio-gpu-base.c | 5 +----
>   hw/intc/arm_gic_kvm.c        | 4 +---
>   hw/intc/arm_gicv3_its_kvm.c  | 4 +---
>   hw/intc/arm_gicv3_kvm.c      | 4 +---
>   hw/misc/ivshmem.c            | 4 +---
>   hw/scsi/vhost-scsi.c         | 4 +---
>   13 files changed, 20 insertions(+), 43 deletions(-)

[..]

> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
> index ad0ebabc87..87bc4aeca1 100644
> --- a/hw/intc/arm_gicv3_its_kvm.c
> +++ b/hw/intc/arm_gicv3_its_kvm.c
> @@ -113,9 +113,7 @@ static void kvm_arm_its_realize(DeviceState *dev, Error 
> **errp)

Remove unused local_err

with fixed:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

>           GITS_CTLR)) {
>           error_setg(&s->migration_blocker, "This operating system kernel "
>                      "does not support vITS migration");
> -        migrate_add_blocker(s->migration_blocker, &local_err);
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> +        if (migrate_add_blocker(s->migration_blocker, errp) < 0) {
>               error_free(s->migration_blocker);
>               return;
>           }

[..]


--
Best regards,
Vladimir