[PATCH 0/4] Add a Coccinelle front-end script

[PATCH 0/4] Add a Coccinelle front-end script

[Nicolas Palix]

New targets are added (coccicheck-<mode>) to call the 'coccinelle.sh' front-end
in the 'scripts' directory with the <mode> argument.

Four modes are defined: report, patch, context, and org.

'report' mode generates a list in the following format:
  file:line:column-column: message

'patch' mode proposes a generic fix, when possible.

'context' mode highlights lines of interest and their context
in a diff-like style.

'org' mode generates a report in the Org mode format of Emacs.

Three semantic patches, with a low rate of false positives, are also
included.  Other semantic patches will be provided later. Note that a
semantic patch does not need to define all four modes. As many semantic
patches may be proposed later, they are organized under sub-directories
of 'scripts/coccinelle/'.

We add a reference to the tool in the proposed changelog because it will
make the changelog easier to understand for someone who is not aware of
the tool.

To apply a single semantic patch, the user can define the COCCI environment
variable which gives the path to the semantic patch.

Nicolas Palix (4):
  Add targets to use the Coccinelle checker
  Add scripts/coccinelle/drop_kmalloc_cast.cocci
  Add scripts/coccinelle/kzalloc-simple.cocci
  Add scripts/coccinelle/resource_size.cocci

 MAINTAINERS                                |   10 +++
 Makefile                                   |   20 +++++-
 scripts/coccinelle.sh                      |   28 ++++++++
 scripts/coccinelle/drop_kmalloc_cast.cocci |   68 ++++++++++++++++++++
 scripts/coccinelle/kzalloc-simple.cocci    |   83 ++++++++++++++++++++++++
 scripts/coccinelle/resource_size.cocci     |   94 ++++++++++++++++++++++++++++
 6 files changed, 301 insertions(+), 2 deletions(-)
 create mode 100755 scripts/coccinelle.sh
 create mode 100644 scripts/coccinelle/drop_kmalloc_cast.cocci
 create mode 100644 scripts/coccinelle/kzalloc-simple.cocci
 create mode 100644 scripts/coccinelle/resource_size.cocci

[PATCH 0/4] Add a Coccinelle front-end script

[Nicolas Palix]

New targets are added (coccicheck-<mode>) to call the 'coccinelle.sh' front-end
in the 'scripts' directory with the <mode> argument.

Four modes are defined: report, patch, context, and org.

'report' mode generates a list in the following format:
  file:line:column-column: message

'patch' mode proposes a generic fix, when possible.

'context' mode highlights lines of interest and their context
in a diff-like style.

'org' mode generates a report in the Org mode format of Emacs.

Three semantic patches, with a low rate of false positives, are also
included.  Other semantic patches will be provided later. Note that a
semantic patch does not need to define all four modes. As many semantic
patches may be proposed later, they are organized under sub-directories
of 'scripts/coccinelle/'.

We add a reference to the tool in the proposed changelog because it will
make the changelog easier to understand for someone who is not aware of
the tool.

To apply a single semantic patch, the user can define the COCCI environment
variable which gives the path to the semantic patch.

Nicolas Palix (4):
  Add targets to use the Coccinelle checker
  Add scripts/coccinelle/drop_kmalloc_cast.cocci
  Add scripts/coccinelle/kzalloc-simple.cocci
  Add scripts/coccinelle/resource_size.cocci

 MAINTAINERS                                |   10 +++
 Makefile                                   |   20 +++++-
 scripts/coccinelle.sh                      |   28 ++++++++
 scripts/coccinelle/drop_kmalloc_cast.cocci |   68 ++++++++++++++++++++
 scripts/coccinelle/kzalloc-simple.cocci    |   83 ++++++++++++++++++++++++
 scripts/coccinelle/resource_size.cocci     |   94 ++++++++++++++++++++++++++++
 6 files changed, 301 insertions(+), 2 deletions(-)
 create mode 100755 scripts/coccinelle.sh
 create mode 100644 scripts/coccinelle/drop_kmalloc_cast.cocci
 create mode 100644 scripts/coccinelle/kzalloc-simple.cocci
 create mode 100644 scripts/coccinelle/resource_size.cocci

[PATCH 1/4] Add targets to use the Coccinelle checker

[Nicolas Palix]

Four targets are added. Each one generates a different
output kind: context, patch, org, report.
Every SmPL file in 'scripts/coccinelle' is given to the spatch frontend
(located in the 'scripts' directory), and applied to the entire
source tree.

Signed-off-by: Nicolas Palix <npalix@diku.dk>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
 MAINTAINERS           |   10 ++++++++++
 Makefile              |   20 ++++++++++++++++++--
 scripts/coccinelle.sh |   28 ++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 2 deletions(-)
 create mode 100755 scripts/coccinelle.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index d5b0b1b..84f527a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1543,6 +1543,16 @@ L:	platform-driver-x86@vger.kernel.org
 S:	Supported
 F:	drivers/platform/x86/classmate-laptop.c
 
+COCCINELLE/Semantic Patches (SmPL)
+M:	Julia Lawall <julia@diku.dk>
+M:	Gilles Muller <Gilles.Muller@lip6.fr>
+M:	Nicolas Palix <npalix@diku.dk>
+L:	cocci@diku.dk (moderated for non-subscribers)
+W:	http://coccinelle.lip6.fr/
+S:	Supported
+F:	scripts/coccinelle/
+F:	scripts/coccinelle.sh
+
 CODA FILE SYSTEM
 M:	Jan Harkes <jaharkes@cs.cmu.edu>
 M:	coda@cs.cmu.edu
diff --git a/Makefile b/Makefile
index 701bc65..7bf168f 100644
--- a/Makefile
+++ b/Makefile
@@ -325,6 +325,7 @@ INSTALLKERNEL  := installkernel
 DEPMOD		= /sbin/depmod
 KALLSYMS	= scripts/kallsyms
 PERL		= perl
