Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756859AbcDLM7K (ORCPT ); Tue, 12 Apr 2016 08:59:10 -0400 Received: from mail-wm0-f41.google.com ([74.125.82.41]:34539 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753372AbcDLM7I (ORCPT ); Tue, 12 Apr 2016 08:59:08 -0400 Date: Tue, 12 Apr 2016 13:59:04 +0100 From: Andy Whitcroft To: Joe Perches Cc: Daniel Walker , open list , Daniel Walker , "xe-kernel@external.cisco.com" Subject: Re: checkpatch false positon on EXPORT_SYMBOL Message-ID: <20160412125904.GB5107@brain> References: <56FD3BAE.1070209@cisco.com> <1459452096.1744.12.camel@perches.com> <570C1C62.1060008@cisco.com> <1460412582.1800.96.camel@perches.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1460412582.1800.96.camel@perches.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4172 Lines: 113 On Mon, Apr 11, 2016 at 03:09:42PM -0700, Joe Perches wrote: > On Mon, 2016-04-11 at 14:51 -0700, Daniel Walker wrote: > > On 03/31/2016 12:21 PM, Joe Perches wrote: > > > > > > On Thu, 2016-03-31 at 08:01 -0700, Daniel Walker wrote: > > > > > > > > The below looks like normal code but the last export symbol gets the > > > > warning, > > > > > > > > > > > > WARNING:EXPORT_SYMBOL: EXPORT_SYMBOL(foo); should immediately follw its > > > > function/variable > > > > #16: FILE: kernel/acct.c:70: > > > > +EXPORT_SYMBOL(test_export);????/* Error ! */ > > > > > > > > It seems to have to do with the comments at the end of the line. The > > > > first two examples don't have warnings because I removed the comments on > > > > different lines. comments on the variable and export symbol lines gets > > > > the error tho. > > > That looks like a false positive I'll leave for Andy. > > > > > > $ cat ~/export_symbol.c > > > int test_export_no_comment; > > > EXPORT_SYMBOL(test_export_no_comment); > > > int test_export_comment_int; /* comment int */ > > > EXPORT_SYMBOL(test_export_int); > > > int test_export_comment_symbol; > > > EXPORT_SYMBOL(test_export_symbol); /* comment symbol */ > > > int test_export_both; /* comment both 1 */ > > > EXPORT_SYMBOL(test_export_both); /* comment both 2 */ > > > $ > > > > > > Something's a bit off with the $stat variable: > > > > > > test_export_int doesn't match the EXPORT_SYMBOL test. > > > test_export_symbol and test_export_both get warnings. > > > > > > > Did this get solved? I haven't see anything else on it. > > Not by me. > > I punted to Andy and I haven't heard from him. > > There aren't many cases of this defect in the current > kernel tree, so I don't know how much he might care. After some debugging it seems we are essentially not finding the appropriate "next line" when we are parsing either of the second or third entries. This leads us to not check the second one at all, and to check the third one only when think we are parsing the comment. This all stems from us thinking there are two statements on the same line as the trailing ; is not actually at the end of line so the next statement is still on this same line. Basically inline comments should be considered as spaces for the purposes of determining the next line for this purpose. The following patch appears to sort this out. A quick scan says this entire next line calculation is still only used for the EXPORT* check so this should be low risk for other tests. This works for me on your example, if you have a real world one could you test it there and let us know. Thanks. -apw >From 223fc7ef4ca0134bf64af0a107532dc3e4010c87 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 12 Apr 2016 13:43:46 +0100 Subject: [PATCH] checkpatch: comments are whitespace for the purposes of finding the next line While parsing statements we are recording the nominal next line for the purposes of checking that EXPORT* follows exactly on below an appropriate statement. Where there is whitespace after a statement end marker (such as ;) we will move to the next line. This also needs to apply to inline comments at the end of a line. Allows us to more correctly parse: +int test_export; +EXPORT_SYMBOL(test_export); /* No Error ! */ + +int test_export2; /* No Error below */ +EXPORT_SYMBOL(test_export2); + +int test_export3; /* Error below */ +EXPORT_SYMBOL(test_export3); /* Error ! */ + Reported-by: Daniel Walker Signed-off-by: Andy Whitcroft --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d574d13..b581529f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3000,7 +3000,7 @@ sub process { $realline_next = $line_nr_next; if (defined $realline_next && (!defined $lines[$realline_next - 1] || - substr($lines[$realline_next - 1], $off_next) =~ /^\s*$/)) { + substr($lines[$realline_next - 1], $off_next) =~ /^($;|\s)*$/)) { $realline_next++; } -- 2.7.4