rfc: checkpatch logical line continuations (was IBM Akebono: Add support for a new PHY interface to the IBM emac driver)
josh at joshtriplett.org
josh at joshtriplett.org
Sat Mar 8 08:30:17 EST 2014
On Fri, Mar 07, 2014 at 01:02:44PM -0800, Joe Perches wrote:
> On Fri, 2014-03-07 at 15:41 -0500, David Miller wrote:
> > From: Alistair Popple <alistair at popple.id.au>
> > Date: Thu, 6 Mar 2014 14:52:25 +1100
> >
> > > + out_be32(dev->reg, in_be32(dev->reg) | WKUP_ETH_RGMIIEN
> > > + | WKUP_ETH_TX_OE | WKUP_ETH_RX_IE);
> >
> > When an expression spans multiple lines, the lines should end with
> > operators rather than begin with them.
>
> That's not in CodingStyle currently.
It's also not even remotely consistent across existing kernel code, and
it isn't obvious that there's a general developer consensus on the
"right" way to write it.
> Right now, checkpatch emits a --strict only warning on "&&" or "||"
> at the beginning of line but that could be changed to any "$Operators"
>
> our $Arithmetic = qr{\+|-|\*|\/|%};
> our $Operators = qr{
> <=|>=|==|!=|
> =>|->|<<|>>|<|>|!|~|
> &&|\|\||,|\^|\+\+|--|&|\||$Arithmetic
> }x;
>
> The ones that likely have a too high false positive rates
> are the negation "!" and bitwise "~".
I don't think warning about operators at start of line seems like a good
idea at all. There are plenty of cases where putting the operator at
the start of the line will produce a better result. (I'd actually
suggest that in *most* cases.)
> Also, using perl, it's hard to distinguish between a
> logical "&" and the address-of "&" as well as the
> multiplication "*" and indirection "*" so maybe those
> should be excluded too.
>
> And I think it should only be added as a --strict test.
Agreed, if even that.
- Josh Triplett
More information about the Linuxppc-dev
mailing list