+COCCINELLE      = spatch
 CHECK		= sparse
 
 CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
@@ -411,7 +412,8 @@ endif
 no-dot-config-targets := clean mrproper distclean \
 			 cscope TAGS tags help %docs check% \
 			 include/linux/version.h headers_% \
-			 kernelrelease kernelversion
+			 kernelrelease kernelversion        \
+			 coccicheck-%
 
 config-targets := 0
 mixed-targets  := 0
@@ -1264,7 +1266,8 @@ help:
 	@echo  '  includecheck    - Check for duplicate included header files'
 	@echo  '  export_report   - List the usages of all exported symbols'
 	@echo  '  headers_check   - Sanity check on exported headers'
-	@echo  '  headerdep       - Detect inclusion cycles in headers'; \
+	@echo  '  headerdep       - Detect inclusion cycles in headers'
+	@echo  '  coccicheck-<m>  - Check with Coccinelle in <m> mode (report, context, patch, org)'; \
 	 echo  ''
 	@echo  'Kernel packaging:'
 	@$(MAKE) $(build)=$(package-dir) help
@@ -1424,6 +1427,19 @@ versioncheck:
 		-name '*.[hcS]' -type f -print | sort \
 		| xargs $(PERL) -w $(srctree)/scripts/checkversion.pl
 
+coccicheck-context coccicheck-patch coccicheck-org coccicheck-report:
+	@echo ''
+	@echo 'Please check for false positives in the output before submitting a patch.'
+	@echo 'When using "patch" mode, carefully review the patch before submitting it.'
+	@echo ''
+	@if [ "$(COCCI)" = "" ] ; then                         \
+	find $(srctree)/scripts/coccinelle/                    \
+		-name '*.cocci' -type f                        \
+		-exec $(srctree)/scripts/coccinelle.sh $(COCCINELLE) $(@:coccicheck-%=%) \{} $(srctree) \; ; \
+	else                                                                                                 \
+		$(srctree)/scripts/coccinelle.sh $(COCCINELLE) $(@:coccicheck-%=%) $(COCCI) $(srctree)     ; \
+	fi
+
 namespacecheck:
 	$(PERL) $(srctree)/scripts/namespace.pl
 
diff --git a/scripts/coccinelle.sh b/scripts/coccinelle.sh
new file mode 100755
index 0000000..0d29e93
--- /dev/null
+++ b/scripts/coccinelle.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+SPATCH="`which $1`"
+MODE="$2"
+COCCI="$3"
+DIR="$4"
+
+OPT=`grep "Option" $COCCI | cut -d':' -f2`
+FILE=`echo $COCCI | sed "s|$DIR/||"`
+
+if [ -x "$SPATCH" ]; then
+
+    echo "Processing `basename $COCCI` with option(s) \"$(OPT)\""
+    echo 'Message example to submit a patch:'
+    sed -e '/\/\/\//!d' -e 's|^///||' $COCCI
+
+    echo ' The semantic patch that makes this change is available'
+    echo " in $FILE."
+    echo ''
+    echo ' More information about semantic patching is available at'
+    echo ' http://coccinelle.lip6.fr/'
+    echo ''
+
+    $SPATCH -D $MODE -very_quiet -sp_file $COCCI $OPT -dir $DIR
+
+else
+    echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/'
+fi
-- 
1.7.0.4

Re: [PATCH 1/4] Add targets to use the Coccinelle checker

[Américo Wang]

On Mon, May 10, 2010 at 06:24:24PM +0200, Nicolas Palix wrote:
>Four targets are added. Each one generates a different
>output kind: context, patch, org, report.
>Every SmPL file in 'scripts/coccinelle' is given to the spatch frontend
>(located in the 'scripts' directory), and applied to the entire
>source tree.
>
>Signed-off-by: Nicolas Palix <npalix@diku.dk>
>Signed-off-by: Julia Lawall <julia@diku.dk>

Hi,

This is great! I would like to see coccinelle to be integrated with kbuild,
but the name 'coccicheck' is really confusing, how about 'sema_check'? Which
means 'semantic check'.

Or we can use something like for sparse, i.e. 'make C=1' etc..

Thanks!

Re: [PATCH 1/4] Add targets to use the Coccinelle checker

[Joerg Roedel]

On Wed, May 12, 2010 at 02:42:30PM +0800, Américo Wang wrote:
> On Mon, May 10, 2010 at 06:24:24PM +0200, Nicolas Palix wrote:
> >Four targets are added. Each one generates a different
> >output kind: context, patch, org, report.
> >Every SmPL file in 'scripts/coccinelle' is given to the spatch frontend
> >(located in the 'scripts' directory), and applied to the entire
> >source tree.
> >
> >Signed-off-by: Nicolas Palix <npalix@diku.dk>
> >Signed-off-by: Julia Lawall <julia@diku.dk>
> 
> This is great!

Totally agreed.

> I would like to see coccinelle to be integrated with kbuild, but the
> name 'coccicheck' is really confusing, how about 'sema_check'? Which
> means 'semantic check'.

I think its okay to keep coccicheck because its the name of the tool
called. We do the same with 'make cscope'.
But since I am a bit lazy I would prefer a shorter name, probably just
'make cocci'?

> Or we can use something like for sparse, i.e. 'make C=1' etc..

Yes, this would also make sense. As a developer I could add this and
only the code I changed is checked.

	Joerg

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] Add targets to use the Coccinelle checker

[Michal Marek]

On 10.5.2010 18:24, Nicolas Palix wrote:
> Four targets are added. Each one generates a different
> output kind: context, patch, org, report.
> Every SmPL file in 'scripts/coccinelle' is given to the spatch frontend
> (located in the 'scripts' directory), and applied to the entire
> source tree.

