From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeremy Fitzhardinge Subject: Re: [PATCH 1/3] xen: Control-flow cleanup Date: Wed, 24 Mar 2010 10:53:13 -0700 Message-ID: <4BAA5189.8080503@goop.org> References: <20100324115137.GA10770@wavehammer.waldi.eu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20100324115137.GA10770@wavehammer.waldi.eu.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Bastian Blank Cc: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On 03/24/2010 04:51 AM, Bastian Blank wrote: > It is nice to have only one return, but it does not make it easier to > read or change. > Thanks, I agree, but I tweaked it to use "goto out" to preserve the single return, and fixed some lines which can now be unwrapped with the shallower indent. J > Signed-off-by: Bastian Blank > --- > arch/x86/xen/pci.c | 52 +++++++++++++++++++++++++++------------------------- > 1 files changed, 27 insertions(+), 25 deletions(-) > > diff --git a/arch/x86/xen/pci.c b/arch/x86/xen/pci.c > index 61e1ade..a83c74c 100644 > --- a/arch/x86/xen/pci.c > +++ b/arch/x86/xen/pci.c > @@ -42,32 +42,34 @@ int xen_register_gsi(u32 gsi, int triggering, int polarity) > > printk(KERN_DEBUG "xen: --> irq=%d\n", irq); > > - if (irq>= 0) { > - setup_gsi.gsi = gsi; > - setup_gsi.triggering = (triggering == ACPI_EDGE_SENSITIVE ? > - 0 : 1); > - setup_gsi.polarity = (polarity == ACPI_ACTIVE_HIGH ? 0 : 1); > - > - rc = HYPERVISOR_physdev_op(PHYSDEVOP_setup_gsi,&setup_gsi); > - if (rc == -EEXIST) > - printk(KERN_INFO "Already setup the GSI :%d\n", gsi); > - else if (rc) { > - printk(KERN_ERR "Failed to setup GSI :%d, err_code:%d\n", > - gsi, rc); > - BUG(); > - } > + if (irq< 0) > + return irq; > + > + setup_gsi.gsi = gsi; > + setup_gsi.triggering = (triggering == ACPI_EDGE_SENSITIVE ? > + 0 : 1); > + setup_gsi.polarity = (polarity == ACPI_ACTIVE_HIGH ? 0 : 1); > + > + rc = HYPERVISOR_physdev_op(PHYSDEVOP_setup_gsi,&setup_gsi); > + if (rc == -EEXIST) > + printk(KERN_INFO "Already setup the GSI :%d\n", gsi); > + else if (rc) { > + printk(KERN_ERR "Failed to setup GSI :%d, err_code:%d\n", > + gsi, rc); > + BUG(); > + } > > - map_irq.domid = DOMID_SELF; > - map_irq.type = MAP_PIRQ_TYPE_GSI; > - map_irq.index = gsi; > - map_irq.pirq = irq; > + map_irq.domid = DOMID_SELF; > + map_irq.type = MAP_PIRQ_TYPE_GSI; > + map_irq.index = gsi; > + map_irq.pirq = irq; > > - rc = HYPERVISOR_physdev_op(PHYSDEVOP_map_pirq,&map_irq); > - if (rc) { > - printk(KERN_WARNING "xen map irq failed %d\n", rc); > - irq = -1; > - } > + rc = HYPERVISOR_physdev_op(PHYSDEVOP_map_pirq,&map_irq); > + if (rc) { > + printk(KERN_WARNING "xen map irq failed %d\n", rc); > + return -1; > } > + > return irq; > } > >