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

[Isaku Yamahata]

On Thu, Nov 12, 2009 at 02:08:05PM +0200, Michael S. Tsirkin wrote:
> 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>

Acked-by: Isaku Yamahata <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,
> 

-- 
yamahata