I think it would be good to repeat the description of the four modes
here (the 0/4 mail won't appear in the changelog).


> +	@if [ "$(COCCI)" = "" ] ; then                         \
> +	find $(srctree)/scripts/coccinelle/                    \
> +		-name '*.cocci' -type f                        \
> +		-exec $(srctree)/scripts/coccinelle.sh $(COCCINELLE) $(@:coccicheck-%=%) \{} $(srctree) \; ; \
> +	else                                                                                                 \
> +		$(srctree)/scripts/coccinelle.sh $(COCCINELLE) $(@:coccicheck-%=%) $(COCCI) $(srctree)     ; \
> +	fi

Please sort the output of 'find' or use a shell glob instead. IMO the
order of tests should be stable (and thus easily diff-able) and not
change when you e.g. switch branches.


> +if [ -x "$SPATCH" ]; then
> +
> +    echo "Processing `basename $COCCI` with option(s) \"$(OPT)\""

/home/mmarek/linux-2.6/scripts/coccinelle.sh: line 13: OPT: command not
found

Please remove the parentheses around OPT.


> +    echo 'Message example to submit a patch:'
> +    sed -e '/\/\/\//!d' -e 's|^///||' $COCCI
> +
> +    echo ' The semantic patch that makes this change is available'
> +    echo " in $FILE."
> +    echo ''
> +    echo ' More information about semantic patching is available at'
> +    echo ' http://coccinelle.lip6.fr/'
> +    echo ''
> +
> +    $SPATCH -D $MODE -very_quiet -sp_file $COCCI $OPT -dir $DIR
> +
> +else
> +    echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/'
> +fi

If 'spatch' is not available, you should exit with an error.

Thanks,
Michal

PS: Sorry for the delay, I didn't get around to to reviewing the v2 of
your patch early enough and then I was offline for two weeks.

Re: [PATCH 1/4] Add targets to use the Coccinelle checker

[Sam Ravnborg]

On Mon, May 10, 2010 at 06:24:24PM +0200, Nicolas Palix wrote:
> Four targets are added. Each one generates a different
> output kind: context, patch, org, report.
> Every SmPL file in 'scripts/coccinelle' is given to the spatch frontend
> (located in the 'scripts' directory), and applied to the entire
> source tree.

A little late feedback.
IIRC all points raised at first review has been addressed - good.

> 
> Signed-off-by: Nicolas Palix <npalix@diku.dk>
> Signed-off-by: Julia Lawall <julia@diku.dk>
> ---
>  MAINTAINERS           |   10 ++++++++++
>  Makefile              |   20 ++++++++++++++++++--

The top-level Makefile is already in a very bad shape.
Adding more stuff there is never good.

How about adding some infrastructure to support the various tools?
Something along these lines:

All targets named %check will call a shell script:
scripts/%check.sh

"make help" will call a script named:
scripts/helpcheck.sh

helpcheck.sh will contain all the ugly details
about help for the various targets.

We need to rename a few seldomly used targets to do this.
Sample patch for top-level Makefile:

diff --git a/Makefile b/Makefile
index efdc3d0..40bf83e 100644
--- a/Makefile
+++ b/Makefile
@@ -412,7 +412,7 @@ endif
 # of make so .config is not included in this case either (for *config).
 
 no-dot-config-targets := clean mrproper distclean \
-			 cscope TAGS tags help %docs check% \
+			 cscope TAGS tags help %docs %check \
 			 include/linux/version.h headers_% \
 			 kernelrelease kernelversion
 
@@ -1024,10 +1024,6 @@ include/linux/version.h: $(srctree)/Makefile FORCE
 include/generated/utsrelease.h: include/config/kernel.release FORCE
 	$(call filechk,utsrelease.h)
 
-PHONY += headerdep
-headerdep:
-	$(Q)find include/ -name '*.h' | xargs --max-args 1 scripts/headerdep.pl
-
 # ---------------------------------------------------------------------------
 
 PHONY += depend dep
@@ -1273,13 +1269,9 @@ help:
 	 echo  '                    (default: $(INSTALL_HDR_PATH))'; \
 	 echo  ''
 	@echo  'Static analysers'
-	@echo  '  checkstack      - Generate a list of stack hogs'
-	@echo  '  namespacecheck  - Name space analysis on compiled kernel'
-	@echo  '  versioncheck    - Sanity check on version.h usage'
-	@echo  '  includecheck    - Check for duplicate included header files'
+	@$(CONFIG_SHELL)$(srctree)/scripts/helpcheck.sh
 	@echo  '  export_report   - List the usages of all exported symbols'
 	@echo  '  headers_check   - Sanity check on exported headers'
-	@echo  '  headerdep       - Detect inclusion cycles in headers'; \
 	 echo  ''
 	@echo  'Kernel packaging:'
 	@$(MAKE) $(build)=$(package-dir) help
@@ -1427,20 +1419,10 @@ tags TAGS cscope: FORCE
 	$(call cmd,tags)
 
 # Scripts to check various things for consistency
+# Call a helper script in scripts/ to do the actual job
 # ---------------------------------------------------------------------------
-
-includecheck:
-	find * $(RCS_FIND_IGNORE) \
-		-name '*.[hcS]' -type f -print | sort \
-		| xargs $(PERL) -w $(srctree)/scripts/checkincludes.pl
-
-versioncheck:
-	find * $(RCS_FIND_IGNORE) \
-		-name '*.[hcS]' -type f -print | sort \
-		| xargs $(PERL) -w $(srctree)/scripts/checkversion.pl
-
-namespacecheck:
-	$(PERL) $(srctree)/scripts/namespace.pl
+%check:
+	$(Q)$(CONFIG_SHELL)$(srctree)/scripts/$@.sh
 
 export_report:
 	$(PERL) $(srctree)/scripts/export_report.pl
@@ -1448,20 +1430,7 @@ export_report:
 endif #ifeq ($(config-targets),1)
 endif #ifeq ($(mixed-targets),1)
 
-PHONY += checkstack kernelrelease kernelversion
-
-# UML needs a little special treatment here.  It wants to use the host
-# toolchain, so needs $(SUBARCH) passed to checkstack.pl.  Everyone
-# else wants $(ARCH), including people doing cross-builds, which means
-# that $(SUBARCH) doesn't work here.
-ifeq ($(ARCH), um)
-CHECKSTACK_ARCH := $(SUBARCH)
-else
-CHECKSTACK_ARCH := $(ARCH)
-endif
-checkstack:
-	$(OBJDUMP) -d vmlinux $$(find . -name '*.ko') | \
-	$(PERL) $(src)/scripts/checkstack.pl $(CHECKSTACK_ARCH)
+PHONY += kernelrelease kernelversion
 
 kernelrelease:
 	$(if $(wildcard include/config/kernel.release), $(Q)echo $(KERNELRELEASE), \



A nice simplification of the top-level Mkefile.
and an opportunity to document some of the
magic shell commands used by the various tools.

If we do this I know the scripts will sum up to a lot more
lines but doing it distributed is better.
And some of the lines would be comments that explains what is
going on.

An additinal benefit is that we can avoid touching
the top-level Makefile next time we add a new
%check target.

As for cocci this would have the the effect that
new scipts can be made as drop-in - no changes
in any files. Just add the files to the
correct directory.

	Sam

Re: [PATCH 1/4] Add targets to use the Coccinelle checker

[Nicolas Palix]

On Thursday 03 June 2010 12:23:15 Sam Ravnborg wrote:
> On Mon, May 10, 2010 at 06:24:24PM +0200, Nicolas Palix wrote:
> > Four targets are added. Each one generates a different
> > output kind: context, patch, org, report.
> > Every SmPL file in 'scripts/coccinelle' is given to the spatch frontend
> > (located in the 'scripts' directory), and applied to the entire
> > source tree.
> 
> A little late feedback.
> IIRC all points raised at first review has been addressed - good.
> 
> > 
> > Signed-off-by: Nicolas Palix <npalix@diku.dk>
> > Signed-off-by: Julia Lawall <julia@diku.dk>
> > ---
> >  MAINTAINERS           |   10 ++++++++++
> >  Makefile              |   20 ++++++++++++++++++--
> 
> The top-level Makefile is already in a very bad shape.
> Adding more stuff there is never good.
> 
> How about adding some infrastructure to support the various tools?
> Something along these lines:
> 
> All targets named %check will call a shell script:
> scripts/%check.sh
> 
> "make help" will call a script named:
> scripts/helpcheck.sh
> 
> helpcheck.sh will contain all the ugly details
> about help for the various targets.
> 
> We need to rename a few seldomly used targets to do this.
> Sample patch for top-level Makefile:
> 
> diff --git a/Makefile b/Makefile
> index efdc3d0..40bf83e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -412,7 +412,7 @@ endif
>  # of make so .config is not included in this case either (for *config).
>  
>  no-dot-config-targets := clean mrproper distclean \
> -			 cscope TAGS tags help %docs check% \
> +			 cscope TAGS tags help %docs %check \
>  			 include/linux/version.h headers_% \
>  			 kernelrelease kernelversion
>  
> @@ -1024,10 +1024,6 @@ include/linux/version.h: $(srctree)/Makefile FORCE
>  include/generated/utsrelease.h: include/config/kernel.release FORCE
>  	$(call filechk,utsrelease.h)
>  
> -PHONY += headerdep
> -headerdep:
> -	$(Q)find include/ -name '*.h' | xargs --max-args 1 scripts/headerdep.pl
> -
>  # ---------------------------------------------------------------------------
>  
>  PHONY += depend dep
> @@ -1273,13 +1269,9 @@ help:
>  	 echo  '                    (default: $(INSTALL_HDR_PATH))'; \
>  	 echo  ''
>  	@echo  'Static analysers'
> -	@echo  '  checkstack      - Generate a list of stack hogs'
> -	@echo  '  namespacecheck  - Name space analysis on compiled kernel'
> -	@echo  '  versioncheck    - Sanity check on version.h usage'
> -	@echo  '  includecheck    - Check for duplicate included header files'
> +	@$(CONFIG_SHELL)$(srctree)/scripts/helpcheck.sh
>  	@echo  '  export_report   - List the usages of all exported symbols'
>  	@echo  '  headers_check   - Sanity check on exported headers'
> -	@echo  '  headerdep       - Detect inclusion cycles in headers'; \
>  	 echo  ''
>  	@echo  'Kernel packaging:'
>  	@$(MAKE) $(build)=$(package-dir) help
> @@ -1427,20 +1419,10 @@ tags TAGS cscope: FORCE
>  	$(call cmd,tags)
>  
>  # Scripts to check various things for consistency
> +# Call a helper script in scripts/ to do the actual job
>  # ---------------------------------------------------------------------------
> -
> -includecheck:
> -	find * $(RCS_FIND_IGNORE) \
> -		-name '*.[hcS]' -type f -print | sort \
> -		| xargs $(PERL) -w $(srctree)/scripts/checkincludes.pl
> -
> -versioncheck:
> -	find * $(RCS_FIND_IGNORE) \
> -		-name '*.[hcS]' -type f -print | sort \
> -		| xargs $(PERL) -w $(srctree)/scripts/checkversion.pl
> -
> -namespacecheck:
> -	$(PERL) $(srctree)/scripts/namespace.pl
> +%check:
> +	$(Q)$(CONFIG_SHELL)$(srctree)/scripts/$@.sh
>  
>  export_report:
>  	$(PERL) $(srctree)/scripts/export_report.pl
> @@ -1448,20 +1430,7 @@ export_report:
>  endif #ifeq ($(config-targets),1)
>  endif #ifeq ($(mixed-targets),1)
>  
> -PHONY += checkstack kernelrelease kernelversion
> -
> -# UML needs a little special treatment here.  It wants to use the host
> -# toolchain, so needs $(SUBARCH) passed to checkstack.pl.  Everyone
> -# else wants $(ARCH), including people doing cross-builds, which means
> -# that $(SUBARCH) doesn't work here.
> -ifeq ($(ARCH), um)
> -CHECKSTACK_ARCH := $(SUBARCH)
> -else
> -CHECKSTACK_ARCH := $(ARCH)
> -endif
> -checkstack:
> -	$(OBJDUMP) -d vmlinux $$(find . -name '*.ko') | \
> -	$(PERL) $(src)/scripts/checkstack.pl $(CHECKSTACK_ARCH)
> +PHONY += kernelrelease kernelversion
>  
>  kernelrelease:
>  	$(if $(wildcard include/config/kernel.release), $(Q)echo $(KERNELRELEASE), \
> 
> 
> 
> A nice simplification of the top-level Mkefile.
> and an opportunity to document some of the
> magic shell commands used by the various tools.
> 
> If we do this I know the scripts will sum up to a lot more
> lines but doing it distributed is better.
> And some of the lines would be comments that explains what is
> going on.
> 
> An additinal benefit is that we can avoid touching
> the top-level Makefile next time we add a new
> %check target.
> 
> As for cocci this would have the the effect that
> new scipts can be made as drop-in - no changes
> in any files. Just add the files to the
> correct directory.
> 
> 	Sam
> 

Hi,

I am not sure to understand your suggestion.

Should I make this change and integrate it into my patch series
or it is something to do later ?

In doing so, I will have to select the 'mode' in a different way.
I was thinking of renaming my targets in one of the following forms
or using an environment variable:

make cocci-report-check
make report-coccicheck
make coccicheck MODE=report

Do you have any preference ?

-- 
Nicolas Palix
Tel: (+33) 1 44 27 87 25
Tel: (+33) 6 81 07 91 72
Web: http://www.diku.dk/~npalix/

Re: [PATCH 1/4] Add targets to use the Coccinelle checker

[Sam Ravnborg]

> > 
> > A nice simplification of the top-level Mkefile.
> > and an opportunity to document some of the
> > magic shell commands used by the various tools.
...
> 
> I am not sure to understand your suggestion.
> 
> Should I make this change and integrate it into my patch series
> or it is something to do later ?

If we agree on this as the base infrastructure I would
like you to introduce it as part of adding Coccinelle support.

That should not touch the existing targets - but only
add Coccinelle support. But in a way so we have the
infrastructure in place so we can later move the other targets later.

> 
> In doing so, I will have to select the 'mode' in a different way.
> I was thinking of renaming my targets in one of the following forms
> or using an environment variable:
> 
> make cocci-report-check
> make report-coccicheck
> make coccicheck MODE=report

I like the latter.
If you do "make coccicheck" then you can show a list of
possible mode's. And if MODE is set you use this mode.

	Sam

[PATCH 2/4] Add scripts/coccinelle/drop_kmalloc_cast.cocci

[Nicolas Palix]

The purpose of this semantic patch is to remove
useless casts, as mentioned in the Linux documentation.
See Chapter 14 in Documentation/CodingStyle for more information.

Signed-off-by: Nicolas Palix <npalix@diku.dk>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
 scripts/coccinelle/drop_kmalloc_cast.cocci |   68 ++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)
 create mode 100644 scripts/coccinelle/drop_kmalloc_cast.cocci

diff --git a/scripts/coccinelle/drop_kmalloc_cast.cocci b/scripts/coccinelle/drop_kmalloc_cast.cocci
new file mode 100644
index 0000000..cb54a4b
--- /dev/null
+++ b/scripts/coccinelle/drop_kmalloc_cast.cocci
@@ -0,0 +1,68 @@
+///
+/// Casting (void *) value returned by kmalloc is useless
+/// as mentioned in Documentation/CodingStyle, Chap 14.
+///
+// Confidence: High
+// Copyright: 2009,2010 Nicolas Palix, DIKU.  GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Options: -no_includes -include_headers
+//
+// Keywords: kmalloc, kzalloc, kcalloc
+// Version min: < 2.6.12 kmalloc
+// Version min: < 2.6.12 kcalloc
+// Version min:   2.6.14 kzalloc
+// Version max: *
+//
+
+virtual context
+virtual patch
+virtual org
+virtual report
+
+//----------------------------------------------------------
+//  For context mode
+//----------------------------------------------------------
+
+@depends on context@
+type T;
+@@
+
+* (T *)
+  \(kmalloc\|kzalloc\|kcalloc\)(...)
+
+//----------------------------------------------------------
+//  For patch mode
+//----------------------------------------------------------
+
+@depends on patch@
+type T;
+@@
+
+- (T *)
+  \(kmalloc\|kzalloc\|kcalloc\)(...)
+
+//----------------------------------------------------------
+//  For org and report mode
+//----------------------------------------------------------
+
+@r depends on org || report@
+type T;
+position p;
+@@
+
+ (T@p *)\(kmalloc\|kzalloc\|kcalloc\)(...)
+
+@script:python depends on org@
+p << r.p;
+t << r.T;
+@@
+
+coccilib.org.print_safe_todo(p[0], t)
+
+@script:python depends on report@
+p << r.p;
+t << r.T;
+@@
+
+msg="WARNING: casting value returned by k[cmz]alloc to (%s *) is useless." % (t)
+coccilib.report.print_report(p[0], msg)
-- 
1.7.0.4

[PATCH 3/4] Add scripts/coccinelle/kzalloc-simple.cocci

[Nicolas Palix]

This semantic patch replaces a pair of calls to kmalloc and memset
by a single call to kzalloc.

It only looks for simple cases to avoid false positives.

Signed-off-by: Nicolas Palix <npalix@diku.dk>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
 scripts/coccinelle/kzalloc-simple.cocci |   83 +++++++++++++++++++++++++++++++
 1 files changed, 83 insertions(+), 0 deletions(-)
 create mode 100644 scripts/coccinelle/kzalloc-simple.cocci

diff --git a/scripts/coccinelle/kzalloc-simple.cocci b/scripts/coccinelle/kzalloc-simple.cocci
new file mode 100644
index 0000000..b099fea
--- /dev/null
+++ b/scripts/coccinelle/kzalloc-simple.cocci
@@ -0,0 +1,83 @@
+///
+/// kzalloc should be used rather than kmalloc followed by memset 0
+///
+// Confidence: High
+// Copyright: (C) 2009-2010 Julia Lawall, Nicolas Palix, DIKU.  GPLv2.
+// Copyright: (C) 2009-2010 Gilles Muller, INRIA/LiP6.  GPLv2.
+// URL: http://coccinelle.lip6.fr/rules/kzalloc.html
+// Options: -no_includes -include_headers
+//
+// Keywords: kmalloc, kzalloc
+// Version min: < 12 kmalloc
+// Version min:   14 kzalloc
+// Version max: *
+//
+
+virtual context
+virtual patch
+virtual org
+virtual report
+
+//----------------------------------------------------------
+//  For context mode
+//----------------------------------------------------------
+
+@depends on context@
+type T, T2;
+expression x;
+expression E1,E2;
+statement S;
+@@
+
+* x = (T)kmalloc(E1,E2);
+  if ((x=NULL) || ...) S
+* memset((T2)x,0,E1);
+
+//----------------------------------------------------------
+//  For patch mode
+//----------------------------------------------------------
+
+@depends on patch@
+type T, T2;
+expression x;
+expression E1,E2;
+statement S;
+@@
+
+- x = (T)kmalloc(E1,E2);
++ x = kzalloc(E1,E2);
+  if ((x=NULL) || ...) S
+- memset((T2)x,0,E1);
+
+//----------------------------------------------------------
+//  For org mode
+//----------------------------------------------------------
+
+@r depends on org || report@
+type T, T2;
+expression x;
+expression E1,E2;
+statement S;
+position p;
+@@
+
+ x = (T)kmalloc@p(E1,E2);
+ if ((x=NULL) || ...) S
+ memset((T2)x,0,E1);
+
+@script:python depends on org@
+p << r.p;
+x << r.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r.p;
+x << r.x;
+@@
+
+msg="WARNING: kzalloc should be used for %s, instead of kmalloc/memset" % (x)
+coccilib.report.print_report(p[0], msg_safe)
-- 
1.7.0.4

Re: [PATCH 3/4] Add scripts/coccinelle/kzalloc-simple.cocci

[Michal Marek]

On 10.5.2010 18:24, Nicolas Palix wrote:
> +@script:python depends on org@
> +p << r.p;
> +x << r.x;
> +@@
> +
> +msg="%s" % (x)
> +msg_safe=msg.replace("[","@(").replace("]",")")
> +coccilib.org.print_todo(p[0], msg_safe)
> +
> +@script:python depends on report@
> +p << r.p;
> +x << r.x;
> +@@
> +
> +msg="WARNING: kzalloc should be used for %s, instead of kmalloc/memset" % (x)
> +coccilib.report.print_report(p[0], msg_safe)

msg_safe is not defined in this scriptlet, I'm only getting

Traceback (most recent call last):
  File "<string>", line 5, in <module>
NameError: name 'msg_safe' is not defined

Michal

Re: [PATCH 3/4] Add scripts/coccinelle/kzalloc-simple.cocci

[Nicolas Palix]

On Thursday 03 June 2010 11:51:43 Michal Marek wrote:
> On 10.5.2010 18:24, Nicolas Palix wrote:
> > +@script:python depends on org@
> > +p << r.p;
> > +x << r.x;
> > +@@
> > +
> > +msg="%s" % (x)
> > +msg_safe=msg.replace("[","@(").replace("]",")")
> > +coccilib.org.print_todo(p[0], msg_safe)
> > +
> > +@script:python depends on report@
> > +p << r.p;
> > +x << r.x;
> > +@@
> > +
> > +msg="WARNING: kzalloc should be used for %s, instead of kmalloc/memset" % (x)
> > +coccilib.report.print_report(p[0], msg_safe)
> 
> msg_safe is not defined in this scriptlet, I'm only getting
> 
> Traceback (most recent call last):
>   File "<string>", line 5, in <module>
> NameError: name 'msg_safe' is not defined

Oops. Thank you for reporting this.

I fixed it for my upcoming submission.

In the meantime, you can replace "msg_safe" by "msg" in the rule
dedicated to the "report" mode.

That is

coccilib.report.print_report(p[0], msg_safe)

by 

coccilib.report.print_report(p[0], msg

The good point is that you only reach this point if Coccinelle has found
something to change in the code. So there is some patches to do... ;)


> 
> Michal
> 

-- 
Nicolas Palix
Tel: (+33) 1 44 27 87 25
Tel: (+33) 6 81 07 91 72
Web: http://www.diku.dk/~npalix/

[PATCH 4/4] Add scripts/coccinelle/resource_size.cocci

[Nicolas Palix]

This semantic patch replaces explicit computations
of resource size by a call to resource_size.

Signed-off-by: Nicolas Palix <npalix@diku.dk>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
 scripts/coccinelle/resource_size.cocci |   94 ++++++++++++++++++++++++++++++++
 1 files changed, 94 insertions(+), 0 deletions(-)
 create mode 100644 scripts/coccinelle/resource_size.cocci

diff --git a/scripts/coccinelle/resource_size.cocci b/scripts/coccinelle/resource_size.cocci
new file mode 100644
index 0000000..a1a007b
--- /dev/null
+++ b/scripts/coccinelle/resource_size.cocci
@@ -0,0 +1,94 @@
+///
+/// Use resource_size function on resource object
+/// instead of explicit computation.
+///
+//  Confidence: High
+//  Copyright: (C) 2009, 2010 Nicolas Palix, DIKU.  GPLv2.
+//  Copyright: (C) 2009, 2010 Julia Lawall, DIKU.  GPLv2.
+//  Copyright: (C) 2009, 2010 Gilles Muller, INRIA/LiP6.  GPLv2.
+//  URL: http://coccinelle.lip6.fr/
+//  Options:
+//
+//  Keywords: resource_size
+//  Version min: 2.6.27 resource_size
+//  Version max: *
+//
+
+virtual context
+virtual patch
+virtual org
+virtual report
+
+//----------------------------------------------------------
+//  For context mode
+//----------------------------------------------------------
+
+@r_context depends on context && !patch && !org@
+struct resource *res;
+@@
+
+* (res->end - res->start) + 1
+
+//----------------------------------------------------------
+//  For patch mode
+//----------------------------------------------------------
+
+@r_patch depends on !context && patch && !org@
+struct resource *res;
+@@
+
+- (res->end - res->start) + 1
++ resource_size(res)
+
+//----------------------------------------------------------
+//  For org mode
+//----------------------------------------------------------
+
+
+@r_org depends on !context && !patch && (org || report)@
+struct resource *res;
+position p;
+@@
+
+ (res->end@p - res->start) + 1
+
+@rbad_org depends on !context && !patch && (org || report)@
+struct resource *res;
+position p != r_org.p;
+@@
+
+ res->end@p - res->start
+
+@script:python depends on org@
+p << r_org.p;
+x << r_org.res;
+@@
+
+msg="ERROR with %s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r_org.p;
+x << r_org.res;
+@@
+
+msg="ERROR: Missing resource_size with %s" % (x)
+coccilib.report.print_report(p[0], msg)
+
+@script:python depends on org@
+p << rbad_org.p;
+x << rbad_org.res;
+@@
+
+msg="WARNING with %s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << rbad_org.p;
+x << rbad_org.res;
+@@
+
+msg="WARNING: Suspicious code. resource_size is maybe missing with %s" % (x)
+coccilib.report.print_report(p[0], msg)
-- 
1.7.0.4

Re: [PATCH 4/4] Add scripts/coccinelle/resource_size.cocci

[Pekka Enberg]

On Mon, May 10, 2010 at 7:24 PM, Nicolas Palix <npalix@diku.dk> wrote:
> @@ -0,0 +1,94 @@
> +///
> +/// Use resource_size function on resource object
> +/// instead of explicit computation.
> +///
> +//  Confidence: High
> +//  Copyright: (C) 2009, 2010 Nicolas Palix, DIKU.  GPLv2.
> +//  Copyright: (C) 2009, 2010 Julia Lawall, DIKU.  GPLv2.
> +//  Copyright: (C) 2009, 2010 Gilles Muller, INRIA/LiP6.  GPLv2.
> +//  URL: http://coccinelle.lip6.fr/
> +//  Options:
> +//
> +//  Keywords: resource_size
> +//  Version min: 2.6.27 resource_size

I guess we don't need this versioning in mainline?
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] Add scripts/coccinelle/resource_size.cocci

[Nicolas Palix]

On Monday 10 May 2010 18:52:10 Pekka Enberg wrote:
> On Mon, May 10, 2010 at 7:24 PM, Nicolas Palix <npalix@diku.dk> wrote:
> > @@ -0,0 +1,94 @@
> > +///
> > +/// Use resource_size function on resource object
> > +/// instead of explicit computation.
> > +///
> > +//  Confidence: High
> > +//  Copyright: (C) 2009, 2010 Nicolas Palix, DIKU.  GPLv2.
> > +//  Copyright: (C) 2009, 2010 Julia Lawall, DIKU.  GPLv2.
> > +//  Copyright: (C) 2009, 2010 Gilles Muller, INRIA/LiP6.  GPLv2.
> > +//  URL: http://coccinelle.lip6.fr/
> > +//  Options:
> > +//
> > +//  Keywords: resource_size
> > +//  Version min: 2.6.27 resource_size
> 
> I guess we don't need this versioning in mainline?
> 

It's indeed not essential for the mainline. It's however
helpful to document the internal APIs and will help people
who want to (back)port this patch series on other branches,
potentially branches forked before 2.6.27 for instance.

-- 
Nicolas Palix

Re: [PATCH 0/4] Add a Coccinelle front-end script

[Andy Isaacson]

On Mon, May 10, 2010 at 06:24:23PM +0200, Nicolas Palix wrote:
> New targets are added (coccicheck-<mode>) to call the 'coccinelle.sh'
> front-end in the 'scripts' directory with the <mode> argument.
> 
> Four modes are defined: report, patch, context, and org.

Please check-in this write-up somewhere (Documentation/README.coccinelle
perhaps?) along with information about how to get the required tools.
(Are they pacakged in Debian and/or Fedora?  What versions are the
scripts tested against?)

Also I'd like to see a short example of how to use the framework.  I've
read Val's spatch article (http://lwn.net/Articles/315686/) and it's
still a bit of a black art to me.

Thanks for the work!

-andy

Re: [PATCH 0/4] Add a Coccinelle front-end script

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 957 bytes --]

