| 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
| ||
|
Message-ID: <20191220191252.GA48729@ubuntu-m2-xlarge-x86>
Date: Fri, 20 Dec 2019 12:12:52 -0700
From: Nathan Chancellor <natechancellor@...il.com>
To: Nick Desaulniers <ndesaulniers@...gle.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jiri Slaby <jslaby@...e.com>,
LKML <linux-kernel@...r.kernel.org>,
clang-built-linux <clang-built-linux@...glegroups.com>
Subject: Re: [PATCH] tty: synclink: Adjust indentation and style in several
functions
On Fri, Dec 20, 2019 at 10:04:02AM -0800, Nick Desaulniers wrote:
> On Tue, Dec 17, 2019 at 6:28 PM Nathan Chancellor
> <natechancellor@...il.com> wrote:
> >
> > Clang warns:
> >
> > ../drivers/tty/synclink.c:1167:3: warning: misleading indentation;
> > statement is not part of the previous 'if' [-Wmisleading-indentation]
> > if ( (status & RXSTATUS_ABORT_RECEIVED) &&
> > ^
> > ../drivers/tty/synclink.c:1163:2: note: previous statement is here
> > if ( debug_level >= DEBUG_LEVEL_ISR )
> > ^
> > ../drivers/tty/synclink.c:1973:3: warning: misleading indentation;
> > statement is not part of the previous 'if' [-Wmisleading-indentation]
> > if (I_BRKINT(info->port.tty) || I_PARMRK(info->port.tty))
> > ^
> > ../drivers/tty/synclink.c:1971:2: note: previous statement is here
> > if (I_INPCK(info->port.tty))
> > ^
> > ../drivers/tty/synclink.c:3229:3: warning: misleading indentation;
> > statement is not part of the previous 'else' [-Wmisleading-indentation]
> > usc_set_serial_signals(info);
> > ^
> > ../drivers/tty/synclink.c:3227:2: note: previous statement is here
> > else
> > ^
> > ../drivers/tty/synclink.c:4918:4: warning: misleading indentation;
> > statement is not part of the previous 'else' [-Wmisleading-indentation]
> > if ( info->params.clock_speed )
> > ^
> > ../drivers/tty/synclink.c:4901:3: note: previous statement is here
> > else
> > ^
> > 4 warnings generated.
> >
> > The indentation on these lines is not at all consistent, tabs and spaces
> > are mixed together. Convert to just using tabs to be consistent with the
> > Linux kernel coding style and eliminate these warnings from clang.
> >
> > Additionally, clean up some of lines touched by the indentation shift to
> > eliminate checkpatch warnings and leave this code in a better condition
> > than when it was left.
>
> Indeed, this file is kind of a mess.
>
> >
> > -:10: ERROR: trailing whitespace
> > -:10: ERROR: that open brace { should be on the previous line
> > -:10: ERROR: space prohibited after that open parenthesis '('
> > -:14: ERROR: space prohibited before that close parenthesis ')'
> > -:82: ERROR: trailing whitespace
> > -:87: WARNING: Block comments use a trailing */ on a separate line
> > -:88: ERROR: that open brace { should be on the previous line
> > -:88: ERROR: space prohibited after that open parenthesis '('
> > -:88: ERROR: space prohibited before that close parenthesis ')'
> > -:99: ERROR: else should follow close brace '}'
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/821
> > Signed-off-by: Nathan Chancellor <natechancellor@...il.com>
> > ---
> > drivers/tty/synclink.c | 55 ++++++++++++++++++++----------------------
> > 1 file changed, 26 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/tty/synclink.c b/drivers/tty/synclink.c
> > index 61dc6b4a43d0..586810defb21 100644
> > --- a/drivers/tty/synclink.c
> > +++ b/drivers/tty/synclink.c
> > @@ -1164,21 +1164,20 @@ static void mgsl_isr_receive_status( struct mgsl_struct *info )
> > printk("%s(%d):mgsl_isr_receive_status status=%04X\n",
> > __FILE__,__LINE__,status);
> >
> > - if ( (status & RXSTATUS_ABORT_RECEIVED) &&
> > + if ((status & RXSTATUS_ABORT_RECEIVED) &&
> > info->loopmode_insert_requested &&
> > - usc_loopmode_active(info) )
> > - {
> > + usc_loopmode_active(info)) {
> > ++info->icount.rxabort;
> > - info->loopmode_insert_requested = false;
> > -
> > - /* clear CMR:13 to start echoing RxD to TxD */
> > + info->loopmode_insert_requested = false;
> > +
> > + /* clear CMR:13 to start echoing RxD to TxD */
> > info->cmr_value &= ~BIT13;
> > - usc_OutReg(info, CMR, info->cmr_value);
> > -
> > + usc_OutReg(info, CMR, info->cmr_value);
> > +
> > /* disable received abort irq (no longer required) */
> > - usc_OutReg(info, RICR,
> > - (usc_InReg(info, RICR) & ~RXSTATUS_ABORT_RECEIVED));
> > - }
> > + usc_OutReg(info, RICR,
> > + (usc_InReg(info, RICR) & ~RXSTATUS_ABORT_RECEIVED));
> > + }
> >
> > if (status & (RXSTATUS_EXITED_HUNT | RXSTATUS_IDLE_RECEIVED)) {
> > if (status & RXSTATUS_EXITED_HUNT)
> > @@ -1970,8 +1969,8 @@ static void mgsl_change_params(struct mgsl_struct *info)
>
> I'm surprised the next hunk isn't mgsl_isr_transmit_status() in
> L1211-L1268? I don't mind reformatting this file, but would you mind:
> 1. splitting the changes that fix the warning and reformatting the
> rest of the file in two? That way the warning fix is more likely to
> merge back cleanly to LTS branches with less risk of merge conflict?
> Warning fix first, then reformat.
> 2. reformat the whole thing, not just most of it.
Yes, I will go ahead and break down these three TTY commits into six
(first three fixing the Clang warnings then the next three fixing all of
the indentation and spacing warnings).
I should be able to do this tonight or tomorrow at some point.
Cheers,
Nathan
Powered by blists - more mailing lists