qemu: remove nested branching to enhance readability

[libvirt] [PATCH] qemu: remove nested branching to enhance readability

Posted by Pavel Mores 4 years, 5 months ago

This is a follow-up to patch series posted in

https://www.redhat.com/archives/libvir-list/2019-November/msg01180.html

It implements a suggestion made by Cole in

https://www.redhat.com/archives/libvir-list/2019-November/msg01207.html

and discussed in follow-up messages as there were no objections to the
change.

The aim is to make the code more readable by replacing nested branching
with a flat structure.

Signed-off-by: Pavel Mores <pmores@redhat.com>
---
 src/qemu/qemu_domain.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d1596a28ca..5e4eede3c2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7914,19 +7914,16 @@ static int
 qemuDomainDefaultVideoDevice(const virDomainDef *def,
                           virQEMUCapsPtr qemuCaps)
 {
-    if (ARCH_IS_PPC64(def->os.arch)) {
+    if (ARCH_IS_PPC64(def->os.arch))
         return VIR_DOMAIN_VIDEO_TYPE_VGA;
-    } else if (qemuDomainIsARMVirt(def) ||
-               qemuDomainIsRISCVVirt(def) ||
-               ARCH_IS_S390(def->os.arch)) {
+    if (qemuDomainIsARMVirt(def) || qemuDomainIsRISCVVirt(def) ||
+        ARCH_IS_S390(def->os.arch))
         return VIR_DOMAIN_VIDEO_TYPE_VIRTIO;
-    } else {
-        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA))
-            return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
-        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA))
-            return VIR_DOMAIN_VIDEO_TYPE_VGA;
-        return VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
-    }
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA))
+        return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
+    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA))
+        return VIR_DOMAIN_VIDEO_TYPE_VGA;
+    return VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
 }
 
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: remove nested branching to enhance readability

Posted by Daniel Henrique Barboza 4 years, 5 months ago


On 12/6/19 6:45 AM, Pavel Mores wrote:
> This is a follow-up to patch series posted in
> 
> https://www.redhat.com/archives/libvir-list/2019-November/msg01180.html
> 
> It implements a suggestion made by Cole in
> 
> https://www.redhat.com/archives/libvir-list/2019-November/msg01207.html
> 
> and discussed in follow-up messages as there were no objections to the
> change.
> 
> The aim is to make the code more readable by replacing nested branching
> with a flat structure.
> 
> Signed-off-by: Pavel Mores <pmores@redhat.com>
> ---


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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: remove nested branching to enhance readability

Posted by Michal Privoznik 4 years, 5 months ago

On 12/6/19 10:45 AM, Pavel Mores wrote:
> This is a follow-up to patch series posted in
> 
> https://www.redhat.com/archives/libvir-list/2019-November/msg01180.html
> 
> It implements a suggestion made by Cole in
> 
> https://www.redhat.com/archives/libvir-list/2019-November/msg01207.html
> 
> and discussed in follow-up messages as there were no objections to the
> change.
> 
> The aim is to make the code more readable by replacing nested branching
> with a flat structure.
> 
> Signed-off-by: Pavel Mores <pmores@redhat.com>
> ---
>   src/qemu/qemu_domain.c | 19 ++++++++-----------
>   1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index d1596a28ca..5e4eede3c2 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7914,19 +7914,16 @@ static int
>   qemuDomainDefaultVideoDevice(const virDomainDef *def,
>                             virQEMUCapsPtr qemuCaps)
>   {
> -    if (ARCH_IS_PPC64(def->os.arch)) {
> +    if (ARCH_IS_PPC64(def->os.arch))
>           return VIR_DOMAIN_VIDEO_TYPE_VGA;
> -    } else if (qemuDomainIsARMVirt(def) ||
> -               qemuDomainIsRISCVVirt(def) ||
> -               ARCH_IS_S390(def->os.arch)) {
> +    if (qemuDomainIsARMVirt(def) || qemuDomainIsRISCVVirt(def) ||
> +        ARCH_IS_S390(def->os.arch))

Actually, I like it more if these are on separate lines.

>           return VIR_DOMAIN_VIDEO_TYPE_VIRTIO;
> -    } else {
> -        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA))
> -            return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
> -        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA))
> -            return VIR_DOMAIN_VIDEO_TYPE_VGA;
> -        return VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
> -    }
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA))
> +        return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
> +    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA))
> +        return VIR_DOMAIN_VIDEO_TYPE_VGA;
> +    return VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
>   }
>   
>   
> 

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

and pushed.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list