On Mon, May 10, 2010 at 07:14:16PM -0700, Andy Isaacson wrote:
> On Mon, May 10, 2010 at 06:24:23PM +0200, Nicolas Palix wrote:
> > New targets are added (coccicheck-<mode>) to call the 'coccinelle.sh'
> > front-end in the 'scripts' directory with the <mode> argument.
> > 
> > Four modes are defined: report, patch, context, and org.
> 
> Please check-in this write-up somewhere (Documentation/README.coccinelle
> perhaps?)

I still haven't made my mind regarding these four modes; but if they are used,
they should be documented. ACK.

> Also I'd like to see a short example of how to use the framework.  I've
> read Val's spatch article (http://lwn.net/Articles/315686/) and it's
> still a bit of a black art to me.

Something like this? :)

https://lwn.net/Articles/380835/

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 0/4] Add a Coccinelle front-end script

[Joerg Roedel]

On Mon, May 10, 2010 at 06:24:23PM +0200, Nicolas Palix wrote:
> Three semantic patches, with a low rate of false positives, are also
> included.

Is there a way to annotate false positives so that they do not produce
warnings over and over?

> Other semantic patches will be provided later. Note that a semantic
> patch does not need to define all four modes. As many semantic patches
> may be proposed later, they are organized under sub-directories of
> 'scripts/coccinelle/'.

