[PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Steven Rostedt]

The use of kzalloc() is preferred over kmalloc/memset(0) pairs.

When a match is made with "memset(p, 0, s);" a search back through the
patch hunk is made looking for "p = kmalloc(s,". If that is found, then
a warning is given, suggesting to use kzalloc() instead.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 58848e3..f28f0e3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2902,6 +2902,22 @@ sub process {
 		    $line =~ /DEVICE_ATTR.*S_IWUGO/ ) {
 			WARN("Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr);
 		}
+
+# The use of kzalloc() is preferred over kmalloc/memset(0) pairs.
+		if ($line =~ /\smemset\s*\((\S*)\s*,\s*(?:0x0|0)\s*,\s*(\S*)\s*\);/) {
+		    my $ptr = $1;
+		    my $size = $2;
+
+		    for (my $i = $linenr-2; $i >= 0; $i--) {
+			next if ($lines[$i] =~ /^-/); # ignore deletions
+			last if ($lines[$i] =~ /^\@\@/);
+			if ($lines[$i] =~ /\s(\S*)\s*=\s*kmalloc\((\S*)\,/ &&
+			    $1 eq $ptr && $2 eq $size) {
+			    WARN("use kzalloc() instead of kmalloc/memset(p,0,size) pair\n"
+				 . $herecurr);
+			}
+		    }
+		}
 	}
 
 	# If we have no input at all, then there is nothing to report on

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Jonathan Corbet]

On Thu, 17 Mar 2011 22:52:24 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> The use of kzalloc() is preferred over kmalloc/memset(0) pairs.
> 
> When a match is made with "memset(p, 0, s);" a search back through the
> patch hunk is made looking for "p = kmalloc(s,". If that is found, then
> a warning is given, suggesting to use kzalloc() instead.

The Coccinelle stuff already has a lot of this kind of test.  See, for
example, scripts/coccinelle/api/alloc/kzalloc-simple.cocci.  Suppose
there is some way all this nice analysis infrastructure could be
integrated instead of duplicated?  Or am I just a crazy dreamer?

jon

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Steven Rostedt]

On Thu, 2011-03-17 at 21:15 -0600, Jonathan Corbet wrote:
> On Thu, 17 Mar 2011 22:52:24 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > The use of kzalloc() is preferred over kmalloc/memset(0) pairs.
> > 
> > When a match is made with "memset(p, 0, s);" a search back through the
> > patch hunk is made looking for "p = kmalloc(s,". If that is found, then
> > a warning is given, suggesting to use kzalloc() instead.
> 
> The Coccinelle stuff already has a lot of this kind of test.  See, for
> example, scripts/coccinelle/api/alloc/kzalloc-simple.cocci.  Suppose
> there is some way all this nice analysis infrastructure could be
> integrated instead of duplicated?  Or am I just a crazy dreamer?
> 

Hmm, to me Coccinelle is sorta the big hammer approach to this. It can
be used to analyze the kernel code that exists. checkpatch is made to
stop developers from adding new crap.

That said, I wonder if there is a way to make one use information from
the other. Perhaps we can make checkpatch read the Concinelle rules.
Honestly, I've never used Concinelle and have no idea how to use it.
Looking at their web site there's a PDF on how to use something called
"spatch" but it seems more complex than checkpatch.

But if we can teach checkpatch to read the rule files, then maybe that
would be beneficial.

-- Steve

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Dave Jones]

On Thu, Mar 17, 2011 at 09:15:48PM -0600, Jonathan Corbet wrote:
 > On Thu, 17 Mar 2011 22:52:24 -0400
 > Steven Rostedt <rostedt@goodmis.org> wrote:
 > 
 > > The use of kzalloc() is preferred over kmalloc/memset(0) pairs.
 > > 
 > > When a match is made with "memset(p, 0, s);" a search back through the
 > > patch hunk is made looking for "p = kmalloc(s,". If that is found, then
 > > a warning is given, suggesting to use kzalloc() instead.
 > 
 > The Coccinelle stuff already has a lot of this kind of test.  See, for
 > example, scripts/coccinelle/api/alloc/kzalloc-simple.cocci.  Suppose
 > there is some way all this nice analysis infrastructure could be
 > integrated instead of duplicated?  Or am I just a crazy dreamer?

Maybe.  Coccinelle is one of those tools that seems to be perpetually
on my "to look into" list that I never get around to.


Something that has crossed my mind over the last few days was the idea
of splitting checkpatch into two tools.
One for checking CodingStyle issues, and one for checking for actual
code problems like the memset example.

The motivation for such is that I think it's pretty clear that many maintainers
never run checkpatch on patches they queue up before pushing to Linus...

$ scripts/checkpatch.pl  ~/Mail/upstream/2.6.39/head-March-18-2011 | wc -l
2361

The bulk of this is all "missing space here" "don't put a space there" type
fluff that most maintainers just don't care about.  Any valuable warnings
are lost in the noise.  If we had a separate tool to check for real flaws,
(or even a way to suppress the stylistic warnings from the existing one)
maybe more maintainers would run their changes through it.

I dunno, maybe I'm just a crazy dreamer too, but I think many people
have written checkpatch off as useless in its current incarnation.

	Dave

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Américo Wang]

On Fri, Mar 18, 2011 at 11:32 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> But if we can teach checkpatch to read the rule files, then maybe that
> would be beneficial.
>

The problem is Perl doesn't really understand C, while Coccinelle does.

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Pekka Enberg]

Hi,

On Fri, Mar 18, 2011 at 11:32 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>> But if we can teach checkpatch to read the rule files, then maybe that
>> would be beneficial.

On Sun, Mar 20, 2011 at 5:40 AM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> The problem is Perl doesn't really understand C, while Coccinelle does.

It would be nice if Coccinelle would be even more easy to setup and
use during development. Like with Dave, Coccinelle has been on my todo
list forever.

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1200 bytes --]

On Sun, 20 Mar 2011, Pekka Enberg wrote:

> Hi,
> 
> On Fri, Mar 18, 2011 at 11:32 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> >> But if we can teach checkpatch to read the rule files, then maybe that
> >> would be beneficial.
> 
> On Sun, Mar 20, 2011 at 5:40 AM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> > The problem is Perl doesn't really understand C, while Coccinelle does.
> 
> It would be nice if Coccinelle would be even more easy to setup and
> use during development. Like with Dave, Coccinelle has been on my todo
> list forever.

You have to get and set up Coccinelle on your own.  However, it is part of 
many Linux distributions, including ubuntu.

Afterwards, you can run make coccicheck to apply the semantic patches that 
are included in the kernel to the entire kernel.  There are some options 
to eg restrict the choice of rule, the choice of considered Linux files, 
and the format of the output.  Where possible we have taken 
decisions that are common with what is done elsewhere in the make file.

Documentation is in Documentation/coccinelle.txt

Suggestions for how to make it easier to use or the documentation more 
understandable are welcome.

thanks,
julia

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Pekka Enberg]

Hi Julia,

On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <julia@diku.dk> wrote:
> Suggestions for how to make it easier to use or the documentation more
> understandable are welcome.

The benefit of scripts/checkpatch.pl is that it doesn't require any
setting up to do. I'm personally less likely to use Coccinelle (and
Sparse for that matter) on boxes where the software is not installed.
I'm not sure how other people feel about it, but I'd personally love
to see tools/coccinelle and tools/sparse.

As for something more concrete, I guess this is what I'm mostly interested in:

penberg@jaguar:~/src/linux$ make C=1 CHECK="scripts/coccicheck" mm/slub.o

I guess it'd be good to document that for 'make help' because now you
need to dig through Documentation/coccinelle.txt to find it.

P.S. It seems there's a bug somewhere because the above command fails
miserably for me:

  CHK     include/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh
  CHECK   mm/slub.c
File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
line 32, column 5,  charpos = 747
    around = '<+...', whole content = -    <+... when != goto l2;
Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
context: <+...")
File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
line 32, column 5,  charpos = 747
    around = '<+...', whole content = -    <+... when != goto l2;
Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
context: <+...")
File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
line 32, column 5,  charpos = 747
    around = '<+...', whole content = -    <+... when != goto l2;
Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
context: <+...")
File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
line 32, column 5,  charpos = 747
    around = '<+...', whole content = -    <+... when != goto l2;
Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
context: <+...")
make[1]: *** [mm/slub.o] Error 1
make: *** [mm/slub.o] Error 2

penberg@jaguar:~/src/linux$ dpkg -l|grep coccinelle
ii  coccinelle                                      0.2.2.deb-2
                                        semantic patching tool for C

[ I'm on Ubuntu 10.10. ]

                        Pekka

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Julia Lawall]

On Sun, 20 Mar 2011, Pekka Enberg wrote:

