2011-03-18 02:52:30

by Steven Rostedt

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

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 <[email protected]>

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


2011-03-18 03:14:08

by Jonathan Corbet

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

On Thu, 17 Mar 2011 22:52:24 -0400
Steven Rostedt <[email protected]> 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

2011-03-18 03:32:57

by Steven Rostedt

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

On Thu, 2011-03-17 at 21:15 -0600, Jonathan Corbet wrote:
> On Thu, 17 Mar 2011 22:52:24 -0400
> Steven Rostedt <[email protected]> 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

2011-03-19 19:40:31

by Dave Jones

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

On Thu, Mar 17, 2011 at 09:15:48PM -0600, Jonathan Corbet wrote:
> On Thu, 17 Mar 2011 22:52:24 -0400
> Steven Rostedt <[email protected]> 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

2011-03-20 03:40:44

by Cong Wang

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

On Fri, Mar 18, 2011 at 11:32 AM, Steven Rostedt <[email protected]> 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.

2011-03-20 07:03:01

by Pekka Enberg

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

Hi,

On Fri, Mar 18, 2011 at 11:32 AM, Steven Rostedt <[email protected]> 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 <[email protected]> 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.

2011-03-20 08:02:53

by Julia Lawall

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

On Sun, 20 Mar 2011, Pekka Enberg wrote:

> Hi,
>
> On Fri, Mar 18, 2011 at 11:32 AM, Steven Rostedt <[email protected]> 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 <[email protected]> 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

2011-03-20 09:02:16

by Pekka Enberg

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

Hi Julia,

On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <[email protected]> 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

2011-03-20 09:17:16

by Julia Lawall

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

On Sun, 20 Mar 2011, Pekka Enberg wrote:

> Hi Julia,
>
> On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <[email protected]> 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

2011-03-20 10:54:47

by Ingo Molnar

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


* Julia Lawall <[email protected]> wrote:

> On Sun, 20 Mar 2011, Pekka Enberg wrote:
>
> > Hi Julia,
> >
> > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <[email protected]> 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

2011-03-20 11:06:31

by Pekka Enberg

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

Hi,

On Sun, Mar 20, 2011 at 12:54 PM, Ingo Molnar <[email protected]> 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?

2011-03-20 11:09:41

by Alexey Dobriyan

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

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

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

2011-03-20 11:32:37

by Nicolas Palix

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

Hi,

On Sun, Mar 20, 2011 at 12:06 PM, Pekka Enberg <[email protected]> wrote:
> Hi,
>
> On Sun, Mar 20, 2011 at 12:54 PM, Ingo Molnar <[email protected]> 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/

2011-03-20 12:00:35

by Julia Lawall

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

On Sun, 20 Mar 2011, Nicolas Palix wrote:

> Hi,
>
> On Sun, Mar 20, 2011 at 12:06 PM, Pekka Enberg <[email protected]> wrote:
> > Hi,
> >
> > On Sun, Mar 20, 2011 at 12:54 PM, Ingo Molnar <[email protected]> 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

2011-03-20 12:38:46

by Julia Lawall

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

> > [...] 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

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

El Sun, 20 Mar 2011 11:54:12 +0100
Ingo Molnar <[email protected]> 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
> --

2011-03-21 13:13:18

by Steven Rostedt

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

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 <[email protected]> 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

2011-03-21 13:26:23

by Steven Rostedt

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

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

2011-03-21 13:29:21

by Steven Rostedt

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

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

2011-03-21 13:55:25

by Borislav Petkov

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

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

2011-03-22 19:59:25

by Dave Jones

[permalink] [raw]
Subject: checkpatch: introduce --nocs to disable CodingStyle warnings.

> 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 <[email protected]>

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) {

2011-03-22 20:21:06

by Joe Perches

[permalink] [raw]
Subject: Re: checkpatch: introduce --nocs to disable CodingStyle warnings.

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 <[email protected]>

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

2011-03-24 15:35:13

by Aneesh Kumar K.V

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

On Sun, 20 Mar 2011 10:17:07 +0100 (CET), Julia Lawall <[email protected]> wrote:
> On Sun, 20 Mar 2011, Pekka Enberg wrote:
>
> > Hi Julia,
> >
> > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <[email protected]> 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: <+...")

2011-03-24 16:09:16

by Julia Lawall

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

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

> On Sun, 20 Mar 2011 10:17:07 +0100 (CET), Julia Lawall <[email protected]> wrote:
> > On Sun, 20 Mar 2011, Pekka Enberg wrote:
> >
> > > Hi Julia,
> > >
> > > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <[email protected]> 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

2011-03-24 16:10:35

by Nicolas Palix

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

On Thu, Mar 24, 2011 at 5:08 PM, Julia Lawall <[email protected]> wrote:
> On Thu, 24 Mar 2011, Aneesh Kumar K. V wrote:
>
>> On Sun, 20 Mar 2011 10:17:07 +0100 (CET), Julia Lawall <[email protected]> wrote:
>> > On Sun, 20 Mar 2011, Pekka Enberg wrote:
>> >
>> > > Hi Julia,
>> > >
>> > > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <[email protected]> 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/

2011-03-24 17:28:47

by Aneesh Kumar K.V

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

On Thu, 24 Mar 2011 17:08:01 +0100 (CET), Julia Lawall <[email protected]> wrote:
> On Thu, 24 Mar 2011, Aneesh Kumar K. V wrote:
>
> > On Sun, 20 Mar 2011 10:17:07 +0100 (CET), Julia Lawall <[email protected]> wrote:
> > > On Sun, 20 Mar 2011, Pekka Enberg wrote:
> > >
> > > > Hi Julia,
> > > >
> > > > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <[email protected]> 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

2011-03-24 18:31:01

by Arnaud Lacombe

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

Hi,

2011/3/24 Nicolas Palix <[email protected]>:
> On Thu, Mar 24, 2011 at 5:08 PM, Julia Lawall <[email protected]> wrote:
>> On Thu, 24 Mar 2011, Aneesh Kumar K. V wrote:
>>
>>> On Sun, 20 Mar 2011 10:17:07 +0100 (CET), Julia Lawall <[email protected]> wrote:
>>> > On Sun, 20 Mar 2011, Pekka Enberg wrote:
>>> >
>>> > > Hi Julia,
>>> > >
>>> > > On Sun, Mar 20, 2011 at 10:01 AM, Julia Lawall <[email protected]> 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

2011-03-24 20:41:01

by Julia Lawall

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

> 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