Don't know how complex the coccinelle software itself is, but I think it
would make sense to ship it with the kernel-sources too. That together
with a README and everything is perfect :-)
Thanks for the great work, this should definitly be upstream.

	Joerg

Re: [PATCH 0/4] Add a Coccinelle front-end script

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

> Don't know how complex the coccinelle software itself is, but I think it
> would make sense to ship it with the kernel-sources too. That together

You can easily find out by browsing the homepage of coccinelle.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 0/4] Add a Coccinelle front-end script

[Julia Lawall]

On Fri, 28 May 2010, Wolfram Sang wrote:

> > Don't know how complex the coccinelle software itself is, but I think it
> > would make sense to ship it with the kernel-sources too. That together
> 
> You can easily find out by browsing the homepage of coccinelle.

Over 100 000 lines of code.

julia

Re: [PATCH 0/4] Add a Coccinelle front-end script

[walter harms]

Julia Lawall schrieb:
> On Fri, 28 May 2010, Wolfram Sang wrote:
> 
>>> Don't know how complex the coccinelle software itself is, but I think it
>>> would make sense to ship it with the kernel-sources too. That together
>> You can easily find out by browsing the homepage of coccinelle.
> 
> Over 100 000 lines of code.
> 
> julia
> --

coccinelle is a great programm and should have its own repository.
I don not thing thats a clever idea to integrate it directly into
the kernel source tree. IMHO it would hinder the development of coccinelle
in the long term.

