[PATCH 05/11] arm/virt: Fix virt_machine_device_plug_cb() error API violation

Markus Armbruster posted 11 patches 5 years, 9 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>, Kevin Wolf <kwolf@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Jason Wang <jasowang@redhat.com>, Anthony Perard <anthony.perard@citrix.com>, Stefano Stabellini <sstabellini@kernel.org>, John Snow <jsnow@redhat.com>, Paul Durrant <paul@xen.org>, Juan Quintela <quintela@redhat.com>, Hailiang Zhang <zhang.zhanghailiang@huawei.com>, Max Reitz <mreitz@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, "Gonglei (Arei)" <arei.gonglei@huawei.com>
There is a newer version of this series
[PATCH 05/11] arm/virt: Fix virt_machine_device_plug_cb() error API violation
Posted by Markus Armbruster 5 years, 9 months ago
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

virt_machine_device_plug_cb() passes @errp to
cryptodev_builtin_sym_close_session() in a loop.  Harmless, because
cryptodev_builtin_sym_close_session() can't actually fail.  Fix by
dropping its Error ** parameter.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/arm/virt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7dc96abf72..cca5316256 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1186,7 +1186,7 @@ static void create_smmu(const VirtMachineState *vms,
     g_free(node);
 }
 
-static void create_virtio_iommu_dt_bindings(VirtMachineState *vms, Error **errp)
+static void create_virtio_iommu_dt_bindings(VirtMachineState *vms)
 {
     const char compat[] = "virtio,pci-iommu";
     uint16_t bdf = vms->virtio_iommu_bdf;
@@ -2118,7 +2118,7 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
 
         vms->iommu = VIRT_IOMMU_VIRTIO;
         vms->virtio_iommu_bdf = pci_get_bdf(pdev);
-        create_virtio_iommu_dt_bindings(vms, errp);
+        create_virtio_iommu_dt_bindings(vms);
     }
 }
 
-- 
2.21.1


Re: [PATCH 05/11] arm/virt: Fix virt_machine_device_plug_cb() error API violation
Posted by Philippe Mathieu-Daudé 5 years, 9 months ago
On 4/20/20 10:32 AM, Markus Armbruster wrote:
> The Error ** argument must be NULL, &error_abort, &error_fatal, or a
> pointer to a variable containing NULL.  Passing an argument of the
> latter kind twice without clearing it in between is wrong: if the
> first call sets an error, it no longer points to NULL for the second
> call.
> 
> virt_machine_device_plug_cb() passes @errp to
> cryptodev_builtin_sym_close_session() in a loop.  Harmless, because
> cryptodev_builtin_sym_close_session() can't actually fail.  Fix by
> dropping its Error ** parameter.
> 
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   hw/arm/virt.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 7dc96abf72..cca5316256 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1186,7 +1186,7 @@ static void create_smmu(const VirtMachineState *vms,
>       g_free(node);
>   }
>   
> -static void create_virtio_iommu_dt_bindings(VirtMachineState *vms, Error **errp)
> +static void create_virtio_iommu_dt_bindings(VirtMachineState *vms)
>   {
>       const char compat[] = "virtio,pci-iommu";
>       uint16_t bdf = vms->virtio_iommu_bdf;
> @@ -2118,7 +2118,7 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>   
>           vms->iommu = VIRT_IOMMU_VIRTIO;
>           vms->virtio_iommu_bdf = pci_get_bdf(pdev);
> -        create_virtio_iommu_dt_bindings(vms, errp);
> +        create_virtio_iommu_dt_bindings(vms);
>       }
>   }
>   
>