> Hi Julia,
> 
> On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <julia@diku.dk> wrote:
> > Suggestions for how to make it easier to use or the documentation more
> > understandable are welcome.
> 
> The benefit of scripts/checkpatch.pl is that it doesn't require any
> setting up to do. I'm personally less likely to use Coccinelle (and
> Sparse for that matter) on boxes where the software is not installed.
> I'm not sure how other people feel about it, but I'd personally love
> to see tools/coccinelle and tools/sparse.

This was discussed before, and it was felt that perhaps 75000 lines of 
ocaml code was not really appropriate for the Linux source tree, and also 
that it would too much complicate our development process.

One reason for using multiple machines would be to work on multiple 
architectures.  But Coccinelle is not sensitive to the architecture on 
which it is run, so perhaps you do't need to have it installed everywhere.

> As for something more concrete, I guess this is what I'm mostly interested in:
> 
> penberg@jaguar:~/src/linux$ make C=1 CHECK="scripts/coccicheck" mm/slub.o
> 
> I guess it'd be good to document that for 'make help' because now you
> need to dig through Documentation/coccinelle.txt to find it.

OK, thanks.  We will look into that.

> P.S. It seems there's a bug somewhere because the above command fails
> miserably for me:
> 
>   CHK     include/linux/version.h
>   CHK     include/generated/utsrelease.h
>   CALL    scripts/checksyscalls.sh
>   CHECK   mm/slub.c
> File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> line 32, column 5,  charpos = 747
>     around = '<+...', whole content = -    <+... when != goto l2;
> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> context: <+...")
> File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> line 32, column 5,  charpos = 747
>     around = '<+...', whole content = -    <+... when != goto l2;
> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> context: <+...")
> File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> line 32, column 5,  charpos = 747
>     around = '<+...', whole content = -    <+... when != goto l2;
> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> context: <+...")
> File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> line 32, column 5,  charpos = 747
>     around = '<+...', whole content = -    <+... when != goto l2;
> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> context: <+...")
> make[1]: *** [mm/slub.o] Error 1
> make: *** [mm/slub.o] Error 2
> 
> penberg@jaguar:~/src/linux$ dpkg -l|grep coccinelle
> ii  coccinelle                                      0.2.2.deb-2
>                                         semantic patching tool for C
> 
> [ I'm on Ubuntu 10.10. ]

Indeed that one seems to be quite out of date.  You can get the most 
recent version here: https://launchpad.net/~npalix/+archive/coccinelle

julia

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Ingo Molnar]

* Julia Lawall <julia@diku.dk> wrote:

> On Sun, 20 Mar 2011, Pekka Enberg wrote:
> 
> > Hi Julia,
> > 
> > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <julia@diku.dk> wrote:
> > > Suggestions for how to make it easier to use or the documentation more
> > > understandable are welcome.
> > 
> > The benefit of scripts/checkpatch.pl is that it doesn't require any
> > setting up to do. I'm personally less likely to use Coccinelle (and
> > Sparse for that matter) on boxes where the software is not installed.
> > I'm not sure how other people feel about it, but I'd personally love
> > to see tools/coccinelle and tools/sparse.
> 
> This was discussed before, and it was felt that perhaps 75000 lines of 
> ocaml code was not really appropriate for the Linux source tree, [...]

We have drivers and rare architectures that have a (much) larger size and which 
are only useful to <0.01% of Linux users.

Coccinelle on the other hand is useful to 100% of Linux users.

We even have C++, perl and python code in the kernel repo so there's no 
language bigotry either - use the language that works best for your project.

> [...] and also that it would too much complicate our development process.

'our' as in the Linux kernel development process? I really don't think it's an 
issue - see above.

or 'our' as in Coccinelle development process? When we brought tools/perf/ into 
the kernel repo all it forced on us were sane Git commits and a predictable, 
(modulo-) 3 months release/stabilization cycle. Both constraints served the 
quality of the perf project very well - but of course your milage may vary.

> One reason for using multiple machines would be to work on multiple 
> architectures.  But Coccinelle is not sensitive to the architecture on 
> which it is run, so perhaps you do't need to have it installed everywhere.

I think the point Pekka tried to make is to have it integrated into the kbuild 
mechanism as well at a certain point. That way it's very easy to use it and we 
maintainers could require frequent patch submitters to use those tools to check 
the quality of their patches. Right now i cannot require that, as it's not part 
of the kernel repo. Requiring a checkpatch.pl check is much easier, as it's 
available to everyone who is writing kernel patches.

'The power of the default' is a very strong force.

Also, IIRC Thomas also sometimes uses Coccinelle to generate *patches*, so 
having it in the kernel would also help that kind of usage.

> > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > line 32, column 5,  charpos = 747
> >     around = '<+...', whole content = -    <+... when != goto l2;
> > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > context: <+...")
> > make[1]: *** [mm/slub.o] Error 1
> > make: *** [mm/slub.o] Error 2
> > 
> > penberg@jaguar:~/src/linux$ dpkg -l|grep coccinelle
> > ii  coccinelle                                      0.2.2.deb-2
> >                                         semantic patching tool for C
> > 
> > [ I'm on Ubuntu 10.10. ]
> 
> Indeed that one seems to be quite out of date.  You can get the most 
> recent version here: https://launchpad.net/~npalix/+archive/coccinelle

With tools/coccinelle/ you would never run into such problems of distributing 
the latest stable version to your fellow kernel developers: it would always be 
available in tools/coccinelle/.

Integration, synergy, availability, distribution and half a dozen other 
buzzwords come to mind as to why it's a good idea to have kernel-focused
tools hosted in the kernel repo :-)

IMO it's an option to consider.

Thanks,

	Ingo

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Pekka Enberg]

Hi,

On Sun, Mar 20, 2011 at 12:54 PM, Ingo Molnar <mingo@elte.hu> wrote:
>> Indeed that one seems to be quite out of date.  You can get the most
>> recent version here: https://launchpad.net/~npalix/+archive/coccinelle
>
> With tools/coccinelle/ you would never run into such problems of distributing
> the latest stable version to your fellow kernel developers: it would always be
> available in tools/coccinelle/.
>
> Integration, synergy, availability, distribution and half a dozen other
> buzzwords come to mind as to why it's a good idea to have kernel-focused
> tools hosted in the kernel repo :-)
>
> IMO it's an option to consider.

That's my thinking too. Yes, 80 KLOC of OCaml in the kernel tree
sounds crazy but I think the practical advantages might be enough to
justify it. Btw, would git-submodule be something to consider here?

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Alexey Dobriyan]

Can we tell "0" token from "sizeof(struct {})" in C compiler?

This would solve memset(,0); issue quite nicely.

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Nicolas Palix]

Hi,

On Sun, Mar 20, 2011 at 12:06 PM, Pekka Enberg <penberg@kernel.org> wrote:
> Hi,
>
> On Sun, Mar 20, 2011 at 12:54 PM, Ingo Molnar <mingo@elte.hu> wrote:
>>> Indeed that one seems to be quite out of date.  You can get the most
>>> recent version here: https://launchpad.net/~npalix/+archive/coccinelle
>>
>> With tools/coccinelle/ you would never run into such problems of distributing
>> the latest stable version to your fellow kernel developers: it would always be
>> available in tools/coccinelle/.
>>
>> Integration, synergy, availability, distribution and half a dozen other
>> buzzwords come to mind as to why it's a good idea to have kernel-focused
>> tools hosted in the kernel repo :-)

Our usage is mainly kernel-focused but not the tool. It is C-program focused
and we have used it on other programs like Wine, OpenSSL, VLC. Others
use it on other projects like Davecot or close-source projets.
So, IMHO Coccinelle should no be part of Linux.

Integrating kernel-focused SmPL scripts is on the other hand a great idea
to check the kernel and to ease kernel developer life.
It is what have been done so far. It is certainly possible to improve that,
at least by adding more and more scripts.

>>
>> IMO it's an option to consider.
>
> That's my thinking too. Yes, 80 KLOC of OCaml in the kernel tree
> sounds crazy but I think the practical advantages might be enough to
> justify it. Btw, would git-submodule be something to consider here?
>

At every RC, we push the Coccinelle code on github. Using git-submodule
seems the way to go thus. Moreover, it will ease the maintenance of
scripts as we may assume users have one of the latest versions.


-- 
Nicolas Palix
http://sardes.inrialpes.fr/~npalix/

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1963 bytes --]

On Sun, 20 Mar 2011, Nicolas Palix wrote:

