linux-kernel - Re: [PATCH 2/3] Staging: comedi: addi-data: cleanup conditional blocks in hwdrv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <530B539B.8050201@mev.co.uk>
Date:	Mon, 24 Feb 2014 14:13:47 +0000
From:	Ian Abbott <abbotti@....co.uk>
To:	Chase Southwood <chase.southwood@...oo.com>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC:	Ian Abbott <ian.abbott@....co.uk>,
	"hsweeten@...ionengravers.com" <hsweeten@...ionengravers.com>,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3] Staging: comedi: addi-data: cleanup conditional blocks
 in hwdrv_apci035.c

On 2014-02-22 03:21, Chase Southwood wrote:
> There were some conditional blocks that had an unneccesary level of
> indentation in them.  We can remove this to improve code clarity.
>
> Signed-off-by: Chase Southwood <chase.southwood@...oo.com>
> ---
>   .../comedi/drivers/addi-data/hwdrv_apci035.c       | 31 ++++++++++------------
>   1 file changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> index 7c40535..34e5321 100644
> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> @@ -207,23 +207,22 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
>   		ui_Command =
>   			(ui_Command & 0xFFF719E2UL) | ui_Mode << 13UL | 0x10UL;
>
> -	} else {
> -		if (data[0] == ADDIDATA_WATCHDOG) {
> +	} else if (data[0] == ADDIDATA_WATCHDOG) {
>
> -			/* Set the mode :             */
> -			/* - Disable the hardware     */
> -			/* - Disable the counter mode */
> -			/* - Disable the warning      */
> -			/* - Disable the reset        */
> -			/* - Disable the timer mode   */
> +		/* Set the mode :             */
> +		/* - Disable the hardware     */
> +		/* - Disable the counter mode */
> +		/* - Disable the warning      */
> +		/* - Disable the reset        */
> +		/* - Disable the timer mode   */
>
> -			ui_Command = ui_Command & 0xFFF819E2UL;
> +		ui_Command = ui_Command & 0xFFF819E2UL;
>
> -		} else {
> -			dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
> -			return -EINVAL;
> -		}
> +	} else {
> +		dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
> +		return -EINVAL;
>   	}
> +
>   	outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
>   	ui_Command = 0;
>   	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
> @@ -628,10 +627,8 @@ static void v_APCI035_Interrupt(int irq, void *d)
>   		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
>   	}
>
> -	else {
> -		if ((ui_StatusRegister2 & 0x1) == 0x1)
> -			send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
> -	}
> +	else if ((ui_StatusRegister2 & 0x1) == 0x1)
> +		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */

The usual style there is to put the "else" or the "else if" on the same 
line as the preceding closing brace.

Also, if any branch in the if, else if, ..., else chain uses braces, 
they all should (see the bit that says "This does not apply if only one 
branch of a conditional statement is a single statement; in the latter 
case use braces in both branches:" in the CodingStyle).

>
>   	return;
>   }
>


-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@....co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/