[Qemu-devel] Re: [PATCH 20/20] pci: remove goto in pci_bridge_filter().

[Michael S. Tsirkin]

On Thu, Nov 12, 2009 at 02:58:48PM +0900, Isaku Yamahata wrote:
> This patch removes ugly goto in pci_bridge_filter() by
> introducing subfunction, pci_bridge_filter_nomap().
> 
> Signed-off-by: Isaku Yamahata <address@hidden>

goto on error is actually cleaner IMO.
just *not into scope*.

> ---
>  hw/pci.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index add919b..90bdf5e 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -691,6 +691,12 @@ static pcibus_t pci_bridge_get_limit(PCIDevice *bridge, 
> uint8_t type)
>      return limit;
>  }
>  
> +static void pci_bridge_filter_nomap(pcibus_t *addr, pcibus_t *size)
> +{
> +    *addr = PCI_BAR_UNMAPPED;
> +    *size = 0;
> +}
> +
>  static void pci_bridge_filter(PCIDevice *d, pcibus_t *addr, pcibus_t *size,
>                                uint8_t type)
>  {
> @@ -703,11 +709,13 @@ static void pci_bridge_filter(PCIDevice *d, pcibus_t 
> *addr, pcibus_t *size,
>  
>          if (type & PCI_BASE_ADDRESS_SPACE_IO) {
>              if (!(cmd & PCI_COMMAND_IO)) {
> -                goto no_map;
> +                pci_bridge_filter_nomap(addr, size);
> +                return;
>              }
>          } else {
>              if (!(cmd & PCI_COMMAND_MEMORY)) {
> -                goto no_map;
> +                pci_bridge_filter_nomap(addr, size);
> +                return;
>              }
>          }
>  
> @@ -716,9 +724,7 @@ static void pci_bridge_filter(PCIDevice *d, pcibus_t 
> *addr, pcibus_t *size,
>      }
>  
>      if (base > limit) {
> -    no_map:
> -        *addr = PCI_BAR_UNMAPPED;
> -        *size = 0;
> +        pci_bridge_filter_nomap(addr, size);
>      } else {
>          *addr = base;
>          *size = limit - base + 1;

Here's what I came up with:

--->
From: Michael S. Tsirkin <address@hidden>
Subject: pci: convert goto into scope in bridge_filter
    
goto into scope is evil. rearrange pci_bridge_filter
so that we always go to end of function on error.

Signed-off-by: Michael S. Tsirkin <address@hidden>

---

diff --git a/hw/pci.c b/hw/pci.c
index 14de2d1..6e5d57b 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -716,13 +716,14 @@ static void pci_bridge_filter(PCIDevice *d, pcibus_t 
*addr, pcibus_t *size,
     }
 
     if (base > limit) {
-    no_map:
-        *addr = PCI_BAR_UNMAPPED;
-        *size = 0;
-    } else {
-        *addr = base;
-        *size = limit - base + 1;
+        goto no_map;
     }
+    *addr = base;
+    *size = limit - base + 1;
+    return;
+no_map:
+    *addr = PCI_BAR_UNMAPPED;
+    *size = 0;
 }
 
 static pcibus_t pci_bar_address(PCIDevice *d,