[PATCH] qemuDomainAttachRedirdevDevice: Remove need_release variable

Yi Li posted 1 patch 3 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210203061834.1429758-1-yili@winhong.com
src/qemu/qemu_hotplug.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
[PATCH] qemuDomainAttachRedirdevDevice: Remove need_release variable
Posted by Yi Li 3 years, 2 months ago
Get rid of the 'need_release' variable.

Signed-off-by: Yi Li <yili@winhong.com>
---
 src/qemu/qemu_hotplug.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 882e5d2384..e07dba3c5e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1921,18 +1921,16 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
     bool chardevAdded = false;
     g_autofree char *tlsAlias = NULL;
     const char *secAlias = NULL;
-    bool need_release = false;
     virErrorPtr orig_err;
 
     if (qemuAssignDeviceRedirdevAlias(def, redirdev, -1) < 0)
-        goto cleanup;
+        return -1;
 
     if (!(charAlias = qemuAliasChardevFromDevAlias(redirdev->info.alias)))
-        goto cleanup;
+        return -1;
 
     if ((virDomainUSBAddressEnsure(priv->usbaddrs, &redirdev->info)) < 0)
-        goto cleanup;
-    need_release = true;
+        return -1;
 
     if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, priv->qemuCaps)))
         goto cleanup;
@@ -1964,7 +1962,7 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
  audit:
     virDomainAuditRedirdev(vm, redirdev, "attach", ret == 0);
  cleanup:
-    if (ret < 0 && need_release)
+    if (ret < 0)
         qemuDomainReleaseDeviceAddress(vm, &redirdev->info);
     return ret;
 
-- 
2.25.3




Re: [PATCH] qemuDomainAttachRedirdevDevice: Remove need_release variable
Posted by Daniel Henrique Barboza 3 years, 2 months ago

On 2/3/21 3:18 AM, Yi Li wrote:
> Get rid of the 'need_release' variable.
> 
> Signed-off-by: Yi Li <yili@winhong.com>
> ---

Good catch. The bool was being used simply as a way to tell the cleanup
code 'do not release the address if we fail before this point'. Might
as well just "return -1" in these cases and unconditionally release the
address in the jump.


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
  

>   src/qemu/qemu_hotplug.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 882e5d2384..e07dba3c5e 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1921,18 +1921,16 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
>       bool chardevAdded = false;
>       g_autofree char *tlsAlias = NULL;
>       const char *secAlias = NULL;
> -    bool need_release = false;
>       virErrorPtr orig_err;
>   
>       if (qemuAssignDeviceRedirdevAlias(def, redirdev, -1) < 0)
> -        goto cleanup;
> +        return -1;
>   
>       if (!(charAlias = qemuAliasChardevFromDevAlias(redirdev->info.alias)))
> -        goto cleanup;
> +        return -1;
>   
>       if ((virDomainUSBAddressEnsure(priv->usbaddrs, &redirdev->info)) < 0)
> -        goto cleanup;
> -    need_release = true;
> +        return -1;
>   
>       if (!(devstr = qemuBuildRedirdevDevStr(def, redirdev, priv->qemuCaps)))
>           goto cleanup;
> @@ -1964,7 +1962,7 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver,
>    audit:
>       virDomainAuditRedirdev(vm, redirdev, "attach", ret == 0);
>    cleanup:
> -    if (ret < 0 && need_release)
> +    if (ret < 0)
>           qemuDomainReleaseDeviceAddress(vm, &redirdev->info);
>       return ret;
>   
> 

Re: [PATCH] qemuDomainAttachRedirdevDevice: Remove need_release variable
Posted by Michal Privoznik 3 years, 2 months ago
On 2/3/21 7:18 AM, Yi Li wrote:
> Get rid of the 'need_release' variable.
> 
> Signed-off-by: Yi Li <yili@winhong.com>
> ---
>   src/qemu/qemu_hotplug.c | 10 ++++------
>   1 file changed, 4 insertions(+), 6 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed.

Michal