| Message ID | 20091112120805.GC11106@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <yamahata@valinux.co.jp> > > 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 <mst@redhat.com> > 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 <mst@redhat.com> Acked-by: Isaku Yamahata <yamahata@valinux.co.jp> > > --- > > 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, >
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,