linux-kernel - Re: [PATCH 5/5] ISDN-CAPI: Delete unnecessary braces

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, 26 Sep 2016 14:52:39 +0200
From:   SF Markus Elfring <elfring@...rs.sourceforge.net>
To:     Paul Bolle <pebolle@...cali.nl>
Cc:     Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
        netdev@...r.kernel.org, Karsten Keil <isdn@...ux-pingi.de>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org,
        Julia Lawall <julia.lawall@...6.fr>
Subject: Re: [PATCH 5/5] ISDN-CAPI: Delete unnecessary braces

>>>> @@ -976,13 +974,12 @@ static void handle_controller(_cmsg *cmsg)
>>>>          if (debugmode)
>>>>              printk(KERN_DEBUG "capidrv-%d: listenconf Info=0x%4x
>>>> (%s) cipmask=0x%x\n",
>>>>                     card->contrnr, cmsg->Info,
>>>> capi_info2str(cmsg->Info), card->cipmask);
>>>> -        if (cmsg->Info) {
>>>> +        if (cmsg->Info)
>>>>              listen_change_state(card, EV_LISTEN_CONF_ERROR);
>>>> -        } else if (card->cipmask == 0) {
>>>> +        else if (card->cipmask == 0)
>>>>              listen_change_state(card, EV_LISTEN_CONF_EMPTY);
>>>> -        } else {
>>>> +             else
>>>
>>>    Indented too much.
>>
>> How do you think about an alignment of this "else"
>> with the corresponding if statement three lines above?
> 
> Well, I think it looks silly.

Thanks for your feedback.


> checkpatch apparently agrees:
>     WARNING: Statements should start on a tabstop
>     #51: FILE: drivers/isdn/capi/capidrv.c:981:
>     +		     else
>     
>     total: 0 errors, 1 warnings, 91 lines checked
…
> You use checkpatch a lot, don't you?

It seems so when I am preparing hundreds of update steps.


> Didn't you use it to, you know, check your patch?

This Perl script showed me also the quoted information.

I dared to present a deviation from this general advice
because I got the mood to gather constructive comments on
source code formatting also around an if statement like
in my update suggestion.

Would it eventually make sense to move this "if" (behind an "else")
to a separate line and increase the indentation for the corresponding
code one level then?

Regards,
Markus