New targets are added (coccicheck-<mode>) to call the spatch 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 added.
Other semantic patches will be propose later.
Nicolas Palix (4):
Add targets to use the Coccinelle checker
Add scripts/smpl/drop_kmalloc_cast.cocci
Add scripts/smpl/kzalloc-simple.cocci
Add scripts/smpl/resource_size.cocci
MAINTAINERS | 10 +++
Makefile | 9 +++
scripts/smpl/drop_kmalloc_cast.cocci | 74 +++++++++++++++++++++++++
scripts/smpl/kzalloc-simple.cocci | 88 +++++++++++++++++++++++++++++
scripts/smpl/resource_size.cocci | 101 ++++++++++++++++++++++++++++++++++
scripts/spatch.sh | 14 +++++
6 files changed, 296 insertions(+), 0 deletions(-)
create mode 100644 scripts/smpl/drop_kmalloc_cast.cocci
create mode 100644 scripts/smpl/kzalloc-simple.cocci
create mode 100644 scripts/smpl/resource_size.cocci
create mode 100755 scripts/spatch.sh
Four targets are added. Each one generates a different
output kind: context, patch, org, report.
Every SmPL file in 'scripts/smpl' is given to the spatch frontend
(located in the 'scripts' directory), and applied to the entire
source tree.
Signed-off-by: Nicolas Palix <[email protected]>
---
MAINTAINERS | 10 ++++++++++
Makefile | 9 +++++++++
scripts/spatch.sh | 14 ++++++++++++++
3 files changed, 33 insertions(+), 0 deletions(-)
create mode 100755 scripts/spatch.sh
diff --git a/MAINTAINERS b/MAINTAINERS
index 3d29fa3..2aab763 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1533,6 +1533,16 @@ L: [email protected]
S: Supported
F: drivers/platform/x86/classmate-laptop.c
+COCCINELLE
+M: Julia Lawall <[email protected]>
+M: Gilles Muller <[email protected]>
+M: Nicolas Palix <[email protected]>
+L: [email protected]
+W: http://coccinelle.lip6.fr/
+S: Supported
+F: scripts/smpl/
+F: scripts/spatch.sh
+
CODA FILE SYSTEM
M: Jan Harkes <[email protected]>
M: [email protected]
diff --git a/Makefile b/Makefile
index 67c1001..293c88b 100644
--- a/Makefile
+++ b/Makefile
@@ -325,6 +325,7 @@ INSTALLKERNEL := installkernel
DEPMOD = /sbin/depmod
KALLSYMS = scripts/kallsyms
PERL = perl
+SPATCH = spatch
CHECK = sparse
CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
@@ -1424,6 +1425,14 @@ versioncheck:
-name '*.[hcS]' -type f -print | sort \
| xargs $(PERL) -w $(srctree)/scripts/checkversion.pl
+coccicheck-context coccicheck-patch coccicheck-org coccicheck-report:
+ @echo "\nPlease, check for false positive in the output before submitting a patch.\n\n"\
+ "Take particularly attention when using the \"patch\" mode\n"\
+ "and carefully review the patch YOU are about to submit.\n"
+ @find $(srctree)/scripts/smpl/ \
+ -name '*.cocci' -type f \
+ -exec $(srctree)/scripts/spatch.sh $(SPATCH) $(@:coccicheck-%=%) \{} $(srctree) \;
+
namespacecheck:
$(PERL) $(srctree)/scripts/namespace.pl
diff --git a/scripts/spatch.sh b/scripts/spatch.sh
new file mode 100755
index 0000000..bdcca15
--- /dev/null
+++ b/scripts/spatch.sh
@@ -0,0 +1,14 @@
+#!/bin/sh
+
+SPATCH="$1"
+MODE="$2"
+COCCI="$3"
+DIR="$4"
+
+OPT=`grep "Option" $COCCI | cut -d':' -f2`
+FILE=`echo $COCCI | sed "s|$DIR/||"`
+
+echo Processing `basename $COCCI` with \"$OPT\"
+echo Message example to submit a patch:
+grep "^///" $COCCI | sed "s|///||" | sed "s|THISFILE|$FILE|"
+$SPATCH -D $MODE -very_quiet -sp_file $COCCI $OPT -dir $DIR
--
1.6.3.3
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 <[email protected]>
---
scripts/smpl/drop_kmalloc_cast.cocci | 74 ++++++++++++++++++++++++++++++++++
1 files changed, 74 insertions(+), 0 deletions(-)
create mode 100644 scripts/smpl/drop_kmalloc_cast.cocci
diff --git a/scripts/smpl/drop_kmalloc_cast.cocci b/scripts/smpl/drop_kmalloc_cast.cocci
new file mode 100644
index 0000000..fbd3950
--- /dev/null
+++ b/scripts/smpl/drop_kmalloc_cast.cocci
@@ -0,0 +1,74 @@
+///
+/// Casting (void *) value returned by kmalloc is useless
+/// as mentioned in Documentation/CodingStyle, Chap 14.
+///
+/// The semantic patch that makes this change is available
+/// in THISFILE.
+///
+/// More information about semantic patching is available at
+/// http://coccinelle.lip6.fr/
+///
+// 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.6.3.3
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 improve the confidence.
Signed-off-by: Nicolas Palix <[email protected]>
---
scripts/smpl/kzalloc-simple.cocci | 88 +++++++++++++++++++++++++++++++++++++
1 files changed, 88 insertions(+), 0 deletions(-)
create mode 100644 scripts/smpl/kzalloc-simple.cocci
diff --git a/scripts/smpl/kzalloc-simple.cocci b/scripts/smpl/kzalloc-simple.cocci
new file mode 100644
index 0000000..d4e1b25
--- /dev/null
+++ b/scripts/smpl/kzalloc-simple.cocci
@@ -0,0 +1,88 @@
+///
+/// kzalloc should be used rather than kmalloc followed by memset 0
+///
+/// The semantic patch that makes this change is available
+/// in THISFILE.
+///
+/// More information about semantic patching is available at
+/// http://coccinelle.lip6.fr/
+///
+// Confidence: High
+// Copyright: (C) 2009-2010 Gilles Muller, Julia Lawall, Nicolas Palix, EMN, DIKU. 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.6.3.3
This semantic patch replaces explicit computations
of resource size by a call to resource_size.
Signed-off-by: Nicolas Palix <[email protected]>
---
scripts/smpl/resource_size.cocci | 101 ++++++++++++++++++++++++++++++++++++++
1 files changed, 101 insertions(+), 0 deletions(-)
create mode 100644 scripts/smpl/resource_size.cocci
diff --git a/scripts/smpl/resource_size.cocci b/scripts/smpl/resource_size.cocci
new file mode 100644
index 0000000..e5e24ac
--- /dev/null
+++ b/scripts/smpl/resource_size.cocci
@@ -0,0 +1,101 @@
+///
+/// Use resource_size function on resource object
+/// instead of explicit computation.
+///
+/// The semantic patch that makes this change is available
+/// in THISFILE.
+///
+/// More information about semantic patching is available at
+/// http://coccinelle.lip6.fr/
+///
+// Confidence: High
+// Copyright: (C) 2009, 2010 Nicolas Palix, DIKU. GPLv2.
+// Copyright: (C) 2009, 2010 Julia Lawall, DIKU. GPLv2.
+// Copyright: (C) 2009, 2010 Gilles Muller, EMN. 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.6.3.3
I like the concept and believe the kernel tree is a better
repository for these scripts than an external website.
On Mon, 2010-04-26 at 23:11 +0200, Nicolas Palix wrote:
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3d29fa3..2aab763 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1533,6 +1533,16 @@ L: [email protected]
> S: Supported
> F: drivers/platform/x86/classmate-laptop.c
>
> +COCCINELLE
COCCINELLE/Semantic Patches (SmPL)
> +coccicheck-context coccicheck-patch coccicheck-org coccicheck-report:
> + @echo "\nPlease, check for false positive in the output before submitting a patch.\n\n"\
Please check for false positives etc...
> + "Take particularly attention when using the \"patch\" mode\n"\
> + "and carefully review the patch YOU are about to submit.\n"
you
On Monday 26 April 2010 23:37:01 Joe Perches wrote:
> I like the concept and believe the kernel tree is a better
> repository for these scripts than an external website.
Thank you for your feedback and support.
I attached an updated version of the patch 1/4.
>
> On Mon, 2010-04-26 at 23:11 +0200, Nicolas Palix wrote:
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 3d29fa3..2aab763 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1533,6 +1533,16 @@ L: [email protected]
> > S: Supported
> > F: drivers/platform/x86/classmate-laptop.c
> >
> > +COCCINELLE
>
> COCCINELLE/Semantic Patches (SmPL)
>
> > +coccicheck-context coccicheck-patch coccicheck-org coccicheck-report:
> > + @echo "\nPlease, check for false positive in the output before submitting a patch.\n\n"\
>
> Please check for false positives etc...
>
> > + "Take particularly attention when using the \"patch\" mode\n"\
> > + "and carefully review the patch YOU are about to submit.\n"
>
> you
>
>
>
>
>From 98aba248f7cba64c771f3632c11b8188819c45a1 Mon Sep 17 00:00:00 2001
From: Nicolas Palix <[email protected]>
Date: Sun, 4 Apr 2010 15:42:57 +0200
Subject: [PATCH 1/4] Add targets to use the Coccinelle checker
Four targets are added. Each one generates a different
output kind: context, patch, org, report.
Every SmPL file in 'scripts/smpl' is given to the spatch frontend
(located in the 'scripts' directory), and applied to the entire
source tree.
Signed-off-by: Nicolas Palix <[email protected]>
---
MAINTAINERS | 10 ++++++++++
Makefile | 9 +++++++++
scripts/spatch.sh | 14 ++++++++++++++
3 files changed, 33 insertions(+), 0 deletions(-)
create mode 100755 scripts/spatch.sh
diff --git a/MAINTAINERS b/MAINTAINERS
index 3d29fa3..2aab763 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1533,6 +1533,16 @@ L: [email protected]
S: Supported
F: drivers/platform/x86/classmate-laptop.c
+COCCINELLE/Semantic Patches (SmPL)
+M: Julia Lawall <[email protected]>
+M: Gilles Muller <[email protected]>
+M: Nicolas Palix <[email protected]>
+L: [email protected]
+W: http://coccinelle.lip6.fr/
+S: Supported
+F: scripts/smpl/
+F: scripts/spatch.sh
+
CODA FILE SYSTEM
M: Jan Harkes <[email protected]>
M: [email protected]
diff --git a/Makefile b/Makefile
index 67c1001..293c88b 100644
--- a/Makefile
+++ b/Makefile
@@ -325,6 +325,7 @@ INSTALLKERNEL := installkernel
DEPMOD = /sbin/depmod
KALLSYMS = scripts/kallsyms
PERL = perl
+SPATCH = spatch
CHECK = sparse
CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
@@ -1424,6 +1425,14 @@ versioncheck:
-name '*.[hcS]' -type f -print | sort \
| xargs $(PERL) -w $(srctree)/scripts/checkversion.pl
+coccicheck-context coccicheck-patch coccicheck-org coccicheck-report:
+ @echo "\nPlease check for false positive in the output before submitting a patch.\n\n"\
+ "Take particularly attention when using the \"patch\" mode\n"\
+ "and carefully review the patch you are about to submit.\n"
+ @find $(srctree)/scripts/smpl/ \
+ -name '*.cocci' -type f \
+ -exec $(srctree)/scripts/spatch.sh $(SPATCH) $(@:coccicheck-%=%) \{} $(srctree) \;
+
namespacecheck:
$(PERL) $(srctree)/scripts/namespace.pl
diff --git a/scripts/spatch.sh b/scripts/spatch.sh
new file mode 100755
index 0000000..bdcca15
--- /dev/null
+++ b/scripts/spatch.sh
@@ -0,0 +1,14 @@
+#!/bin/sh
+
+SPATCH="$1"
+MODE="$2"
+COCCI="$3"
+DIR="$4"
+
+OPT=`grep "Option" $COCCI | cut -d':' -f2`
+FILE=`echo $COCCI | sed "s|$DIR/||"`
+
+echo Processing `basename $COCCI` with \"$OPT\"
+echo Message example to submit a patch:
+grep "^///" $COCCI | sed "s|///||" | sed "s|THISFILE|$FILE|"
+$SPATCH -D $MODE -very_quiet -sp_file $COCCI $OPT -dir $DIR
--
1.6.3.3
Nicolas Palix wrote:
> diff --git a/Makefile b/Makefile
> index 67c1001..293c88b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1424,6 +1425,14 @@ versioncheck:
> -name '*.[hcS]' -type f -print | sort \
> | xargs $(PERL) -w $(srctree)/scripts/checkversion.pl
>
> +coccicheck-context coccicheck-patch coccicheck-org coccicheck-report:
> + @echo "\nPlease check for false positive in the output before submitting a patch.\n\n"\
positives
> + "Take particularly attention when using the \"patch\" mode\n"\
particular
> + "and carefully review the patch you are about to submit.\n"
> + @find $(srctree)/scripts/smpl/ \
> + -name '*.cocci' -type f \
> + -exec $(srctree)/scripts/spatch.sh $(SPATCH) $(@:coccicheck-%=%) \{} $(srctree) \;
> +
> namespacecheck:
> $(PERL) $(srctree)/scripts/namespace.pl
>
Hi Nicolas,
On 27.4.2010 00:20, Nicolas Palix wrote:
> Four targets are added. Each one generates a different
> output kind: context, patch, org, report.
> Every SmPL file in 'scripts/smpl' is given to the spatch frontend
> (located in the 'scripts' directory), and applied to the entire
> source tree.
Cool!
> Signed-off-by: Nicolas Palix <[email protected]>
> ---
> MAINTAINERS | 10 ++++++++++
> Makefile | 9 +++++++++
> scripts/spatch.sh | 14 ++++++++++++++
> 3 files changed, 33 insertions(+), 0 deletions(-)
> create mode 100755 scripts/spatch.sh
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3d29fa3..2aab763 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1533,6 +1533,16 @@ L: [email protected]
> S: Supported
> F: drivers/platform/x86/classmate-laptop.c
>
> +COCCINELLE/Semantic Patches (SmPL)
> +M: Julia Lawall <[email protected]>
> +M: Gilles Muller <[email protected]>
> +M: Nicolas Palix <[email protected]>
> +L: [email protected]
> +W: http://coccinelle.lip6.fr/
> +S: Supported
> +F: scripts/smpl/
> +F: scripts/spatch.sh
> +
> CODA FILE SYSTEM
> M: Jan Harkes <[email protected]>
> M: [email protected]
> diff --git a/Makefile b/Makefile
> index 67c1001..293c88b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -325,6 +325,7 @@ INSTALLKERNEL := installkernel
> DEPMOD = /sbin/depmod
> KALLSYMS = scripts/kallsyms
> PERL = perl
> +SPATCH = spatch
> CHECK = sparse
>
> CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
> @@ -1424,6 +1425,14 @@ versioncheck:
> -name '*.[hcS]' -type f -print | sort \
> | xargs $(PERL) -w $(srctree)/scripts/checkversion.pl
>
> +coccicheck-context coccicheck-patch coccicheck-org coccicheck-report:
> + @echo "\nPlease check for false positive in the output before submitting a patch.\n\n"\
> + "Take particularly attention when using the \"patch\" mode\n"\
> + "and carefully review the patch you are about to submit.\n"
This should be echo -e in bash, but then it won't work in dash I guess.
So better use multiple echo commands. Also please add the targets to
'make help' text and add coccicheck-% to the no-dot-config-targets variable.
> + @find $(srctree)/scripts/smpl/ \
> + -name '*.cocci' -type f \
> + -exec $(srctree)/scripts/spatch.sh $(SPATCH) $(@:coccicheck-%=%) \{} $(srctree) \;
> +
Please use 'for file in $(srctree)/scripts/smpl/*.cocci; do ...', so
that the reports are in a defined order. Or do you plan to use
subdirectories below scripts/smpl?
> namespacecheck:
> $(PERL) $(srctree)/scripts/namespace.pl
>
> diff --git a/scripts/spatch.sh b/scripts/spatch.sh
> new file mode 100755
> index 0000000..bdcca15
> --- /dev/null
> +++ b/scripts/spatch.sh
> @@ -0,0 +1,14 @@
> +#!/bin/sh
> +
> +SPATCH="$1"
> +MODE="$2"
> +COCCI="$3"
> +DIR="$4"
> +
> +OPT=`grep "Option" $COCCI | cut -d':' -f2`
> +FILE=`echo $COCCI | sed "s|$DIR/||"`
> +
> +echo Processing `basename $COCCI` with \"$OPT\"
> +echo Message example to submit a patch:
> +grep "^///" $COCCI | sed "s|///||" | sed "s|THISFILE|$FILE|"
echo "The semantic patch that makes this change is available"
echo "in $FILE"
Then you don't need to add the same comment to each of the *.cocci
files. Also is it necessary to advertise
"More information about semantic patching is available at
http://coccinelle.lip6.fr/"
before processing each *.cocci file? If you want the banner, you could
append it to the "Please check for false positives..." text printed once
in the beginning.
> +$SPATCH -D $MODE -very_quiet -sp_file $COCCI $OPT -dir $DIR
You can also print the URL here if the spatch command is not available.
Michal
On Mon, Apr 26, 2010 at 11:11:15PM +0200, Nicolas Palix wrote:
> New targets are added (coccicheck-<mode>) to call the spatch 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.
So, as I understood all semantic patches have to support 'org'?
I think this makes submitting slightly more complicated...
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Tue, 27 Apr 2010, Wolfram Sang wrote:
> On Mon, Apr 26, 2010 at 11:11:15PM +0200, Nicolas Palix wrote:
> > New targets are added (coccicheck-<mode>) to call the spatch 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.
>
> So, as I understood all semantic patches have to support 'org'?
> I think this makes submitting slightly more complicated...
No, this should not be the case. It just won't do anything if you request
org output and nothing is defined to produce org output.
julia
On Mon, Apr 26, 2010 at 11:11:16PM +0200, Nicolas Palix wrote:
> Four targets are added. Each one generates a different
> output kind: context, patch, org, report.
> Every SmPL file in 'scripts/smpl' is given to the spatch frontend
> (located in the 'scripts' directory), and applied to the entire
> source tree.
>
> Signed-off-by: Nicolas Palix <[email protected]>
> ---
> MAINTAINERS | 10 ++++++++++
> Makefile | 9 +++++++++
> scripts/spatch.sh | 14 ++++++++++++++
> 3 files changed, 33 insertions(+), 0 deletions(-)
> create mode 100755 scripts/spatch.sh
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3d29fa3..2aab763 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1533,6 +1533,16 @@ L: [email protected]
> S: Supported
> F: drivers/platform/x86/classmate-laptop.c
>
> +COCCINELLE
> +M: Julia Lawall <[email protected]>
> +M: Gilles Muller <[email protected]>
> +M: Nicolas Palix <[email protected]>
> +L: [email protected]
> +W: http://coccinelle.lip6.fr/
> +S: Supported
> +F: scripts/smpl/
> +F: scripts/spatch.sh
> +
> CODA FILE SYSTEM
> M: Jan Harkes <[email protected]>
> M: [email protected]
> diff --git a/Makefile b/Makefile
> index 67c1001..293c88b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -325,6 +325,7 @@ INSTALLKERNEL := installkernel
> DEPMOD = /sbin/depmod
> KALLSYMS = scripts/kallsyms
> PERL = perl
> +SPATCH = spatch
> CHECK = sparse
>
> CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
> @@ -1424,6 +1425,14 @@ versioncheck:
> -name '*.[hcS]' -type f -print | sort \
> | xargs $(PERL) -w $(srctree)/scripts/checkversion.pl
>
> +coccicheck-context coccicheck-patch coccicheck-org coccicheck-report:
> + @echo "\nPlease, check for false positive in the output before submitting a patch.\n\n"\
> + "Take particularly attention when using the \"patch\" mode\n"\
> + "and carefully review the patch YOU are about to submit.\n"
> + @find $(srctree)/scripts/smpl/ \
> + -name '*.cocci' -type f \
> + -exec $(srctree)/scripts/spatch.sh $(SPATCH) $(@:coccicheck-%=%) \{} $(srctree) \;
> +
> namespacecheck:
> $(PERL) $(srctree)/scripts/namespace.pl
>
> diff --git a/scripts/spatch.sh b/scripts/spatch.sh
> new file mode 100755
> index 0000000..bdcca15
> --- /dev/null
> +++ b/scripts/spatch.sh
> @@ -0,0 +1,14 @@
> +#!/bin/sh
> +
> +SPATCH="$1"
> +MODE="$2"
> +COCCI="$3"
> +DIR="$4"
> +
> +OPT=`grep "Option" $COCCI | cut -d':' -f2`
> +FILE=`echo $COCCI | sed "s|$DIR/||"`
> +
> +echo Processing `basename $COCCI` with \"$OPT\"
> +echo Message example to submit a patch:
> +grep "^///" $COCCI | sed "s|///||" | sed "s|THISFILE|$FILE|"
I think the part containing "THISFILE" the patches should be generated here at
runtime. Also, the grep-command can be done via sed.
> +$SPATCH -D $MODE -very_quiet -sp_file $COCCI $OPT -dir $DIR
> --
> 1.6.3.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On 27.4.2010 00:20, Nicolas Palix wrote:
> +L: [email protected]
The list should be marked as "moderated for non-subscribers".
Michal
Hej Nicolas.
On Mon, Apr 26, 2010 at 11:11:16PM +0200, Nicolas Palix wrote:
> Four targets are added. Each one generates a different
> output kind: context, patch, org, report.
> Every SmPL file in 'scripts/smpl' is given to the spatch frontend
> (located in the 'scripts' directory), and applied to the entire
> source tree.
For outsiders smpl does not rellate to coccinelle.
I suggest to consistently use the name of the tool in all places.
So smpl directory to be named coccinelle
SPATCH named coccinelle
spatch.sh named coccinelle.sh
etc.
This way we use one name for one thing. And not today where you
use three names for the same thing.
The files may be named *.smpl - because their home in
scripts/coccinelle/
will tell what they are used for.
[I regret that I used CHECK to name the sparse tool].
Sam
On Tue, Apr 27, 2010 at 10:24:03PM +0200, Sam Ravnborg wrote:
> Hej Nicolas.
>
> On Mon, Apr 26, 2010 at 11:11:16PM +0200, Nicolas Palix wrote:
> > Four targets are added. Each one generates a different
> > output kind: context, patch, org, report.
> > Every SmPL file in 'scripts/smpl' is given to the spatch frontend
> > (located in the 'scripts' directory), and applied to the entire
> > source tree.
>
> For outsiders smpl does not rellate to coccinelle.
>
> I suggest to consistently use the name of the tool in all places.
>
> So smpl directory to be named coccinelle
> SPATCH named coccinelle
> spatch.sh named coccinelle.sh
> etc.
>
> This way we use one name for one thing. And not today where you
> use three names for the same thing.
> The files may be named *.smpl - because their home in
>
> scripts/coccinelle/
>
> will tell what they are used for.
Looking again they are named *.cocci. So I confused myself on the smpl thing.
The point above consistent naming still holds.
Sam
> > + "Take particularly attention when using the \"patch\" mode\n"\
>
> particular
Actually, "take particular attention" is not particularly grammatical.
I might phrase this as
Be especially careful when using the "patch" mode to review the patch
you are about to submit.
- R.
--
Roland Dreier <[email protected]> || For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/index.html
On Thu, 29 Apr 2010 10:01:54 -0700 Roland Dreier wrote:
> > > + "Take particularly attention when using the \"patch\" mode\n"\
> >
> > particular
>
> Actually, "take particular attention" is not particularly grammatical.
> I might phrase this as
>
> Be especially careful when using the "patch" mode to review the patch
> you are about to submit.
Yes, or:
When using "patch" mode, carefully review the patch before submitting it.
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
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.
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 <[email protected]>
> Signed-off-by: Julia Lawall <[email protected]>
> ---
> 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/[email protected]
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
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 <[email protected]>
> > Signed-off-by: Julia Lawall <[email protected]>
> > ---
> > 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/[email protected]
>
> 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/
> >
> > 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