From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752206AbdKWCLT (ORCPT ); Wed, 22 Nov 2017 21:11:19 -0500 Received: from smtprelay0006.hostedemail.com ([216.40.44.6]:44191 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751525AbdKWCLS (ORCPT ); Wed, 22 Nov 2017 21:11:18 -0500 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,joe@perches.com,:::::,RULES_HIT:41:355:379:541:599:800:960:968:973:982:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1431:1437:1515:1516:1518:1534:1544:1593:1594:1605:1711:1730:1747:1777:1792:2110:2194:2198:2199:2200:2393:2559:2562:2828:3138:3139:3140:3141:3142:3622:3653:3865:3866:3867:3868:3870:3871:3872:3873:3874:4250:4321:4605:5007:6117:6119:6691:7903:8784:8957:9108:10004:10848:11026:11232:11473:11658:11914:12043:12291:12295:12296:12438:12555:12740:12760:12895:12986:13161:13229:13255:13439:14181:14659:14721:21067:21080:21212:21324:21433:21451:21611:21627:30003:30034:30054:30070:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:2,LUA_SUMMARY:none X-HE-Tag: verse97_b329e26bed56 X-Filterd-Recvd-Size: 4969 Message-ID: <1511403075.2385.6.camel@perches.com> Subject: Re: [PATCH] checkpatch: Add a warning for log messages that don't end in a line feed From: Joe Perches To: Logan Gunthorpe , linux-kernel@vger.kernel.org Cc: Andy Whitcroft Date: Wed, 22 Nov 2017 18:11:15 -0800 In-Reply-To: <20171122205516.26090-1-logang@deltatee.com> References: <20171122205516.26090-1-logang@deltatee.com> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.26.1-1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-11-22 at 13:55 -0700, Logan Gunthorpe wrote: > Check for lines with a log function using a $logLineFeedFunctions > expression which is similar to the existing $logFunctions expression > except we don't include MODULE and seq_ functions. > > Once an appropriate log function is found, mark that we are in a > log function (for multiline calls). The mark is removed once we see a > line ending in ';' or the end of a patch hunk (similar to $in_comment). > > For lines that are in a log function (including the first and last), > if we see a quoted string that ends in \n, we remove the mark as we are > likely good. Otherwise, if we see a quote followed by a comma or a close > paraenthesis, that isn't preceded by a backslash than it looks like we > have found the end of the format string without a \n and we WARN. > > Signed-off-by: Logan Gunthorpe > Cc: Andy Whitcroft > Cc: Joe Perches > --- > > This is my penance for breaking this rule for a while. > > I've run these changes on a number of patchsets I've submitted and it > seems to perform quite well. > > I've also done my best to try and trick > it with different forms of log messages but I haven't come up with > anything that's a false positive or negative. If anyone's creative > enough to come up with something that does break it I can see if I can > address it. nack. Try running it on the kernel source tree with -f and see what you think. $ git ls-files -- "*.[ch]" | xargs --max-args=20 --max-procs=$(grep -c ^processor /proc/cpuinfo) \ ./scripts/checkpatch.pl -f --quiet --no-summary \ --types=LOGGING_MISSING_LINEFEED There are a lot of false positives. Any printk with a pr_cont/printk(KERN_CONT that follows generates this warning. A lot of macros also add "\n" to the passed format string (e.g.: ext4_warning) Any concatenated format like "foo" ##bar "baz\n" would also get this eror. A couple more comments below: cheers, Joe > scripts/checkpatch.pl | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 8b80bac055e4..917725f36283 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -460,6 +460,13 @@ our $logFunctions = qr{(?x: > seq_vprintf|seq_printf|seq_puts > )}; > > +our $logLineFeedFunctions = qr{(?x: > + printk(?:_ratelimited|_once|_deferred_once|_deferred|)| > + (?:[a-z0-9]+_){1,2}(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)(?:_ratelimited|_once|)| > + WARN(?:_RATELIMIT|_ONCE|)| > + panic > +)}; > + > our $signature_tags = qr{(?xi: > Signed-off-by:| > Acked-by:| > @@ -2202,6 +2209,7 @@ sub process { > my $here = ''; > my $context_function; #undef'd unless there's a known function > my $in_comment = 0; > + my $in_log_function = 0; > my $comment_edge = 0; > my $first_line = 0; > my $p1_prefix = ''; > @@ -2247,6 +2255,7 @@ sub process { > $realcnt=1+1; > } > $in_comment = 0; > + $in_log_function = 0; > > # Guestimate if this is a continuing comment. Run > # the context looking for a comment "edge". If this > @@ -5389,6 +5398,23 @@ sub process { > } > } > > +# check for logging functions with lines that don't end in a '\n"' > + if ($line =~ /\b$logLineFeedFunctions\s*\(/) { > + $in_log_function = 1; > + } > + if ($in_log_function) { > + my $qstr = get_quoted_string($line, $rawline); > + if ($qstr =~ /\\n"$/) { > + $in_log_function = 0; This doesn't work if there are multiple patch fragments. > + } elsif ($line =~ /[^\\]"[,)]/) { > + WARN("LOGGING_MISSING_LINEFEED", LINEFEED isn't correct, NEWLINE please > + "Log messages should end in a line feed (\\n)\n" . $herecurr); > + $in_log_function = 0; > + } elsif ($line =~ /;$/) { > + $in_log_function = 0; > + } > + } > + > # check for logging continuations > if ($line =~ /\bprintk\s*\(\s*KERN_CONT\b|\bpr_cont\s*\(/) { > WARN("LOGGING_CONTINUATION", > -- > 2.11.0