> Hi,
> 
> On Sun, Mar 20, 2011 at 12:06 PM, Pekka Enberg <penberg@kernel.org> wrote:
> > Hi,
> >
> > On Sun, Mar 20, 2011 at 12:54 PM, Ingo Molnar <mingo@elte.hu> wrote:
> >>> Indeed that one seems to be quite out of date.  You can get the most
> >>> recent version here: https://launchpad.net/~npalix/+archive/coccinelle
> >>
> >> With tools/coccinelle/ you would never run into such problems of distributing
> >> the latest stable version to your fellow kernel developers: it would always be
> >> available in tools/coccinelle/.
> >>
> >> Integration, synergy, availability, distribution and half a dozen other
> >> buzzwords come to mind as to why it's a good idea to have kernel-focused
> >> tools hosted in the kernel repo :-)
> 
> Our usage is mainly kernel-focused but not the tool. It is C-program focused
> and we have used it on other programs like Wine, OpenSSL, VLC. Others
> use it on other projects like Davecot or close-source projets.
> So, IMHO Coccinelle should no be part of Linux.
> 
> Integrating kernel-focused SmPL scripts is on the other hand a great idea
> to check the kernel and to ease kernel developer life.
> It is what have been done so far. It is certainly possible to improve that,
> at least by adding more and more scripts.
> 
> >>
> >> IMO it's an option to consider.
> >
> > That's my thinking too. Yes, 80 KLOC of OCaml in the kernel tree
> > sounds crazy but I think the practical advantages might be enough to
> > justify it. Btw, would git-submodule be something to consider here?
> >
> 
> At every RC, we push the Coccinelle code on github. Using git-submodule
> seems the way to go thus. Moreover, it will ease the maintenance of
> scripts as we may assume users have one of the latest versions.

The latter is indeed the more serious problem.  But even if the Coccinelle 
code is available, the person will still have to have ocaml installed to 
be able to compile it.

julia

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Julia Lawall]

> > [...] and also that it would too much complicate our development process.
> 
> 'our' as in the Linux kernel development process? I really don't think it's an 
> issue - see above.
> 
> or 'our' as in Coccinelle development process? When we brought tools/perf/ into 
> the kernel repo all it forced on us were sane Git commits and a predictable, 
> (modulo-) 3 months release/stabilization cycle. Both constraints served the 
> quality of the perf project very well - but of course your milage may vary.

Yes, I meant the Coccinelle development process.

> > One reason for using multiple machines would be to work on multiple 
> > architectures.  But Coccinelle is not sensitive to the architecture on 
> > which it is run, so perhaps you do't need to have it installed everywhere.
> 
> I think the point Pekka tried to make is to have it integrated into the kbuild 
> mechanism as well at a certain point. That way it's very easy to use it and we 
> maintainers could require frequent patch submitters to use those tools to check 
> the quality of their patches. Right now i cannot require that, as it's not part 
> of the kernel repo. Requiring a checkpatch.pl check is much easier, as it's 
> available to everyone who is writing kernel patches.

There remains the problem that if it is just the sources that are part of 
the kernel, the user has to have the ocaml compiler installed.

julia

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Alejandro Riveira Fernández]

El Sun, 20 Mar 2011 11:54:12 +0100
Ingo Molnar <mingo@elte.hu> escribió:


> 
> I think the point Pekka tried to make is to have it integrated into the kbuild 
> mechanism as well at a certain point. That way it's very easy to use it and we 
> maintainers could require frequent patch submitters to use those tools to check 
> the quality of their patches. Right now i cannot require that, as it's not part 
> of the kernel repo. Requiring a checkpatch.pl check is much easier, as it's 
> available to everyone who is writing kernel patches.

 But a perl interpreter is not shipped with the kernel, is it? neither a
 posix shell or a python interpreter ...
 The scripts are already shipped with the kernel, the "interpreter" have
 to be installed in the dev machine like you have to install perl, make,
 gcc, etc
 
 just my two cents as a bystander :)

> 
> Thanks,
> 
> 	Ingo
> --

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Steven Rostedt]

On Sun, 2011-03-20 at 15:48 +0100, Alejandro Riveira Fernández wrote:
> El Sun, 20 Mar 2011 11:54:12 +0100
> Ingo Molnar <mingo@elte.hu> escribió:
> 

>  But a perl interpreter is not shipped with the kernel, is it? neither a
>  posix shell or a python interpreter ...
>  The scripts are already shipped with the kernel, the "interpreter" have
>  to be installed in the dev machine like you have to install perl, make,
>  gcc, etc

We don't ship gcc either. Note, there are some cases where you need a
perl interpreter to build the kernel.

-- Steve

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Steven Rostedt]

On Sat, 2011-03-19 at 15:39 -0400, Dave Jones wrote:

> Something that has crossed my mind over the last few days was the idea
> of splitting checkpatch into two tools.
> One for checking CodingStyle issues, and one for checking for actual
> code problems like the memset example.
> 
> The motivation for such is that I think it's pretty clear that many maintainers
> never run checkpatch on patches they queue up before pushing to Linus...
> 
> $ scripts/checkpatch.pl  ~/Mail/upstream/2.6.39/head-March-18-2011 | wc -l
> 2361
> 

I run it on all my patches. But there are some warnings that I ignore.
Sometimes I don't split the 80char lines if doing so makes the code even
uglier.


> The bulk of this is all "missing space here" "don't put a space there" type
> fluff that most maintainers just don't care about.  Any valuable warnings
> are lost in the noise.  If we had a separate tool to check for real flaws,
> (or even a way to suppress the stylistic warnings from the existing one)
> maybe more maintainers would run their changes through it.

As I replied with my phone, having a suppress warnings would be nice.

checkpatch -e patch

where -e is errors only?

> 
> I dunno, maybe I'm just a crazy dreamer too, but I think many people
> have written checkpatch off as useless in its current incarnation.

Well I and I'm sure Ingo use it quite a bit. I'm sure there are others
that do too.

I would really like to disable warnings, as the patches to my magic
macros break all sorts of checkpatch formatting rules, but real errors
may still exist. And because of that, I seldom use checkpatch on patches
that modify those macros.

Note, I've even found myself running checkpatch on non Linux code too ;)

-- Steve

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Steven Rostedt]

Also, there seems to be one important thing missing from this thread.
Can we please apply my patch?

It may be months or years before we decide how to handle this checkpatch
vs Coccinelle and I think it is important that we flag this as a
warning/error.

-- Steve

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Borislav Petkov]

On Mon, Mar 21, 2011 at 09:26:20AM -0400, Steven Rostedt wrote:
> On Sat, 2011-03-19 at 15:39 -0400, Dave Jones wrote:
> 
> > Something that has crossed my mind over the last few days was the idea
> > of splitting checkpatch into two tools.
> > One for checking CodingStyle issues, and one for checking for actual
> > code problems like the memset example.
> > 
> > The motivation for such is that I think it's pretty clear that many maintainers
> > never run checkpatch on patches they queue up before pushing to Linus...
> > 
> > $ scripts/checkpatch.pl  ~/Mail/upstream/2.6.39/head-March-18-2011 | wc -l
> > 2361
> > 
> 
> I run it on all my patches. But there are some warnings that I ignore.
> Sometimes I don't split the 80char lines if doing so makes the code even
> uglier.

Same here, certain warnings checkpatch spits are plain dumb in certain
situations. What we want is to fix checkpatch to warn/error out only
on real issues which are valid 100% of the cases and stick all the
remaining, not-always-an-issue stuff which deserves a warning only
sometimes behind a "--pedantic" or "--extended-checks" option or
whatever.

> > The bulk of this is all "missing space here" "don't put a space there" type
> > fluff that most maintainers just don't care about.  Any valuable warnings
> > are lost in the noise.  If we had a separate tool to check for real flaws,
> > (or even a way to suppress the stylistic warnings from the existing one)
> > maybe more maintainers would run their changes through it.
> 
> As I replied with my phone, having a suppress warnings would be nice.
> 
> checkpatch -e patch
> 
> where -e is errors only?

Yeah, let's enable the errors-only checking by default, i.e. no args and if you
want additional ones, you need to switch on an option.

> > 
> > I dunno, maybe I'm just a crazy dreamer too, but I think many people
> > have written checkpatch off as useless in its current incarnation.
> 
> Well I and I'm sure Ingo use it quite a bit. I'm sure there are others
> that do too.

Yep, I have it as a pre-commit hook in git and it helps a lot. However,
sometimes I need to do '--no-verify' on false positives.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

checkpatch: introduce --nocs to disable CodingStyle warnings.

[Dave Jones]

 > I would really like to disable warnings, as the patches to my magic
 > macros break all sorts of checkpatch formatting rules, but real errors
 > may still exist. 

How about this ?  running checkpatch with --nocs will now only print
non-CodingStyle related warnings.   I may have missed some of the
conversions, but this seems to silence the more egregious 'noise'.

Signed-off-by: Dave Jones <davej@redhat.com>

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 58848e3..a2b0086 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -19,6 +19,7 @@ my $tree = 1;
 my $chk_signoff = 1;
 my $chk_patch = 1;
 my $tst_only;