re,
 wh

Re: [PATCH 0/4] Add a Coccinelle front-end script

[Joerg Roedel]

On Fri, May 28, 2010 at 09:31:34AM +0200, Julia Lawall wrote:
> On Fri, 28 May 2010, Wolfram Sang wrote:
> 
> > > Don't know how complex the coccinelle software itself is, but I think it
> > > would make sense to ship it with the kernel-sources too. That together
> > 
> > You can easily find out by browsing the homepage of coccinelle.
> 
> Over 100 000 lines of code.

I just noticed that distributions already ship it in their repositories.
So this its okay to keep it outside the kernel sources.

	Joerg

[PATCH 3/4] Add scripts/coccinelle/kzalloc-simple.cocci

[Nicolas Palix]

This semantic patch replaces a pair of calls to kmalloc and memset
by a single call to kzalloc.

It only looks for simple cases to avoid false positives.

Signed-off-by: Nicolas Palix <npalix@diku.dk>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
 scripts/coccinelle/kzalloc-simple.cocci |   83 +++++++++++++++++++++++++++++++
 1 files changed, 83 insertions(+), 0 deletions(-)
 create mode 100644 scripts/coccinelle/kzalloc-simple.cocci

diff --git a/scripts/coccinelle/kzalloc-simple.cocci b/scripts/coccinelle/kzalloc-simple.cocci
new file mode 100644
index 0000000..b099fea
--- /dev/null
+++ b/scripts/coccinelle/kzalloc-simple.cocci
@@ -0,0 +1,83 @@
+///
+/// kzalloc should be used rather than kmalloc followed by memset 0
+///
+// Confidence: High
+// Copyright: (C) 2009-2010 Julia Lawall, Nicolas Palix, DIKU.  GPLv2.
+// Copyright: (C) 2009-2010 Gilles Muller, INRIA/LiP6.  GPLv2.
+// URL: http://coccinelle.lip6.fr/rules/kzalloc.html
+// Options: -no_includes -include_headers
+//
+// Keywords: kmalloc, kzalloc
+// Version min: < 12 kmalloc
+// Version min:   14 kzalloc
+// Version max: *
+//
+
+virtual context
+virtual patch
+virtual org
+virtual report
+
+//----------------------------------------------------------
+//  For context mode
+//----------------------------------------------------------
+
+@depends on context@
+type T, T2;
+expression x;
+expression E1,E2;
+statement S;
+@@
+
+* x = (T)kmalloc(E1,E2);
+  if ((x=NULL) || ...) S
+* memset((T2)x,0,E1);
+
+//----------------------------------------------------------
+//  For patch mode
+//----------------------------------------------------------
+
+@depends on patch@
+type T, T2;
+expression x;
+expression E1,E2;
+statement S;
+@@
+
+- x = (T)kmalloc(E1,E2);
++ x = kzalloc(E1,E2);
+  if ((x=NULL) || ...) S
+- memset((T2)x,0,E1);
+
+//----------------------------------------------------------
+//  For org mode
+//----------------------------------------------------------
+
+@r depends on org || report@
+type T, T2;
+expression x;
+expression E1,E2;
+statement S;
+position p;
+@@
+
+ x = (T)kmalloc@p(E1,E2);
+ if ((x=NULL) || ...) S
+ memset((T2)x,0,E1);
+
+@script:python depends on org@
+p << r.p;
+x << r.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r.p;
+x << r.x;
+@@
+
+msg="WARNING: kzalloc should be used for %s, instead of kmalloc/memset" % (x)
+coccilib.report.print_report(p[0], msg_safe)
-- 
1.7.0.4