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]

Date:	Mon, 24 Feb 2014 08:00:26 -0800 (PST)
From:	Chase Southwood <chase.southwood@...oo.com>
To:	Ian Abbott <abbotti@....co.uk>,
	"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 Monday, February 24, 2014 8:13 AM, Ian Abbott <abbotti@....co.uk> wrote:

>>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).
>

Ah geez, don't know how that else if placement slipped through the cracks...my mistake.  I've got a revision coming straight away that moves it to the proper place and puts back my overzealous brace deletion.

>
>>
>>       return;>>
>>   }
>>
>
>
>-- 
>-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@....co.uk>        )=-
>-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
>

Thanks,
Chase
--
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/