Re: [PATCH 1/3] xen: Control-flow cleanup

[Jeremy Fitzhardinge <jeremy@goop.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<waldi@debian.org>
> ---
>   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;
>   }
>
>