Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161227AbbEEJl2 (ORCPT ); Tue, 5 May 2015 05:41:28 -0400 Received: from mail-wi0-f173.google.com ([209.85.212.173]:38209 "EHLO mail-wi0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757291AbbEEJlO (ORCPT ); Tue, 5 May 2015 05:41:14 -0400 Date: Tue, 5 May 2015 10:41:09 +0100 From: Andy Whitcroft To: Nicholas Mc Guire Cc: Joe Perches , linux-kernel@vger.kernel.org Subject: Re: [PATCH RFC] checkpatch: flag split arithmetic operations with CHECK Message-ID: <20150505094109.GA3654@bark> References: <1430816016-24427-1-git-send-email-hofrat@osadl.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1430816016-24427-1-git-send-email-hofrat@osadl.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3089 Lines: 69 On Tue, May 05, 2015 at 10:53:36AM +0200, Nicholas Mc Guire wrote: > Simple arithmetic operations should be on one line, if they can be fit, > rather than splitting at the operator. As this is not in the CodingStyle it > is limited to --strict use of checkpatch.pl and emits a CHECK only. > > Signed-off-by: Nicholas Mc Guire > --- > > Patches by people like me splitting arithmetic operations, e.g.: > hdw->encoder_run_timer.expires = jiffies + > msecs_to_jiffies(TIME_MSEC_ENCODER_OK); > were flagged by Joe Perches but checkpatch.pl was not fussing, not even > with --strict. This extension should flag such lines as CHECK only, as > this is not mandated by the CodingStyle and in some cases they are > justified (e.g. kernel/sched/fair.c:760 and a few other examples in that > file) > > Limitation: this is for + and - only still have to figure out some false > positives for the * and /, % ($Arithmetic) case. I assume that these relate to being able to confirm the variant of the operator, unary/binary etc. If you look for "annotate_values", after that is run there is additional information for each character of the current line, tracking the type of the operator. This is used when determining spacing for + etc later as unary ones are typically tight bound and binary ones spaced out. You might find that useful, if that is your issue. And indeed you might find it useful for determining if the +/- you find at line end is indeed unary. Running with --debug values=1 should dump out the variants information for those. > > patch is against 4.0-rc2 (localversion-next is -next-20150505) > > scripts/checkpatch.pl | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 89b1df4..051ea99 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -2577,6 +2577,19 @@ sub process { > "Logical continuations should be on the previous line\n" . $hereprev); > } > > +# check for + or - at the end of a line > + if ($rawline =~ /^\+[^*]*(\+|-)$/) { > + CHK("ARITHMETIC_CONTINUATIONS", > + "Arithmetic expressions should be on one line\n" . $hereprev); > + } > + > +# check for + or - at beginning of a line but exclude ++var/--var > + if (!($rawline =~ /^\+\s*(\+\+|--).*$/) && > + $rawline =~ /^\+\s*(\+|-).*$/) { > + CHK("ARITHMETIC_CONTINUATIONS", > + "Arithmetic expressions should be on one line\n" . $hereprev); > + } > + > # check multi-line statement indentation matches previous line > if ($^V && $^V ge 5.10.0 && > $prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|$Ident\s*=\s*$Ident\s*)\(.*(\&\&|\|\||,)\s*$/) { > -- > 1.7.10.4 -apw -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/