[PATCH v2 41/44] error: Avoid error_propagate() after migrate_add_blocker()

Markus Armbruster posted 44 patches 5 years, 7 months ago
Maintainers: Andrey Smirnov <andrew.smirnov@gmail.com>, Antony Pavlov <antonynpavlov@gmail.com>, Eric Blake <eblake@redhat.com>, Matthew Rosato <mjrosato@linux.ibm.com>, John Snow <jsnow@redhat.com>, Jason Wang <jasowang@redhat.com>, Richard Henderson <rth@twiddle.net>, Kevin Wolf <kwolf@redhat.com>, Paul Burton <pburton@wavecomp.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Zhang Chen <chen.zhang@intel.com>, "Michael S. Tsirkin" <mst@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Paul Durrant <paul@xen.org>, Christian Borntraeger <borntraeger@de.ibm.com>, David Hildenbrand <david@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Max Reitz <mreitz@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Joel Stanley <joel@jms.id.au>, Igor Mammedov <imammedo@redhat.com>, Andrew Baumann <Andrew.Baumann@microsoft.com>, Wen Congyang <wencongyang2@huawei.com>, Liu Yuan <namei.unix@gmail.com>, Laurent Vivier <lvivier@redhat.com>, Jeff Cody <codyprime@gmail.com>, Stefan Berger <stefanb@linux.ibm.com>, Riku Voipio <riku.voipio@iki.fi>, Ronnie Sahlberg <ronniesahlberg@gmail.com>, Ari Sundholm <ari@tuxera.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Jan Kiszka <jan.kiszka@web.de>, Michael Roth <mdroth@linux.vnet.ibm.com>, Alex Williamson <alex.williamson@redhat.com>, Li Zhijian <lizhijian@cn.fujitsu.com>, Beniamino Galvani <b.galvani@gmail.com>, Alistair Francis <alistair@alistair23.me>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Stefan Weil <sw@weilnetz.de>, Yoshinori Sato <ysato@users.sourceforge.jp>, Aurelien Jarno <aurelien@aurel32.net>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Leif Lindholm <leif@nuviainc.com>, "Hervé Poussineau" <hpoussin@reactos.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com>, Peter Lieven <pl@kamp.de>, Eric Auger <eric.auger@redhat.com>, Xiao Guangrong <xiaoguangrong.eric@gmail.com>, Alberto Garcia <berto@igalia.com>, Niek Linnenbank <nieklinnenbank@gmail.com>, Xie Changlong <xiechanglong.d@gmail.com>, Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>, Markus Armbruster <armbru@redhat.com>, Stefano Stabellini <sstabellini@kernel.org>, Fam Zheng <fam@euphon.net>, "Daniel P. Berrangé" <berrange@redhat.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Igor Mitsyanko <i.mitsyanko@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Peter Chubb <peter.chubb@nicta.com.au>, "Denis V. Lunev" <den@openvz.org>, Halil Pasic <pasic@linux.ibm.com>, Anthony Perard <anthony.perard@citrix.com>, Andrew Jeffery <andrew@aj.id.au>, Jason Dillaman <dillaman@redhat.com>, Rob Herring <robh@kernel.org>, "Richard W.M. Jones" <rjones@redhat.com>, "Cédric Le Goater" <clg@kaod.org>, Subbaraya Sundeep <sundeep.lkml@gmail.com>, Peter Maydell <peter.maydell@linaro.org>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Jean-Christophe Dubois <jcd@tribudubois.net>, David Gibson <david@gibson.dropbear.id.au>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Radoslaw Biernacki <radoslaw.biernacki@linaro.org>, Thomas Huth <thuth@redhat.com>, Amit Shah <amit@kernel.org>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH v2 41/44] error: Avoid error_propagate() after migrate_add_blocker()
Posted by Markus Armbruster 5 years, 7 months ago
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>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.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                | 6 ++----
 hw/display/virtio-gpu-base.c | 5 +----
 hw/intc/arm_gic_kvm.c        | 4 +---
 hw/intc/arm_gicv3_its_kvm.c  | 5 +----
 hw/intc/arm_gicv3_kvm.c      | 4 +---
 hw/misc/ivshmem.c            | 4 +---
 hw/scsi/vhost-scsi.c         | 4 +---
 13 files changed, 20 insertions(+), 45 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index f489c0d4ba..3c22dfdc9d 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -859,9 +859,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     error_setg(&s->migration_blocker, "The Parallels format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
-    ret = migrate_add_blocker(s->migration_blocker, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    ret = migrate_add_blocker(s->migration_blocker, errp);
+    if (ret < 0) {
         error_free(s->migration_blocker);
         goto fail;
     }
diff --git a/block/qcow.c b/block/qcow.c
index c22d1bf6b8..1e134f3445 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -121,7 +121,6 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
     unsigned int len, i, shift;
     int ret;
     QCowHeader header;
-    Error *local_err = NULL;
     QCryptoBlockOpenOptions *crypto_opts = NULL;
     unsigned int cflags = 0;
     QDict *encryptopts = NULL;
@@ -314,9 +313,8 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
     error_setg(&s->migration_blocker, "The qcow format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
-    ret = migrate_add_blocker(s->migration_blocker, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    ret = migrate_add_blocker(s->migration_blocker, errp);
+    if (ret < 0) {
         error_free(s->migration_blocker);
         goto fail;
     }
diff --git a/block/vdi.c b/block/vdi.c
index 0a60be773e..3a3df45f84 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -375,7 +375,6 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
     VdiHeader header;
     size_t bmap_size;
     int ret;
-    Error *local_err = NULL;
     QemuUUID uuid_link, uuid_parent;
 
     bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
@@ -496,9 +495,8 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
     error_setg(&s->migration_blocker, "The vdi format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
-    ret = migrate_add_blocker(s->migration_blocker, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    ret = migrate_add_blocker(s->migration_blocker, errp);
+    if (ret < 0) {
         error_free(s->migration_blocker);
         goto fail_free_bmap;
     }
diff --git a/block/vhdx.c b/block/vhdx.c
index eed9c3b860..1f9fca837d 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1089,9 +1089,8 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
     error_setg(&s->migration_blocker, "The vhdx format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
-    ret = migrate_add_blocker(s->migration_blocker, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    ret = migrate_add_blocker(s->migration_blocker, errp);
+    if (ret < 0) {
         error_free(s->migration_blocker);
         goto fail;
     }
diff --git a/block/vmdk.c b/block/vmdk.c
index 9a09193f3b..28cec50f38 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1263,7 +1263,6 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
     int ret;
     BDRVVmdkState *s = bs->opaque;
     uint32_t magic;
-    Error *local_err = NULL;
 
     bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
                                BDRV_CHILD_IMAGE, false, errp);
@@ -1317,9 +1316,8 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
     error_setg(&s->migration_blocker, "The vmdk format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
-    ret = migrate_add_blocker(s->migration_blocker, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    ret = migrate_add_blocker(s->migration_blocker, errp);
+    if (ret < 0) {
         error_free(s->migration_blocker);
         goto fail;
     }
diff --git a/block/vpc.c b/block/vpc.c
index 77043c2424..259d0ad2fe 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -446,9 +446,8 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     error_setg(&s->migration_blocker, "The vpc format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
-    ret = migrate_add_blocker(s->migration_blocker, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    ret = migrate_add_blocker(s->migration_blocker, errp);
+    if (ret < 0) {
         error_free(s->migration_blocker);
         goto fail;
     }
diff --git a/block/vvfat.c b/block/vvfat.c
index df4b993add..fc46de1cfc 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1154,7 +1154,6 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
     bool floppy;
     const char *dirname, *label;
     QemuOpts *opts;
-    Error *local_err = NULL;
     int ret;
 
 #ifdef DEBUG
@@ -1280,9 +1279,8 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
                    "The vvfat (rw) format used by node '%s' "
                    "does not support live migration",
                    bdrv_get_device_or_node_name(bs));
-        ret = migrate_add_blocker(s->migration_blocker, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        ret = migrate_add_blocker(s->migration_blocker, errp);
+        if (ret < 0) {
             error_free(s->migration_blocker);
             goto fail;
         }
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index c159351be3..7961308606 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -128,7 +128,6 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
     VirtIOGPUBase *g = VIRTIO_GPU_BASE(qdev);
-    Error *local_err = NULL;
     int i;
 
     if (g->conf.max_outputs > VIRTIO_GPU_MAX_SCANOUTS) {
@@ -139,9 +138,7 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
     g->use_virgl_renderer = false;
     if (virtio_gpu_virgl_enabled(g->conf)) {
         error_setg(&g->migration_blocker, "virgl is not yet migratable");
-        migrate_add_blocker(g->migration_blocker, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
+        if (migrate_add_blocker(g->migration_blocker, errp) < 0) {
             error_free(g->migration_blocker);
             return false;
         }
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index d7df423a7a..07b95143c9 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -517,9 +517,7 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
     if (!kvm_arm_gic_can_save_restore(s)) {
         error_setg(&s->migration_blocker, "This operating system kernel does "
                                           "not support vGICv2 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;
         }
diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index ad0ebabc87..46835ed085 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -91,7 +91,6 @@ static void vm_change_state_handler(void *opaque, int running,
 static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
 {
     GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
-    Error *local_err = NULL;
 
     s->dev_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_ITS, false);
     if (s->dev_fd < 0) {
@@ -113,9 +112,7 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
         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;
         }
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index ca43bf87ca..eddd07c743 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -858,9 +858,7 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
                                GICD_CTLR)) {
         error_setg(&s->migration_blocker, "This operating system kernel does "
                                           "not support vGICv3 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;
         }
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index a8dc9b377d..fc128b25e2 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -898,9 +898,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
     if (!ivshmem_is_master(s)) {
         error_setg(&s->migration_blocker,
                    "Migration is disabled when using feature 'peer mode' in device 'ivshmem'");
-        migrate_add_blocker(s->migration_blocker, &err);
-        if (err) {
-            error_propagate(errp, err);
+        if (migrate_add_blocker(s->migration_blocker, errp) < 0) {
             error_free(s->migration_blocker);
             return;
         }
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index c1b012aea4..13b05af29b 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -207,9 +207,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
                 "When external environment supports it (Orchestrator migrates "
                 "target SCSI device state or use shared storage over network), "
                 "set 'migratable' property to true to enable migration.");
-        migrate_add_blocker(vsc->migration_blocker, &err);
-        if (err) {
-            error_propagate(errp, err);
+        if (migrate_add_blocker(vsc->migration_blocker, errp) < 0) {
             error_free(vsc->migration_blocker);
             goto free_virtio;
         }
-- 
2.26.2