*/ in string confuses checkpatch.pl

*/ in string confuses checkpatch.pl

[penguin-kernel]

Hello.

The below code confuses checkpatch.pl ver 0.21.

Regards.
----------
# cat /tmp/foo.c
void foo(void)
{
	bar(\" /proc/\\\\*/\");
	bar(\" /proc/\\\\$/\");
}
# /usr/src/vanilla/linux-2.6.27.2/scripts/checkpatch.pl --file /tmp/foo.c
ERROR: need consistent spacing around \'/\' (ctx:WxV)
#4: FILE: tmp/foo.c:4:
+       bar(\" /proc/\\\\$/\");
              ^

total: 1 errors, 0 warnings, 5 lines checked

/tmp/foo.c has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Re: */ in string confuses checkpatch.pl

[Andy Whitcroft]

On Tue, Oct 21, 2008 at 09:57:26AM +0900, penguin-kernel@i-love.sakura.ne.jp wrote:
> Hello.
> 
> The below code confuses checkpatch.pl ver 0.21.
> 
> Regards.
> ----------
> # cat /tmp/foo.c
> void foo(void)
> {
> 	bar(\" /proc/\\\\*/\");
> 	bar(\" /proc/\\\\$/\");
> }
> # /usr/src/vanilla/linux-2.6.27.2/scripts/checkpatch.pl --file /tmp/foo.c
> ERROR: need consistent spacing around \'/\' (ctx:WxV)
> #4: FILE: tmp/foo.c:4:
> +       bar(\" /proc/\\\\$/\");
>               ^
> 
> total: 1 errors, 0 warnings, 5 lines checked
> 
> /tmp/foo.c has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.

I am unable to reproduce this here with any version from current back to
v0.19, nor with the one I find in v2.6.27.2 (see below).  Though looking at
the code as presented in your example I can see how it might be interpreted
incorrectly.  That said the compiler doesn't seem to be able to understand
this either (also below).  In particular you effectivly open a quote on
the third line and never close it.

Could you send me both the checkpatch script and the foo.c as attachments
so I can be sure I have them without some emailer somewhere mushing
them up.

Thanks for you report.

-apw

$ cat ../checkpatch/Z213.c
void foo(void)
{
	bar(\" /proc/\\\\*/\");
	bar(\" /proc/\\\\$/\");
}
$ cc -c Z213.c
Z213.c: In function ‘foo’:
Z213.c:3: error: stray ‘\’ in program
Z213.c:3:7: warning: missing terminating " character
Z213.c:3: error: missing terminating " character
Z213.c:4: error: stray ‘\’ in program
Z213.c:4:7: warning: missing terminating " character
Z213.c:4: error: missing terminating " character
Z213.c:5: error: expected expression before ‘}’ token
Z213.c:5: error: expected ‘)’ before ‘}’ token
Z213.c:5: error: expected ‘;’ before ‘}’ token

$ git checkout v2.6.27.2
HEAD is now at 6bcd6d7... Linux 2.6.27.2
apw@brain$ ./scripts/checkpatch.pl  --file ../checkpatch/Z213.c
total: 0 errors, 0 warnings, 5 lines checked

../checkpatch/Z213.c has no obvious style problems and is ready for submission.
$

Re: */ in string confuses checkpatch.pl

[Tetsuo Handa]

Hello.

> I am unable to reproduce this here with any version
I'm sorry. Webmail system broke the source code.

Please see http://lkml.org/lkml/2008/10/21/114 and try to reproduce.

Regards.

Re: */ in string confuses checkpatch.pl

[Andy Whitcroft]

On Wed, Oct 22, 2008 at 08:56:17AM +0100, Andy Whitcroft wrote:
> On Tue, Oct 21, 2008 at 09:57:26AM +0900, penguin-kernel@i-love.sakura.ne.jp wrote:
> > Hello.
> > 
> > The below code confuses checkpatch.pl ver 0.21.
> > 
> > Regards.
> > ----------
> > # cat /tmp/foo.c
> > void foo(void)
> > {
> > 	bar(\" /proc/\\\\*/\");
> > 	bar(\" /proc/\\\\$/\");
> > }
> > # /usr/src/vanilla/linux-2.6.27.2/scripts/checkpatch.pl --file /tmp/foo.c
> > ERROR: need consistent spacing around \'/\' (ctx:WxV)
> > #4: FILE: tmp/foo.c:4:
> > +       bar(\" /proc/\\\\$/\");
> >               ^
> > 
> > total: 1 errors, 0 warnings, 5 lines checked
> > 
> > /tmp/foo.c has style problems, please review.  If any of these errors
> > are false positives report them to the maintainer, see
> > CHECKPATCH in MAINTAINERS.

Ok, I can see whats happened here.  Most of these \'s are extraneous.
Without those I can reproduce this.  Its a bug in the 'comment is open
at the start of hunk' detection.  I think I have updated this heuristic
to cope wit this.  Could you try your original patch (I assume there was
one) with the version below:

  http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/checkpatch.pl-testing

Thanks for your report.

-apw

Re: */ in string confuses checkpatch.pl

[Tetsuo Handa]

Hello.

Andy Whitcroft wrote:
> Ok, I can see whats happened here.  Most of these \'s are extraneous.
> Without those I can reproduce this.  Its a bug in the 'comment is open
> at the start of hunk' detection.  I think I have updated this heuristic
> to cope wit this.  Could you try your original patch (I assume there was
> one) with the version below:
> 
>   http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/checkpatch.pl-testing
> 
> Thanks for your report.
> 
> -apw
> 
It solved this problem.

By the way, I'm using checkpatch.pl for checking userland application.
It would be convenient if there is a option that omits some checks like
'# check for external initialisers.' and '# check for static initialisers.'

Thank you.

[checkpatch.pl] two bugs

[Tetsuo Handa]

Hello.

I think these are bugs.

Regards.
-----

Bug 1:
  assignment in 'if' is not warned if '\n' is inserted before the assignment.

# cat /tmp/test1.c
void foo(void)
{
	int i = 0;
	if (0 ||
	    (i = 0) != 0)
		return;
}
# /usr/src/vanilla/linux-2.6.27.3/scripts/checkpatch.pl --file /tmp/test1.c
total: 0 errors, 0 warnings, 7 lines checked

/tmp/test1.c has no obvious style problems and is ready for submission.
# ./scripts/checkpatch.pl-testing --strict --file /tmp/test1.c
total: 0 errors, 0 warnings, 0 checks, 7 lines checked

/tmp/test1.c has no obvious style problems and is ready for submission.



Bug 2:
  'while' placed after 'if { }' block is not handled properly.

# cat /tmp/test2.c
int i;
void foo(void)
{
	if (!i) {
		i++;
		return;
	}
	while (i);
}
# /usr/src/vanilla/linux-2.6.27.3/scripts/checkpatch.pl --file /tmp/test2.c
ERROR: trailing statements should be on next line
#8: FILE: tmp/test2.c:8:
+       while (i);

ERROR: while should follow close brace '}'
#8: FILE: tmp/test2.c:8:
+       }
+       while (i);

total: 2 errors, 0 warnings, 9 lines checked

/tmp/test2.c has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
# ./scripts/checkpatch.pl-testing --strict --file /tmp/test2.c
ERROR: trailing statements should be on next line
#8: FILE: tmp/test2.c:8:
+       while (i);
+       while (i);
ERROR: while should follow close brace '}'
#8: FILE: tmp/test2.c:8:
+       }
+       while (i);

total: 2 errors, 0 warnings, 0 checks, 9 lines checked

/tmp/test2.c has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Re: [checkpatch.pl] two bugs

[Andy Whitcroft]

On Thu, Oct 23, 2008 at 08:27:34PM +0900, Tetsuo Handa wrote:
> Hello.
> 
> I think these are bugs.
> 
> Regards.
> -----
> 
> Bug 1:
>   assignment in 'if' is not warned if '\n' is inserted before the assignment.

Yep that looks wrong, too line oriented.

> # cat /tmp/test1.c
> void foo(void)
> {
> 	int i = 0;
> 	if (0 ||
> 	    (i = 0) != 0)
> 		return;
> }
> # /usr/src/vanilla/linux-2.6.27.3/scripts/checkpatch.pl --file /tmp/test1.c
> total: 0 errors, 0 warnings, 7 lines checked
> 
> /tmp/test1.c has no obvious style problems and is ready for submission.
> # ./scripts/checkpatch.pl-testing --strict --file /tmp/test1.c
> total: 0 errors, 0 warnings, 0 checks, 7 lines checked
> 
> /tmp/test1.c has no obvious style problems and is ready for submission.
> 
> 
> 
> Bug 2:
>   'while' placed after 'if { }' block is not handled properly.
> 
> # cat /tmp/test2.c
> int i;
> void foo(void)
> {
> 	if (!i) {
> 		i++;
> 		return;
> 	}
> 	while (i);
> }
> # /usr/src/vanilla/linux-2.6.27.3/scripts/checkpatch.pl --file /tmp/test2.c
> ERROR: trailing statements should be on next line
> #8: FILE: tmp/test2.c:8:
> +       while (i);
> 
> ERROR: while should follow close brace '}'
> #8: FILE: tmp/test2.c:8:
> +       }
> +       while (i);
> 
> total: 2 errors, 0 warnings, 9 lines checked
> 
> /tmp/test2.c has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> # ./scripts/checkpatch.pl-testing --strict --file /tmp/test2.c
> ERROR: trailing statements should be on next line
> #8: FILE: tmp/test2.c:8:
> +       while (i);
> +       while (i);

Hmmm, we have done a lot of work here to figure out this is a while
without a do and therefore the ; should be on the next line; yet we then
let the looser check below fire too.  A bug indeed.

> ERROR: while should follow close brace '}'
> #8: FILE: tmp/test2.c:8:
> +       }
> +       while (i);
> 
> total: 2 errors, 0 warnings, 0 checks, 9 lines checked

Thanks for your reports.  Will get them on the tofix list.

-apw