Re: checkpatch false positon on EXPORT_SYMBOL

[Andy Whitcroft]

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