+my $nocs = 0;
 my $emacs = 0;
 my $terse = 0;
 my $file = 0;
@@ -55,6 +56,7 @@ Options:
                              is all off)
   --test-only=WORD           report only warnings/errors containing WORD
                              literally
+  --nocs                     don't display warnings about CodingStyle.
   -h, --help, --version      display this help and exit
 
 When FILE is - read standard input.
@@ -80,6 +82,7 @@ GetOptions(
 
 	'debug=s'	=> \%debug,
 	'test-only=s'	=> \$tst_only,
+	'nocs'		=> \$nocs,
 	'h|help'	=> \$help,
 	'version'	=> \$help
 ) or help(1);
@@ -1113,6 +1116,22 @@ sub WARN {
 		our $cnt_warn++;
 	}
 }
+sub CS_ERROR {
+	if ($nocs == 0) {
+		if (report("ERROR: $_[0]\n")) {
+			our $clean = 0;
+			our $cnt_error++;
+		}
+	}
+}
+sub CS_WARN {
+	if ($nocs == 0) {
+		if (report("WARNING: $_[0]\n")) {
+			our $clean = 0;
+			our $cnt_warn++;
+		}
+	}
+}
 sub CHK {
 	if ($check && report("CHECK: $_[0]\n")) {
 		our $clean = 0;
@@ -1417,11 +1436,11 @@ sub process {
 #trailing whitespace
 		if ($line =~ /^\+.*\015/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			ERROR("DOS line endings\n" . $herevet);
+			CS_ERROR("DOS line endings\n" . $herevet);
 
 		} elsif ($rawline =~ /^\+.*\S\s+$/ || $rawline =~ /^\+\s+$/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			ERROR("trailing whitespace\n" . $herevet);
+			CS_ERROR("trailing whitespace\n" . $herevet);
 			$rpt_cleaners = 1;
 		}
 
@@ -1466,17 +1485,17 @@ sub process {
 		    $line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) &&
 		    $length > 80)
 		{
-			WARN("line over 80 characters\n" . $herecurr);
+			CS_WARN("line over 80 characters\n" . $herecurr);
 		}
 
 # check for spaces before a quoted newline
 		if ($rawline =~ /^.*\".*\s\\n/) {
-			WARN("unnecessary whitespace before a quoted newline\n" . $herecurr);
+			CS_WARN("unnecessary whitespace before a quoted newline\n" . $herecurr);
 		}
 
 # check for adding lines without a newline.
 		if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/) {
-			WARN("adding a line without newline at end of file\n" . $herecurr);
+			CS_WARN("adding a line without newline at end of file\n" . $herecurr);
 		}
 
 # Blackfin: use hi/lo macros
@@ -1499,14 +1518,14 @@ sub process {
 		if ($rawline =~ /^\+\s* \t\s*\S/ ||
 		    $rawline =~ /^\+\s*        \s*/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			ERROR("code indent should use tabs where possible\n" . $herevet);
+			CS_ERROR("code indent should use tabs where possible\n" . $herevet);
 			$rpt_cleaners = 1;
 		}
 
 # check for space before tabs.
 		if ($rawline =~ /^\+/ && $rawline =~ / \t/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			WARN("please, no space before tabs\n" . $herevet);
+			CS_WARN("please, no space before tabs\n" . $herevet);
 		}
 
 # check for spaces at the beginning of a line.
@@ -1516,7 +1535,7 @@ sub process {
 #  3) hanging labels
 		if ($rawline =~ /^\+ / && $line !~ /\+ *(?:$;|#|$Ident:)/)  {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			WARN("please, no spaces at the start of a line\n" . $herevet);
+			CS_WARN("please, no spaces at the start of a line\n" . $herevet);
 		}
 
 # check we are in a valid C source file if not then ignore this hunk
@@ -1623,7 +1642,7 @@ sub process {
 				}
 			}
 			if ($err ne '') {
-				ERROR("switch and case should be at the same indent\n$hereline$err");
+				CS_ERROR("switch and case should be at the same indent\n$hereline$err");
 			}
 		}
 
@@ -1651,7 +1670,7 @@ sub process {
 			#print "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n";
 
 			if ($ctx !~ /{\s*/ && defined($lines[$ctx_ln -1]) && $lines[$ctx_ln - 1] =~ /^\+\s*{/) {
-				ERROR("that open brace { should be on the previous line\n" .
+				CS_ERROR("that open brace { should be on the previous line\n" .
 					"$here\n$ctx\n$rawlines[$ctx_ln - 1]\n");
 			}
 			if ($level == 0 && $pre_ctx !~ /}\s*while\s*\($/ &&
@@ -1790,7 +1809,7 @@ sub process {
 # check for initialisation to aggregates open brace on the next line
 		if ($line =~ /^.\s*{/ &&
 		    $prevline =~ /(?:^|[^=])=\s*$/) {
-			ERROR("that open brace { should be on the previous line\n" . $hereprev);
+			CS_ERROR("that open brace { should be on the previous line\n" . $hereprev);
 		}
 
 #
@@ -1808,7 +1827,7 @@ sub process {
 
 # no C99 // comments
 		if ($line =~ m{//}) {
-			ERROR("do not use C99 // comments\n" . $herecurr);
+			CS_ERROR("do not use C99 // comments\n" . $herecurr);
 		}
 		# Remove C99 comments.
 		$line =~ s@//.*@@;
@@ -1911,7 +1930,7 @@ sub process {
 
 			#print "from<$from> to<$to>\n";
 			if ($from ne $to) {
-				ERROR("\"(foo$from)\" should be \"(foo$to)\"\n" .  $herecurr);
+				CS_ERROR("\"(foo$from)\" should be \"(foo$to)\"\n" .  $herecurr);
 			}
 		} elsif ($line =~ m{\b$NonptrType(\s*(?:$Modifier\b\s*|\*\s*)+)($Ident)}) {
 			my ($from, $to, $ident) = ($1, $1, $2);
@@ -1928,7 +1947,7 @@ sub process {
 
 			#print "from<$from> to<$to> ident<$ident>\n";
 			if ($from ne $to && $ident !~ /^$Modifier$/) {
-				ERROR("\"foo${from}bar\" should be \"foo${to}bar\"\n" .  $herecurr);
+				CS_ERROR("\"foo${from}bar\" should be \"foo${to}bar\"\n" .  $herecurr);
 			}
 		}
 
@@ -1970,18 +1989,18 @@ sub process {
 # or if closed on same line
 		if (($line=~/$Type\s*$Ident\(.*\).*\s{/) and
 		    !($line=~/\#\s*define.*do\s{/) and !($line=~/}/)) {
-			ERROR("open brace '{' following function declarations go on the next line\n" . $herecurr);
+			CS_ERROR("open brace '{' following function declarations go on the next line\n" . $herecurr);
 		}
 
 # open braces for enum, union and struct go on the same line.
 		if ($line =~ /^.\s*{/ &&
 		    $prevline =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?\s*$/) {
-			ERROR("open brace '{' following $1 go on the same line\n" . $hereprev);
+			CS_ERROR("open brace '{' following $1 go on the same line\n" . $hereprev);
 		}
 
 # missing space after union, struct or enum definition
 		if ($line =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?(?:\s+$Ident)?[=\{]/) {
-		    WARN("missing space after $1 definition\n" . $herecurr);
+		    CS_WARN("missing space after $1 definition\n" . $herecurr);
 		}
 
 # check for spacing round square brackets; allowed:
@@ -1993,7 +2012,7 @@ sub process {
 			if ($prefix !~ /$Type\s+$/ &&
 			    ($where != 0 || $prefix !~ /^.\s+$/) &&
 			    $prefix !~ /{\s+$/) {
-				ERROR("space prohibited before open square bracket '['\n" . $herecurr);
+				CS_ERROR("space prohibited before open square bracket '['\n" . $herecurr);
 			}
 		}
 
@@ -2024,7 +2043,7 @@ sub process {
 			} elsif ($ctx =~ /$Type$/) {
 
 			} else {
-				WARN("space prohibited between function name and open parenthesis '('\n" . $herecurr);
+				CS_WARN("space prohibited between function name and open parenthesis '('\n" . $herecurr);
 			}
 		}
 # Check operator spacing.
@@ -2098,7 +2117,7 @@ sub process {
 				} elsif ($op eq ';') {
 					if ($ctx !~ /.x[WEBC]/ &&
 					    $cc !~ /^\\/ && $cc !~ /^;/) {
-						ERROR("space required after that '$op' $at\n" . $hereptr);
+						CS_ERROR("space required after that '$op' $at\n" . $hereptr);
 					}
 
 				# // is a comment
@@ -2109,13 +2128,13 @@ sub process {
 				#   :   when part of a bitfield
 				} elsif ($op eq '->' || $opv eq ':B') {
 					if ($ctx =~ /Wx.|.xW/) {
-						ERROR("spaces prohibited around that '$op' $at\n" . $hereptr);
+						CS_ERROR("spaces prohibited around that '$op' $at\n" . $hereptr);
 					}
 
 				# , must have a space on the right.
 				} elsif ($op eq ',') {
 					if ($ctx !~ /.x[WEC]/ && $cc !~ /^}/) {
-						ERROR("space required after that '$op' $at\n" . $hereptr);
+						CS_ERROR("space required after that '$op' $at\n" . $hereptr);
 					}
 
 				# '*' as part of a type definition -- reported already.
@@ -2129,26 +2148,26 @@ sub process {
 					 $opv eq '*U' || $opv eq '-U' ||
 					 $opv eq '&U' || $opv eq '&&U') {
 					if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
-						ERROR("space required before that '$op' $at\n" . $hereptr);
+						CS_ERROR("space required before that '$op' $at\n" . $hereptr);
 					}
 					if ($op eq '*' && $cc =~/\s*$Modifier\b/) {
 						# A unary '*' may be const
 
 					} elsif ($ctx =~ /.xW/) {
-						ERROR("space prohibited after that '$op' $at\n" . $hereptr);
+						CS_ERROR("space prohibited after that '$op' $at\n" . $hereptr);
 					}
 
 				# unary ++ and unary -- are allowed no space on one side.
 				} elsif ($op eq '++' or $op eq '--') {
 					if ($ctx !~ /[WEOBC]x[^W]/ && $ctx !~ /[^W]x[WOBEC]/) {
-						ERROR("space required one side of that '$op' $at\n" . $hereptr);
+						CS_ERROR("space required one side of that '$op' $at\n" . $hereptr);
 					}
 					if ($ctx =~ /Wx[BE]/ ||
 					    ($ctx =~ /Wx./ && $cc =~ /^;/)) {
-						ERROR("space prohibited before that '$op' $at\n" . $hereptr);
+						CS_ERROR("space prohibited before that '$op' $at\n" . $hereptr);
 					}
 					if ($ctx =~ /ExW/) {
-						ERROR("space prohibited after that '$op' $at\n" . $hereptr);
+						CS_ERROR("space prohibited after that '$op' $at\n" . $hereptr);
 					}
 
 
@@ -2160,7 +2179,7 @@ sub process {
 					 $op eq '%')
 				{
 					if ($ctx =~ /Wx[^WCE]|[^WCE]xW/) {
-						ERROR("need consistent spacing around '$op' $at\n" .
+						CS_ERROR("need consistent spacing around '$op' $at\n" .
 							$hereptr);
 					}
 
@@ -2168,7 +2187,7 @@ sub process {
 				# terminating a case value or a label.
 				} elsif ($opv eq ':C' || $opv eq ':L') {
 					if ($ctx =~ /Wx./) {
-						ERROR("space prohibited before that '$op' $at\n" . $hereptr);
+						CS_ERROR("space prohibited before that '$op' $at\n" . $hereptr);
 					}
 
 				# All the others need spaces both sides.
@@ -2191,7 +2210,7 @@ sub process {
 					}
 
 					if ($ok == 0) {
-						ERROR("spaces required around that '$op' $at\n" . $hereptr);
+						CS_ERROR("spaces required around that '$op' $at\n" . $hereptr);
 					}
 				}
 				$off += length($elements[$n + 1]);
@@ -2221,38 +2240,38 @@ sub process {
 #need space before brace following if, while, etc
 		if (($line =~ /\(.*\){/ && $line !~ /\($Type\){/) ||
 		    $line =~ /do{/) {
-			ERROR("space required before the open brace '{'\n" . $herecurr);
+			CS_ERROR("space required before the open brace '{'\n" . $herecurr);
 		}
 
 # closing brace should have a space following it when it has anything
 # on the line
 		if ($line =~ /}(?!(?:,|;|\)))\S/) {
-			ERROR("space required after that close brace '}'\n" . $herecurr);
+			CS_ERROR("space required after that close brace '}'\n" . $herecurr);
 		}
 
 # check spacing on square brackets
 		if ($line =~ /\[\s/ && $line !~ /\[\s*$/) {
-			ERROR("space prohibited after that open square bracket '['\n" . $herecurr);
+			CS_ERROR("space prohibited after that open square bracket '['\n" . $herecurr);
 		}
 		if ($line =~ /\s\]/) {
-			ERROR("space prohibited before that close square bracket ']'\n" . $herecurr);
+			CS_ERROR("space prohibited before that close square bracket ']'\n" . $herecurr);
 		}
 
 # check spacing on parentheses
 		if ($line =~ /\(\s/ && $line !~ /\(\s*(?:\\)?$/ &&
 		    $line !~ /for\s*\(\s+;/) {
-			ERROR("space prohibited after that open parenthesis '('\n" . $herecurr);
+			CS_ERROR("space prohibited after that open parenthesis '('\n" . $herecurr);
 		}
 		if ($line =~ /(\s+)\)/ && $line !~ /^.\s*\)/ &&
 		    $line !~ /for\s*\(.*;\s+\)/ &&
 		    $line !~ /:\s+\)/) {
-			ERROR("space prohibited before that close parenthesis ')'\n" . $herecurr);
+			CS_ERROR("space prohibited before that close parenthesis ')'\n" . $herecurr);
 		}
 
 #goto labels aren't indented, allow a single space however
 		if ($line=~/^.\s+[A-Za-z\d_]+:(?![0-9]+)/ and
 		   !($line=~/^. [A-Za-z\d_]+:/) and !($line=~/^.\s+default:/)) {
-			WARN("labels should not be indented\n" . $herecurr);
+			CS_WARN("labels should not be indented\n" . $herecurr);
 		}
 
 # Return is not a function.
@@ -2271,10 +2290,10 @@ sub process {
 			}
 #print "value<$value>\n";
 			if ($value =~ /^\s*(?:$Ident|-?$Constant)\s*$/) {
-				ERROR("return is not a function, parentheses are not required\n" . $herecurr);
+				CS_ERROR("return is not a function, parentheses are not required\n" . $herecurr);
 
 			} elsif ($spacing !~ /\s+/) {
-				ERROR("space required before the open parenthesis '('\n" . $herecurr);
+				CS_ERROR("space required before the open parenthesis '('\n" . $herecurr);
 			}
 		}
 # Return of what appears to be an errno should normally be -'ve
@@ -2287,7 +2306,7 @@ sub process {
 
 # Need a space before open parenthesis after if, while etc
 		if ($line=~/\b(if|while|for|switch)\(/) {
-			ERROR("space required before the open parenthesis '('\n" . $herecurr);
+			CS_ERROR("space required before the open parenthesis '('\n" . $herecurr);
 		}
 
 # Check for illegal assignment in if conditional -- and check for trailing
@@ -2337,7 +2356,7 @@ sub process {
 					$stat_real = "[...]\n$stat_real";
 				}
 
-				ERROR("trailing statements should be on next line\n" . $herecurr . $stat_real);
+				CS_ERROR("trailing statements should be on next line\n" . $herecurr . $stat_real);
 			}
 		}
 
@@ -2383,7 +2402,7 @@ sub process {
 		# indent level to be relevant to each other.
 		if ($prevline=~/}\s*$/ and $line=~/^.\s*else\s*/ and
 						$previndent == $indent) {
-			ERROR("else should follow close brace '}'\n" . $hereprev);
+			CS_ERROR("else should follow close brace '}'\n" . $hereprev);
 		}
 
 		if ($prevline=~/}\s*$/ and $line=~/^.\s*while\s*/ and
@@ -2396,7 +2415,7 @@ sub process {
 			$s =~ s/\n.*//g;
 
 			if ($s =~ /^\s*;/) {
-				ERROR("while should follow close brace '}'\n" . $hereprev);
+				CS_ERROR("while should follow close brace '}'\n" . $hereprev);
 			}
 		}
 
@@ -2576,7 +2595,7 @@ sub process {
 					}
 				}
 				if ($seen && !$allowed) {
-					WARN("braces {} are not necessary for any arm of this statement\n" . $herectx);
+					CS_WARN("braces {} are not necessary for any arm of this statement\n" . $herectx);
 				}
 			}
 		}
@@ -2630,7 +2649,7 @@ sub process {
 					$herectx .= raw_line($linenr, $n) . "\n";;
 				}
 
-				WARN("braces {} are not necessary for single statement blocks\n" . $herectx);
+				CS_WARN("braces {} are not necessary for single statement blocks\n" . $herectx);
 			}
 		}
 
@@ -2699,7 +2718,7 @@ sub process {
 
 # warn about spacing in #ifdefs
 		if ($line =~ /^.\s*\#\s*(ifdef|ifndef|elif)\s\s+/) {
-			ERROR("exactly one space required after that #$1\n" . $herecurr);
+			CS_ERROR("exactly one space required after that #$1\n" . $herecurr);
 		}
 
 # check for spinlock_t definitions without a comment.
@@ -2723,24 +2742,24 @@ sub process {
 
 # Check that the storage class is at the beginning of a declaration
 		if ($line =~ /\b$Storage\b/ && $line !~ /^.\s*$Storage\b/) {
-			WARN("storage class should be at the beginning of the declaration\n" . $herecurr)
+			CS_WARN("storage class should be at the beginning of the declaration\n" . $herecurr)
 		}
 
 # check the location of the inline attribute, that it is between
 # storage class and type.
 		if ($line =~ /\b$Type\s+$Inline\b/ ||
 		    $line =~ /\b$Inline\s+$Storage\b/) {
-			ERROR("inline keyword should sit between storage class and type\n" . $herecurr);
+			CS_ERROR("inline keyword should sit between storage class and type\n" . $herecurr);
 		}
 
 # Check for __inline__ and __inline, prefer inline
 		if ($line =~ /\b(__inline__|__inline)\b/) {
-			WARN("plain inline is preferred over $1\n" . $herecurr);
+			CS_WARN("plain inline is preferred over $1\n" . $herecurr);
 		}
 
 # Check for __attribute__ packed, prefer __packed
 		if ($line =~ /\b__attribute__\s*\(\s*\(.*\bpacked\b/) {
-			WARN("__packed is preferred over __attribute__((packed))\n" . $herecurr);
+			CS_WARN("__packed is preferred over __attribute__((packed))\n" . $herecurr);
 		}
 
 # check for sizeof(&)
@@ -2791,7 +2810,7 @@ sub process {
 
 # check for multiple semicolons
 		if ($line =~ /;\s*;\s*$/) {
-		    WARN("Statements terminations use 1 semicolon\n" . $herecurr);
+		    CS_WARN("Statements terminations use 1 semicolon\n" . $herecurr);
 		}
 
 # check for gcc specific __FUNCTION__
@@ -2945,6 +2964,9 @@ sub process {
 			print "NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or\n";
 			print "      scripts/cleanfile\n\n";
 		}
+		if ($nocs == 1) {
+			print "You should rerun without the --nocs switch before patch submission\n";
+		}
 	}
 
 	if ($clean == 1 && $quiet == 0) {

Re: checkpatch: introduce --nocs to disable CodingStyle warnings.

[Joe Perches]

On Tue, 2011-03-22 at 15:58 -0400, Dave Jones wrote:
> How about this ?  running checkpatch with --nocs will now only print
> non-CodingStyle related warnings.   I may have missed some of the
> conversions, but this seems to silence the more egregious 'noise'.
> Signed-off-by: Dave Jones <davej@redhat.com>

Hi Dave.

Trivia only:

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 58848e3..a2b0086 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -19,6 +19,7 @@ my $tree = 1;
>  my $chk_signoff = 1;
>  my $chk_patch = 1;
>  my $tst_only;
> +my $nocs = 0;

To match the other checkpatch control flag uses,
I think this would be better as:

my $chk_style = 1;

> @@ -55,6 +56,7 @@ Options:

+  --style               display warnings about CodingStyle.

etc...

> @@ -80,6 +82,7 @@ GetOptions(
>  
>  	'debug=s'	=> \%debug,
>  	'test-only=s'	=> \$tst_only,
> +	'nocs'		=> \$nocs,

'style!' => \$chk_style,

etc...

And the command line use would be --nostyle

cheers, Joe

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Aneesh Kumar K. V]

On Sun, 20 Mar 2011 10:17:07 +0100 (CET), Julia Lawall <julia@diku.dk> wrote:
> On Sun, 20 Mar 2011, Pekka Enberg wrote:
> 
> > Hi Julia,
> > 
> > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <julia@diku.dk> wrote:
> > > Suggestions for how to make it easier to use or the documentation more
> > > understandable are welcome.
> > 
> > The benefit of scripts/checkpatch.pl is that it doesn't require any
> > setting up to do. I'm personally less likely to use Coccinelle (and
> > Sparse for that matter) on boxes where the software is not installed.
> > I'm not sure how other people feel about it, but I'd personally love
> > to see tools/coccinelle and tools/sparse.
> 
> This was discussed before, and it was felt that perhaps 75000 lines of 
> ocaml code was not really appropriate for the Linux source tree, and also 
> that it would too much complicate our development process.
> 
> One reason for using multiple machines would be to work on multiple 
> architectures.  But Coccinelle is not sensitive to the architecture on 
> which it is run, so perhaps you do't need to have it installed everywhere.
> 
> > As for something more concrete, I guess this is what I'm mostly interested in:
> > 
> > penberg@jaguar:~/src/linux$ make C=1 CHECK="scripts/coccicheck" mm/slub.o
> > 
> > I guess it'd be good to document that for 'make help' because now you
> > need to dig through Documentation/coccinelle.txt to find it.
> 
> OK, thanks.  We will look into that.
> 
> > P.S. It seems there's a bug somewhere because the above command fails
> > miserably for me:
> > 
> >   CHK     include/linux/version.h
> >   CHK     include/generated/utsrelease.h
> >   CALL    scripts/checksyscalls.sh
> >   CHECK   mm/slub.c
> > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > line 32, column 5,  charpos = 747
> >     around = '<+...', whole content = -    <+... when != goto l2;
> > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > context: <+...")
> > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > line 32, column 5,  charpos = 747
> >     around = '<+...', whole content = -    <+... when != goto l2;
> > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > context: <+...")
> > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > line 32, column 5,  charpos = 747
> >     around = '<+...', whole content = -    <+... when != goto l2;
> > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > context: <+...")
> > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > line 32, column 5,  charpos = 747
> >     around = '<+...', whole content = -    <+... when != goto l2;
> > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > context: <+...")
> > make[1]: *** [mm/slub.o] Error 1
> > make: *** [mm/slub.o] Error 2
> > 
> > penberg@jaguar:~/src/linux$ dpkg -l|grep coccinelle
> > ii  coccinelle                                      0.2.2.deb-2
> >                                         semantic patching tool for C
> > 
> > [ I'm on Ubuntu 10.10. ]
> 
> Indeed that one seems to be quite out of date.  You can get the most 
> recent version here: https://launchpad.net/~npalix/+archive/coccinelle

I never got a working version of coccinelle with scripts in the kernel.
Even the ppa version doesn't work for me. 

make  C=2  CHECK=scripts/coccicheck fs/namei.o
...
....
  CHECK   scripts/mod/empty.c
File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
    around = '<+...', whole content = -    <+... when != goto l2;
Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
    around = '<+...', whole content = -    <+... when != goto l2;
Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
    around = '<+...', whole content = -    <+... when != goto l2;
Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
    around = '<+...', whole content = -    <+... when != goto l2;
Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Julia Lawall]

On Thu, 24 Mar 2011, Aneesh Kumar K. V wrote:

> On Sun, 20 Mar 2011 10:17:07 +0100 (CET), Julia Lawall <julia@diku.dk> wrote:
> > On Sun, 20 Mar 2011, Pekka Enberg wrote:
> > 
> > > Hi Julia,
> > > 
> > > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <julia@diku.dk> wrote:
> > > > Suggestions for how to make it easier to use or the documentation more
> > > > understandable are welcome.
> > > 
> > > The benefit of scripts/checkpatch.pl is that it doesn't require any
> > > setting up to do. I'm personally less likely to use Coccinelle (and
> > > Sparse for that matter) on boxes where the software is not installed.
> > > I'm not sure how other people feel about it, but I'd personally love
> > > to see tools/coccinelle and tools/sparse.
> > 
> > This was discussed before, and it was felt that perhaps 75000 lines of 
> > ocaml code was not really appropriate for the Linux source tree, and also 
> > that it would too much complicate our development process.
> > 
> > One reason for using multiple machines would be to work on multiple 
> > architectures.  But Coccinelle is not sensitive to the architecture on 
> > which it is run, so perhaps you do't need to have it installed everywhere.
> > 
> > > As for something more concrete, I guess this is what I'm mostly interested in:
> > > 
> > > penberg@jaguar:~/src/linux$ make C=1 CHECK="scripts/coccicheck" mm/slub.o
> > > 
> > > I guess it'd be good to document that for 'make help' because now you
> > > need to dig through Documentation/coccinelle.txt to find it.
> > 
> > OK, thanks.  We will look into that.
> > 
> > > P.S. It seems there's a bug somewhere because the above command fails
> > > miserably for me:
> > > 
> > >   CHK     include/linux/version.h
> > >   CHK     include/generated/utsrelease.h
> > >   CALL    scripts/checksyscalls.sh
> > >   CHECK   mm/slub.c
> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > > line 32, column 5,  charpos = 747
> > >     around = '<+...', whole content = -    <+... when != goto l2;
> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > > context: <+...")
> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > > line 32, column 5,  charpos = 747
> > >     around = '<+...', whole content = -    <+... when != goto l2;
> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > > context: <+...")
> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > > line 32, column 5,  charpos = 747
> > >     around = '<+...', whole content = -    <+... when != goto l2;
> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > > context: <+...")
> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > > line 32, column 5,  charpos = 747
> > >     around = '<+...', whole content = -    <+... when != goto l2;
> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > > context: <+...")
> > > make[1]: *** [mm/slub.o] Error 1
> > > make: *** [mm/slub.o] Error 2
> > > 
> > > penberg@jaguar:~/src/linux$ dpkg -l|grep coccinelle
> > > ii  coccinelle                                      0.2.2.deb-2
> > >                                         semantic patching tool for C
> > > 
> > > [ I'm on Ubuntu 10.10. ]
> > 
> > Indeed that one seems to be quite out of date.  You can get the most 
> > recent version here: https://launchpad.net/~npalix/+archive/coccinelle
> 
> I never got a working version of coccinelle with scripts in the kernel.
> Even the ppa version doesn't work for me. 
> 
> make  C=2  CHECK=scripts/coccicheck fs/namei.o
> ...
> ....
>   CHECK   scripts/mod/empty.c
> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>     around = '<+...', whole content = -    <+... when != goto l2;
> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>     around = '<+...', whole content = -    <+... when != goto l2;
> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>     around = '<+...', whole content = -    <+... when != goto l2;
> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>     around = '<+...', whole content = -    <+... when != goto l2;
> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")

That's quite strange.  Could you check that you are using the ppa version 
and not the old version?  spatch -version should give you 0.2.5-rc8

julia

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Nicolas Palix]

On Thu, Mar 24, 2011 at 5:08 PM, Julia Lawall <julia@diku.dk> wrote:
> On Thu, 24 Mar 2011, Aneesh Kumar K. V wrote:
>
>> On Sun, 20 Mar 2011 10:17:07 +0100 (CET), Julia Lawall <julia@diku.dk> wrote:
>> > On Sun, 20 Mar 2011, Pekka Enberg wrote:
>> >
>> > > Hi Julia,
>> > >
>> > > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <julia@diku.dk> wrote:
>> > > > Suggestions for how to make it easier to use or the documentation more
>> > > > understandable are welcome.
>> > >
>> > > The benefit of scripts/checkpatch.pl is that it doesn't require any
>> > > setting up to do. I'm personally less likely to use Coccinelle (and
>> > > Sparse for that matter) on boxes where the software is not installed.
>> > > I'm not sure how other people feel about it, but I'd personally love
>> > > to see tools/coccinelle and tools/sparse.
>> >
>> > This was discussed before, and it was felt that perhaps 75000 lines of
>> > ocaml code was not really appropriate for the Linux source tree, and also
>> > that it would too much complicate our development process.
>> >
>> > One reason for using multiple machines would be to work on multiple
>> > architectures.  But Coccinelle is not sensitive to the architecture on
>> > which it is run, so perhaps you do't need to have it installed everywhere.
>> >
>> > > As for something more concrete, I guess this is what I'm mostly interested in:
>> > >
>> > > penberg@jaguar:~/src/linux$ make C=1 CHECK="scripts/coccicheck" mm/slub.o
>> > >
>> > > I guess it'd be good to document that for 'make help' because now you
>> > > need to dig through Documentation/coccinelle.txt to find it.
>> >
>> > OK, thanks.  We will look into that.
>> >
>> > > P.S. It seems there's a bug somewhere because the above command fails
>> > > miserably for me:
>> > >
>> > >   CHK     include/linux/version.h
>> > >   CHK     include/generated/utsrelease.h
>> > >   CALL    scripts/checksyscalls.sh
>> > >   CHECK   mm/slub.c
>> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
>> > > line 32, column 5,  charpos = 747
>> > >     around = '<+...', whole content = -    <+... when != goto l2;
>> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
>> > > context: <+...")
>> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
>> > > line 32, column 5,  charpos = 747
>> > >     around = '<+...', whole content = -    <+... when != goto l2;
>> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
>> > > context: <+...")
>> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
>> > > line 32, column 5,  charpos = 747
>> > >     around = '<+...', whole content = -    <+... when != goto l2;
>> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
>> > > context: <+...")
>> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
>> > > line 32, column 5,  charpos = 747
>> > >     around = '<+...', whole content = -    <+... when != goto l2;
>> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
>> > > context: <+...")
>> > > make[1]: *** [mm/slub.o] Error 1
>> > > make: *** [mm/slub.o] Error 2
>> > >
>> > > penberg@jaguar:~/src/linux$ dpkg -l|grep coccinelle
>> > > ii  coccinelle                                      0.2.2.deb-2
>> > >                                         semantic patching tool for C
>> > >
>> > > [ I'm on Ubuntu 10.10. ]
>> >
>> > Indeed that one seems to be quite out of date.  You can get the most
>> > recent version here: https://launchpad.net/~npalix/+archive/coccinelle
>>
>> I never got a working version of coccinelle with scripts in the kernel.
>> Even the ppa version doesn't work for me.
>>
>> make  C=2  CHECK=scripts/coccicheck fs/namei.o
>> ...
>> ....
>>   CHECK   scripts/mod/empty.c
>> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>>     around = '<+...', whole content = -    <+... when != goto l2;
>> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
>> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>>     around = '<+...', whole content = -    <+... when != goto l2;
>> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
>> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>>     around = '<+...', whole content = -    <+... when != goto l2;
>> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
>> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>>     around = '<+...', whole content = -    <+... when != goto l2;
>> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
>
> That's quite strange.  Could you check that you are using the ppa version
> and not the old version?  spatch -version should give you 0.2.5-rc8

Indeed... I just checked and I got not error.

npalix@penpen:~/Build/linux$ make  C=2  CHECK=scripts/coccicheck
COCCI="scripts/coccinelle/api/memdup_user.cocci" fs/namei.o
  CHK     include/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh
  CHECK   scripts/mod/empty.c
  CHECK   fs/namei.c
npalix@penpen:~/Build/linux$ make  C=2  CHECK=scripts/coccicheck
fs/namei.o  CHK     include/linux/version.h
  CHK     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh
  CHECK   scripts/mod/empty.c
  CHECK   fs/namei.c
npalix@penpen:~/Build/linux$ spatch -version
spatch version 0.2.5-rc8 with Python support

>
> julia
>



-- 
Nicolas Palix
http://sardes.inrialpes.fr/~npalix/

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Aneesh Kumar K. V]

On Thu, 24 Mar 2011 17:08:01 +0100 (CET), Julia Lawall <julia@diku.dk> wrote:
> On Thu, 24 Mar 2011, Aneesh Kumar K. V wrote:
> 
> > On Sun, 20 Mar 2011 10:17:07 +0100 (CET), Julia Lawall <julia@diku.dk> wrote:
> > > On Sun, 20 Mar 2011, Pekka Enberg wrote:
> > > 
> > > > Hi Julia,
> > > > 
> > > > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <julia@diku.dk> wrote:
> > > > > Suggestions for how to make it easier to use or the documentation more
> > > > > understandable are welcome.
> > > > 
> > > > The benefit of scripts/checkpatch.pl is that it doesn't require any
> > > > setting up to do. I'm personally less likely to use Coccinelle (and
> > > > Sparse for that matter) on boxes where the software is not installed.
> > > > I'm not sure how other people feel about it, but I'd personally love
> > > > to see tools/coccinelle and tools/sparse.
> > > 
> > > This was discussed before, and it was felt that perhaps 75000 lines of 
> > > ocaml code was not really appropriate for the Linux source tree, and also 
> > > that it would too much complicate our development process.
> > > 
> > > One reason for using multiple machines would be to work on multiple 
> > > architectures.  But Coccinelle is not sensitive to the architecture on 
> > > which it is run, so perhaps you do't need to have it installed everywhere.
> > > 
> > > > As for something more concrete, I guess this is what I'm mostly interested in:
> > > > 
> > > > penberg@jaguar:~/src/linux$ make C=1 CHECK="scripts/coccicheck" mm/slub.o
> > > > 
> > > > I guess it'd be good to document that for 'make help' because now you
> > > > need to dig through Documentation/coccinelle.txt to find it.
> > > 
> > > OK, thanks.  We will look into that.
> > > 
> > > > P.S. It seems there's a bug somewhere because the above command fails
> > > > miserably for me:
> > > > 
> > > >   CHK     include/linux/version.h
> > > >   CHK     include/generated/utsrelease.h
> > > >   CALL    scripts/checksyscalls.sh
> > > >   CHECK   mm/slub.c
> > > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > > > line 32, column 5,  charpos = 747
> > > >     around = '<+...', whole content = -    <+... when != goto l2;
> > > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > > > context: <+...")
> > > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > > > line 32, column 5,  charpos = 747
> > > >     around = '<+...', whole content = -    <+... when != goto l2;
> > > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > > > context: <+...")
> > > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > > > line 32, column 5,  charpos = 747
> > > >     around = '<+...', whole content = -    <+... when != goto l2;
> > > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > > > context: <+...")
> > > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
> > > > line 32, column 5,  charpos = 747
> > > >     around = '<+...', whole content = -    <+... when != goto l2;
> > > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
> > > > context: <+...")
> > > > make[1]: *** [mm/slub.o] Error 1
> > > > make: *** [mm/slub.o] Error 2
> > > > 
> > > > penberg@jaguar:~/src/linux$ dpkg -l|grep coccinelle
> > > > ii  coccinelle                                      0.2.2.deb-2
> > > >                                         semantic patching tool for C
> > > > 
> > > > [ I'm on Ubuntu 10.10. ]
> > > 
> > > Indeed that one seems to be quite out of date.  You can get the most 
> > > recent version here: https://launchpad.net/~npalix/+archive/coccinelle
> > 
> > I never got a working version of coccinelle with scripts in the kernel.
> > Even the ppa version doesn't work for me. 
> > 
> > make  C=2  CHECK=scripts/coccicheck fs/namei.o
> > ...
> > ....
> >   CHECK   scripts/mod/empty.c
> > File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
> >     around = '<+...', whole content = -    <+... when != goto l2;
> > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
> > File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
> >     around = '<+...', whole content = -    <+... when != goto l2;
> > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
> > File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
> >     around = '<+...', whole content = -    <+... when != goto l2;
> > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
> > File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
> >     around = '<+...', whole content = -    <+... when != goto l2;
> > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
> 
> That's quite strange.  Could you check that you are using the ppa version 
> and not the old version?  spatch -version should give you 0.2.5-rc8

missed an apt-get update between add-apt-repository and apt-get install 

-aneesh

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Arnaud Lacombe]

Hi,

2011/3/24 Nicolas Palix <Nicolas.Palix@inria.fr>:
> On Thu, Mar 24, 2011 at 5:08 PM, Julia Lawall <julia@diku.dk> wrote:
>> On Thu, 24 Mar 2011, Aneesh Kumar K. V wrote:
>>
>>> On Sun, 20 Mar 2011 10:17:07 +0100 (CET), Julia Lawall <julia@diku.dk> wrote:
>>> > On Sun, 20 Mar 2011, Pekka Enberg wrote:
>>> >
>>> > > Hi Julia,
>>> > >
>>> > > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <julia@diku.dk> wrote:
>>> > > > Suggestions for how to make it easier to use or the documentation more
>>> > > > understandable are welcome.
>>> > >
>>> > > The benefit of scripts/checkpatch.pl is that it doesn't require any
>>> > > setting up to do. I'm personally less likely to use Coccinelle (and
>>> > > Sparse for that matter) on boxes where the software is not installed.
>>> > > I'm not sure how other people feel about it, but I'd personally love
>>> > > to see tools/coccinelle and tools/sparse.
>>> >
>>> > This was discussed before, and it was felt that perhaps 75000 lines of
>>> > ocaml code was not really appropriate for the Linux source tree, and also
>>> > that it would too much complicate our development process.
>>> >
>>> > One reason for using multiple machines would be to work on multiple
>>> > architectures.  But Coccinelle is not sensitive to the architecture on
>>> > which it is run, so perhaps you do't need to have it installed everywhere.
>>> >
>>> > > As for something more concrete, I guess this is what I'm mostly interested in:
>>> > >
>>> > > penberg@jaguar:~/src/linux$ make C=1 CHECK="scripts/coccicheck" mm/slub.o
>>> > >
>>> > > I guess it'd be good to document that for 'make help' because now you
>>> > > need to dig through Documentation/coccinelle.txt to find it.
>>> >
>>> > OK, thanks.  We will look into that.
>>> >
>>> > > P.S. It seems there's a bug somewhere because the above command fails
>>> > > miserably for me:
>>> > >
>>> > >   CHK     include/linux/version.h
>>> > >   CHK     include/generated/utsrelease.h
>>> > >   CALL    scripts/checksyscalls.sh
>>> > >   CHECK   mm/slub.c
>>> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
>>> > > line 32, column 5,  charpos = 747
>>> > >     around = '<+...', whole content = -    <+... when != goto l2;
>>> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
>>> > > context: <+...")
>>> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
>>> > > line 32, column 5,  charpos = 747
>>> > >     around = '<+...', whole content = -    <+... when != goto l2;
>>> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
>>> > > context: <+...")
>>> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
>>> > > line 32, column 5,  charpos = 747
>>> > >     around = '<+...', whole content = -    <+... when != goto l2;
>>> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
>>> > > context: <+...")
>>> > > File "/home/penberg/src/linux/scripts/coccinelle/api/memdup_user.cocci",
>>> > > line 32, column 5,  charpos = 747
>>> > >     around = '<+...', whole content = -    <+... when != goto l2;
>>> > > Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty
>>> > > context: <+...")
>>> > > make[1]: *** [mm/slub.o] Error 1
>>> > > make: *** [mm/slub.o] Error 2
>>> > >
>>> > > penberg@jaguar:~/src/linux$ dpkg -l|grep coccinelle
>>> > > ii  coccinelle                                      0.2.2.deb-2
>>> > >                                         semantic patching tool for C
>>> > >
>>> > > [ I'm on Ubuntu 10.10. ]
>>> >
>>> > Indeed that one seems to be quite out of date.  You can get the most
>>> > recent version here: https://launchpad.net/~npalix/+archive/coccinelle
>>>
>>> I never got a working version of coccinelle with scripts in the kernel.
>>> Even the ppa version doesn't work for me.
>>>
>>> make  C=2  CHECK=scripts/coccicheck fs/namei.o
>>> ...
>>> ....
>>>   CHECK   scripts/mod/empty.c
>>> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>>>     around = '<+...', whole content = -    <+... when != goto l2;
>>> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
>>> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>>>     around = '<+...', whole content = -    <+... when != goto l2;
>>> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
>>> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>>>     around = '<+...', whole content = -    <+... when != goto l2;
>>> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
>>> File "/home/opensource/sources/kernels/linux-2.6/scripts/coccinelle/api/memdup_user.cocci", line 32, column 5,  charpos = 747
>>>     around = '<+...', whole content = -    <+... when != goto l2;
>>> Fatal error: exception Lexer_cocci.Lexical("invalid in a nonempty context: <+...")
>>
>> That's quite strange.  Could you check that you are using the ppa version
>> and not the old version?  spatch -version should give you 0.2.5-rc8
>
> Indeed... I just checked and I got not error.
>
> npalix@penpen:~/Build/linux$ make  C=2  CHECK=scripts/coccicheck
> COCCI="scripts/coccinelle/api/memdup_user.cocci" fs/namei.o
>  CHK     include/linux/version.h
>  CHK     include/generated/utsrelease.h
>  CALL    scripts/checksyscalls.sh
>  CHECK   scripts/mod/empty.c
>  CHECK   fs/namei.c
> npalix@penpen:~/Build/linux$ make  C=2  CHECK=scripts/coccicheck
> fs/namei.o  CHK     include/linux/version.h
>  CHK     include/generated/utsrelease.h
>  CALL    scripts/checksyscalls.sh
>  CHECK   scripts/mod/empty.c
>  CHECK   fs/namei.c
> npalix@penpen:~/Build/linux$ spatch -version
> spatch version 0.2.5-rc8 with Python support
>
So does it mean that you do not even have a stable grammar ? Do you at
least offer any guarantee that a script made for version X will still
work in version X+1 ?

Thanks,
 - Arnaud

Re: [PATCH] checkpatch: Test for kmalloc/memset(0) pairs

[Julia Lawall]

> So does it mean that you do not even have a stable grammar ? Do you at
> least offer any guarantee that a script made for version X will still
> work in version X+1 ?

The inability to put - on <+... ...+> was a bug.  So I don't know if 
fixing a bug would be considered a sign of an unstable grammar.  There was 
also one deliberate change in the grammar some rc's ago related to the use 
of commas in sequences (parameter lists etc).  But otherwise, the changes 
in the grammar have been additions, as people eg ask for new kinds of 
metavariables.

But Coccinelle is a research prototype under development, not a product.  
So I'm not sure it is appropriate to say that we guarantee anything.

julia