coccicheck: modernize

This has no functional changes. This is being done to enable us to later use spatch binary for some flag checking for certain features early on.

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 scripts/coccicheck | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

scripts/coccicheck +5 -5

index f6627863fdc3..f137b04dfdd3 100755
@@ -7,6 +7,11 @@
 
 SPATCH="`which ${SPATCH:=spatch}`"
 
+if [ ! -x "$SPATCH" ]; then
+    echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/'
+    exit 1
+fi
+
 trap kill_running SIGTERM SIGINT
 declare -a SPATCH_PID
 

@@ -51,11 +56,6 @@ if [ "$KBUILD_EXTMOD" != "" ] ; then
     OPTIONS="--patch $srctree $OPTIONS"
 fi
 
-if [ ! -x "$SPATCH" ]; then
-    echo 'spatch is part of the Coccinelle project and is available at http://coccinelle.lip6.fr/'
-    exit 1
-fi
-
 if [ "$MODE" = "" ] ; then
     if [ "$ONLINE" = "0" ] ; then
 	echo 'You have not explicitly specified the mode to use. Using default "report" mode.'

SPFLAGS is set early, it means that any heuristics done on coccicheck cannot be overridden currently. Move SPFLAGS after OPTIONS and set this at the end. This lets you override any heuristics as coccinelle treats conflicts by only listening to the last option that makes sense.

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 scripts/coccicheck | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

scripts/coccicheck +4 -1

index f137b04dfdd3..5319fae910b4 100755
@@ -30,7 +30,7 @@ else
 	NPROC="$J"
 fi
 
-FLAGS="--very-quiet $SPFLAGS"
+FLAGS="--very-quiet"
 
 # spatch only allows include directories with the syntax "-I include"
 # while gcc also allows "-Iinclude" and "-include include"

@@ -106,6 +106,9 @@ kill_running() {
 	done
 }
 
+# You can override heuristics with SPFLAGS, these must always go last
+OPTIONS="$OPTIONS $SPFLAGS"
+
 coccinelle () {
     COCCI="$1"
 

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Also since while at it propagate back into the shell script any coccinelle error code. When used in serialized mode where all cocci files are run this also stops processing if an error has occured. This lets us handle some errors in coccinelle cocci files and if they bail out we should inspect the errors. This will be more useful later to help annotate coccinelle version dependency requirements. This will let you run only SmPL files that your system supports.

As a small example, prior to this change, on an 8-core system:

Before:

$ export COCCI=scripts/coccinelle/free/kfree.cocci
$ time make coccicheck MODE=report DEBUG_FILE=cocci.err
...

real    29m14.912s
user    103m1.796s
sys     0m4.464s

After:

real    16m22.435s
user    128m30.060s
sys     0m2.712s

v3:

o move USE_JOBS to avoid being overriden

v2:

o redirect coccinelle stderr to /dev/null by default and only if DEBUG_FILE is used do we pass it to a file
o fix typo of paramap/parmap

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 scripts/coccicheck | 41 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

scripts/coccicheck +38 -3

index 5319fae910b4..a77f0f246405 100755
@@ -12,8 +12,8 @@ if [ ! -x "$SPATCH" ]; then
     exit 1
 fi
 
-trap kill_running SIGTERM SIGINT
-declare -a SPATCH_PID
+USE_JOBS="no"
+$SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes"
 
 # The verbosity may be set by the environmental parameter V=
 # as for example with 'make V=1 coccicheck'

@@ -56,6 +56,14 @@ if [ "$KBUILD_EXTMOD" != "" ] ; then
     OPTIONS="--patch $srctree $OPTIONS"
 fi
 
+# You can override by using SPFLAGS
+if [ "$USE_JOBS" = "no" ]; then
+	trap kill_running SIGTERM SIGINT
+	declare -a SPATCH_PID
+else
+	OPTIONS="$OPTIONS --jobs $NPROC --chunksize 1"
+fi
+
 if [ "$MODE" = "" ] ; then
     if [ "$ONLINE" = "0" ] ; then
 	echo 'You have not explicitly specified the mode to use. Using default "report" mode.'

@@ -82,7 +90,26 @@ if [ "$ONLINE" = "0" ] ; then
     echo ''
 fi
 
-run_cmd() {
+run_cmd_parmap() {
+	if [ $VERBOSE -ne 0 ] ; then
+		echo "Running ($NPROC in parallel): $@"
+	fi
+	if [ "$DEBUG_FILE" != "/dev/null" -a "$DEBUG_FILE" != "" ]; then
+		if [ -f $DEBUG_FILE ]; then
+			echo "Debug file $DEBUG_FILE exists, bailing"
+			exit
+		fi
+	else
+		DEBUG_FILE="/dev/null"
+	fi
+	$@ 2>$DEBUG_FILE
+	if [[ $? -ne 0 ]]; then
+		echo "coccicheck failed"
+		exit $?
+	fi
+}
+
+run_cmd_old() {
 	local i
 	if [ $VERBOSE -ne 0 ] ; then
 		echo "Running ($NPROC in parallel): $@"

@@ -97,6 +124,14 @@ run_cmd() {
 	wait
 }
 
+run_cmd() {
+	if [ "$USE_JOBS" = "yes" ]; then
+		run_cmd_parmap $@
+	else
+		run_cmd_old $@
+	fi
+}
+
 kill_running() {
 	for i in $(seq 0 $(( NPROC - 1 )) ); do
 		if [ $VERBOSE -eq 2 ] ; then

Signature · 1 line

2.8.2

Quoted 12 lines · Luis R. Rodriguez

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

Quoted 4 lines

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Why do you want to do something different for standard error in the parmap and nonparmap case?

Quoted 21 lines · Julia Lawall

Quoted 16 lines · Luis R. Rodriguez

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Quoted 2 lines

Why do you want to do something different for standard error in the parmap and nonparmap case?

We should just deprecate non-parmap later.

Quoted 25 lines · Luis R. Rodriguez

Quoted 23 lines · Julia Lawall

Quoted 8 lines · Luis R. Rodriguez

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Quoted 8 lines

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Why do you want to do something different for standard error in the parmap and nonparmap case?

Quoted 1 line

We should just deprecate non-parmap later.

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

Quoted 31 lines · Julia Lawall

Quoted 26 lines · Luis R. Rodriguez

Quoted 16 lines · Julia Lawall

Quoted 12 lines · Luis R. Rodriguez

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

Quoted 7 lines

Quoted 4 lines

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Why do you want to do something different for standard error in the parmap and nonparmap case?

We should just deprecate non-parmap later.

Quoted 4 lines

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

stderr is not being merged to stdout though. By default stderr goes to /dev/null and if you want it you specify a DEBUG_FILE.

What will be deprecated has no clean solution for any of this and its unclear exactly what happens given separate processes are run in the background and we just wait.

Luis

Quoted 37 lines · Luis R. Rodriguez

Quoted 35 lines · Julia Lawall

Quoted 22 lines · Luis R. Rodriguez

Quoted 21 lines · Julia Lawall

Quoted 16 lines · Luis R. Rodriguez

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Quoted 4 lines

Quoted 2 lines

Why do you want to do something different for standard error in the parmap and nonparmap case?

We should just deprecate non-parmap later.

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

Quoted 2 lines

stderr is not being merged to stdout though. By default stderr goes to /dev/null and if you want it you specify a DEBUG_FILE.

Above it says:

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If DEBUG_FILE is an option for the parmap case, it should be mentioned there too.

Quoted 41 lines · Julia Lawall

Quoted 37 lines · Luis R. Rodriguez

Quoted 35 lines · Julia Lawall

Quoted 22 lines · Luis R. Rodriguez

Quoted 21 lines · Julia Lawall

Quoted 16 lines · Luis R. Rodriguez

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Quoted 4 lines

Quoted 2 lines

Why do you want to do something different for standard error in the parmap and nonparmap case?

We should just deprecate non-parmap later.

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

Quoted 8 lines

Quoted 2 lines

stderr is not being merged to stdout though. By default stderr goes to /dev/null and if you want it you specify a DEBUG_FILE.

Above it says:

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

Ah crap, sorry I left that in the commit log message by mistake.

Quoted 2 lines

If DEBUG_FILE is an option for the parmap case, it should be mentioned there too.

Indeed, its as described instead, stderr goes to /dev/null by default unless DEBUG_FILE is specified, if it is specified then stderr goes to DEBUG_FILE.

Quoted 26 lines · Julia Lawall

Quoted 23 lines · Luis R. Rodriguez

Quoted 9 lines

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Quoted 8 lines

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Why do you want to do something different for standard error in the parmap and nonparmap case?

Quoted 1 line

We should just deprecate non-parmap later.

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

julia

Hi,

Le 21/06/16 ? 22:43, Julia Lawall a ?crit :

Quoted 29 lines · Luis R. Rodriguez

Quoted 26 lines · Julia Lawall

Quoted 23 lines · Luis R. Rodriguez

Quoted 9 lines

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Quoted 8 lines

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Why do you want to do something different for standard error in the parmap and nonparmap case?

Quoted 6 lines

Quoted 1 line

We should just deprecate non-parmap later.

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

I agree. You're not just "enabling parmap support". You're also changing how messages to stderr are handled. Maybe add the DEBUG_FILE mechanism in a separate patch for both modes (parmap and non-parmap).

Quoted 39 lines · Nicolas Palix (LIG)

Hi,

Le 21/06/16 ? 22:43, Julia Lawall a ?crit :

Quoted 35 lines · Luis R. Rodriguez

Quoted 23 lines · Julia Lawall

Quoted 21 lines · Luis R. Rodriguez

Quoted 17 lines

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Quoted 4 lines

Quoted 2 lines

Why do you want to do something different for standard error in the parmap and nonparmap case?

We should just deprecate non-parmap later.

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

Quoted 4 lines

I agree. You're not just "enabling parmap support". You're also changing how messages to stderr are handled. Maybe add the DEBUG_FILE mechanism in a separate patch for both modes (parmap and non-parmap).

I'd prefer to just rip out non-parmap support and bump coccinelle requiremetns to at least 1.0.3, thoughts?

Quoted 45 lines · Luis R. Rodriguez

Quoted 43 lines · Nicolas Palix (LIG)

Hi,

Le 21/06/16 ? 22:43, Julia Lawall a ?crit :

Quoted 29 lines · Luis R. Rodriguez

Quoted 26 lines · Julia Lawall

Quoted 23 lines · Luis R. Rodriguez

Quoted 9 lines

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Quoted 8 lines

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Why do you want to do something different for standard error in the parmap and nonparmap case?

Quoted 6 lines

Quoted 1 line

We should just deprecate non-parmap later.

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

I agree. You're not just "enabling parmap support". You're also changing how messages to stderr are handled. Maybe add the DEBUG_FILE mechanism in a separate patch for both modes (parmap and non-parmap).

Quoted 2 lines

I'd prefer to just rip out non-parmap support and bump coccinelle requiremetns to at least 1.0.3, thoughts?

There are already too many changes in this patch series.

Also, I don't know what the 0-day people would find convenient.

Quoted 49 lines · Julia Lawall

Quoted 45 lines · Luis R. Rodriguez

Quoted 43 lines · Nicolas Palix (LIG)

Hi,

Le 21/06/16 ? 22:43, Julia Lawall a ?crit :

Quoted 29 lines · Luis R. Rodriguez

Quoted 26 lines · Julia Lawall

Quoted 23 lines · Luis R. Rodriguez

Quoted 9 lines

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Quoted 8 lines

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Why do you want to do something different for standard error in the parmap and nonparmap case?

Quoted 6 lines

Quoted 1 line

We should just deprecate non-parmap later.

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

I agree. You're not just "enabling parmap support". You're also changing how messages to stderr are handled. Maybe add the DEBUG_FILE mechanism in a separate patch for both modes (parmap and non-parmap).

Quoted 6 lines

Quoted 2 lines

I'd prefer to just rip out non-parmap support and bump coccinelle requiremetns to at least 1.0.3, thoughts?

There are already too many changes in this patch series.

Also, I don't know what the 0-day people would find convenient.

I'd really prefer to not deal with supporting DEBUG_FILE for non-parmap case due to the way parallelism is supported there, it uses wait(1) to wait on the shell, and for spawning this nasty thing:

eval "$@ --max $NPROC --index $i &"

Specially since we are likely to be able to deprecate this sooner rather than later I see little point in adding DEBUG_FILE into this mess.

Quoted 61 lines · Luis R. Rodriguez

Quoted 49 lines · Julia Lawall

Quoted 45 lines · Luis R. Rodriguez

Quoted 43 lines · Nicolas Palix (LIG)

Hi,

Le 21/06/16 ? 22:43, Julia Lawall a ?crit :

Quoted 29 lines · Luis R. Rodriguez

Quoted 26 lines · Julia Lawall

Quoted 23 lines · Luis R. Rodriguez

Quoted 9 lines

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Quoted 8 lines

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Why do you want to do something different for standard error in the parmap and nonparmap case?

Quoted 6 lines

Quoted 1 line

We should just deprecate non-parmap later.

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

I agree. You're not just "enabling parmap support". You're also changing how messages to stderr are handled. Maybe add the DEBUG_FILE mechanism in a separate patch for both modes (parmap and non-parmap).

Quoted 6 lines

Quoted 2 lines

I'd prefer to just rip out non-parmap support and bump coccinelle requiremetns to at least 1.0.3, thoughts?

There are already too many changes in this patch series.

Also, I don't know what the 0-day people would find convenient.

I'd really prefer to not deal with supporting DEBUG_FILE for non-parmap case due to the way parallelism is supported there, it uses wait(1) to wait on the shell, and for spawning this nasty thing:

Quoted 5 lines

eval "$@ --max $NPROC --index $i &"

Specially since we are likely to be able to deprecate this sooner rather than later I see little point in adding DEBUG_FILE into this mess.

Sorry, I didn't realize there was parallelism without parmap.

My thought
was that if someone is running Coccinelle on only one core, then why force them to use parmap.

Coccinelle could of course be updated to not use
parmap when the number of cores is 1.

Quoted 65 lines · Julia Lawall

Quoted 61 lines · Luis R. Rodriguez

Quoted 49 lines · Julia Lawall

Quoted 45 lines · Luis R. Rodriguez

Quoted 43 lines · Nicolas Palix (LIG)

Hi,

Le 21/06/16 ? 22:43, Julia Lawall a ?crit :

Quoted 29 lines · Luis R. Rodriguez

Quoted 26 lines · Julia Lawall

Quoted 23 lines · Luis R. Rodriguez

Quoted 9 lines

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Quoted 8 lines

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Why do you want to do something different for standard error in the parmap and nonparmap case?

Quoted 6 lines

Quoted 1 line

We should just deprecate non-parmap later.

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

I agree. You're not just "enabling parmap support". You're also changing how messages to stderr are handled. Maybe add the DEBUG_FILE mechanism in a separate patch for both modes (parmap and non-parmap).

Quoted 6 lines

Quoted 2 lines

I'd prefer to just rip out non-parmap support and bump coccinelle requiremetns to at least 1.0.3, thoughts?

There are already too many changes in this patch series.

Also, I don't know what the 0-day people would find convenient.

I'd really prefer to not deal with supporting DEBUG_FILE for non-parmap case due to the way parallelism is supported there, it uses wait(1) to wait on the shell, and for spawning this nasty thing:

Quoted 7 lines

Quoted 5 lines

eval "$@ --max $NPROC --index $i &"

Specially since we are likely to be able to deprecate this sooner rather than later I see little point in adding DEBUG_FILE into this mess.

Sorry, I didn't realize there was parallelism without parmap.

Yea :( so is the change OK as-is then, only I need to update the commit log?

Quoted 3 lines

My thought
was that if someone is running Coccinelle on only one core, then why force them to use parmap.

Oh but that's different feedback. Sure, but why should that be an issue ? It would seem that coccinelle would just do the right thing with -j 1 used.

Quoted 2 lines

Coccinelle could of course be updated to not use
parmap when the number of cores is 1.

:) Single CPU systems are probably odd bests these days, either way I can update the script to avoid parmap if number of cpus is 1 since I'm respinning.

Quoted 80 lines · Luis R. Rodriguez

Quoted 65 lines · Julia Lawall

Quoted 61 lines · Luis R. Rodriguez

Quoted 49 lines · Julia Lawall

Quoted 45 lines · Luis R. Rodriguez

Quoted 43 lines · Nicolas Palix (LIG)

Hi,

Le 21/06/16 ? 22:43, Julia Lawall a ?crit :

Quoted 29 lines · Luis R. Rodriguez

Quoted 26 lines · Julia Lawall

Quoted 23 lines · Luis R. Rodriguez

Quoted 9 lines

Coccinelle has had parmap support since 1.0.2, this means it supports --jobs, enabling built-in multithreaded functionality, instead of needing one to script it out. Just look for --jobs in the help output to determine if this is supported.

Also enable the load balancing to be dynamic, so that if a thread finishes early we keep feeding it.

Quoted 8 lines

Note: now that we have all things handled for us, redirect stderr to stdout as well to capture any possible errors or warnings issued by coccinelle.

If --jobs is not supported we fallback to the old mechanism. This also now accepts DEBUG_FILE= to specify where you want stderr to be redirected to, by default we redirect stderr to /dev/null.

Why do you want to do something different for standard error in the parmap and nonparmap case?

Quoted 6 lines

Quoted 1 line

We should just deprecate non-parmap later.

that's not really getting at the point. I like the DEBUG_FILE= solution. I don't like merging stderr and stdout. So you've put what to my mind is the good solution only in the deprecated case (to my understanding of the commit message).

I agree. You're not just "enabling parmap support". You're also changing how messages to stderr are handled. Maybe add the DEBUG_FILE mechanism in a separate patch for both modes (parmap and non-parmap).

Quoted 6 lines

Quoted 2 lines

I'd prefer to just rip out non-parmap support and bump coccinelle requiremetns to at least 1.0.3, thoughts?

There are already too many changes in this patch series.

Also, I don't know what the 0-day people would find convenient.

I'd really prefer to not deal with supporting DEBUG_FILE for non-parmap case due to the way parallelism is supported there, it uses wait(1) to wait on the shell, and for spawning this nasty thing:

Quoted 7 lines

Quoted 5 lines

eval "$@ --max $NPROC --index $i &"

Specially since we are likely to be able to deprecate this sooner rather than later I see little point in adding DEBUG_FILE into this mess.

Sorry, I didn't realize there was parallelism without parmap.

Yea :( so is the change OK as-is then, only I need to update the commit log?

Quoted 3 lines

My thought
was that if someone is running Coccinelle on only one core, then why force them to use parmap.

Quoted 8 lines

Oh but that's different feedback. Sure, but why should that be an issue ? It would seem that coccinelle would just do the right thing with -j 1 used.

Quoted 2 lines

Coccinelle could of course be updated to not use
parmap when the number of cores is 1.

:) Single CPU systems are probably odd bests these days, either way I can update the script to avoid parmap if number of cpus is 1 since I'm respinning.

Some semantic patches have to be run single core, eg due to the use of finalize. Perhaps there would be some reason to run them single core, if one had the same nmber of semantic patches as cores. This was more relevant before dynamic load balancing though. Single core is also better when using an option that takes a lot of include files and when using --include-headers-for-types. Then one has maximal sharing of include file information across the treatment of the different C files. In contrast, chunksize 1 is worst. In that case, there is no effective caching of parsed header files, because Coccinelle has no shared memory.

Actually, it would be probably good to raise the default chunksize a bit for the latter reason. It would depend on which files get assigned to which chunks though how much benefit it might have.

Quoted 3 lines

Actually, it would be probably good to raise the default chunksize a bit for the latter reason. It would depend on which files get assigned to which chunks though how much benefit it might have.

What value do you have in mind? Or should we leave this as a separate future change?

Help Coccinelle when used against Linux with a set of sensible defaults options for Linux. This hints to coccinelle git can be used for 'git grep' queries over coccigrep. A timeout of 200 seconds should suffice for now.

If you use idutils you can override for 'make coccicheck' by using the SPFLAGS option as follows:

First build the index, for example:
mkid -s

Pick the cocci file you wnat to work with:
export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci

Then run coccicheck:

$ make coccicheck V=1 MODE=report SPFLAGS="--use-idutils ID"

Coccinelle supports reading .cocciconfig from different directories, the later one overriding the previous reads in the following order:

o Your current user's home directory is processed first
o Your directory from which spatch is called is processed next
o The directory provided with the --dir option is processed last, if used

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 .cocciconfig | 3 +++
 .gitignore   | 1 +
 2 files changed, 4 insertions(+)
 create mode 100644 .cocciconfig

.cocciconfig new file +3 -0

new file mode 100644
index 000000000000..43967c6b2015
@@ -0,0 +1,3 @@
+[spatch]
+	options = --timeout 200
+	options = --use-gitgrep

.gitignore +1 -0

index 2be25f771bd8..c2ed4ecb0acd 100644
@@ -67,6 +67,7 @@ Module.symvers
 #
 !.gitignore
 !.mailmap
+!.cocciconfig
 
 #
 # Generated include files

Signature · 1 line

2.8.2

mkid -i C --output .id-utils.index

[...] Coccinelle searches for the index in the directory on which it is working

On Tue, Jun 21, 2016 at 10:29:53PM +0200, Julia Lawall wrote:
[...]

Quoted 4 lines

mkid -i C --output .id-utils.index

[...] Coccinelle searches for the index in the directory on which it is working

Can you clarify if this is $PWD from which we spawn spatch or the --dir, or the current directory that spatch is working on at the moment.

Help Coccinelle when used against Linux with a set of sensible defaults options for Linux. This hints to coccinelle git can be used for 'git grep' queries over coccigrep. A timeout of 200 seconds should suffice for now.

If you use idutils you can override for 'make coccicheck' by using the SPFLAGS option as follows:

First build the index, for example:
mkid -s

Pick the cocci file you wnat to work with:
export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci

Then run coccicheck:

$ make coccicheck V=1 MODE=report SPFLAGS="--use-idutils ID"

Coccinelle supports reading .cocciconfig from different directories, the later one overriding the previous reads in the following order:

o Your current user's home directory is processed first
o Your directory from which spatch is called is processed next
o The directory provided with the --dir option is processed last, if used

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 .cocciconfig | 3 +++
 .gitignore   | 1 +
 2 files changed, 4 insertions(+)
 create mode 100644 .cocciconfig

.cocciconfig new file +3 -0

new file mode 100644
index 000000000000..43967c6b2015
@@ -0,0 +1,3 @@
+[spatch]
+	options = --timeout 200
+	options = --use-gitgrep

.gitignore +1 -0

index 2be25f771bd8..c2ed4ecb0acd 100644
@@ -67,6 +67,7 @@ Module.symvers
 #
 !.gitignore
 !.mailmap
+!.cocciconfig
 
 #
 # Generated include files

Le 21/06/16 ? 21:21, Luis R. Rodriguez a ?crit :

Quoted 19 lines

Help Coccinelle when used against Linux with a set of sensible defaults options for Linux. This hints to coccinelle git can be used for 'git grep' queries over coccigrep. A timeout of 200 seconds should suffice for now.

If you use idutils you can override for 'make coccicheck' by using the SPFLAGS option as follows:

First build the index, for example:
mkid -s

Pick the cocci file you wnat to work with:
export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci

Then run coccicheck:

$ make coccicheck V=1 MODE=report SPFLAGS="--use-idutils ID"

Coccinelle supports reading .cocciconfig from different directories, the later one overriding the previous reads in the following order:

Quoted 5 lines

o Your current user's home directory is processed first
o Your directory from which spatch is called is processed next
o The directory provided with the --dir option is processed last, if used

Signed-off-by Luis R. Rodriguez <[email protected]>

Acked-by Nicolas Palix <[email protected]>

Quoted 29 lines

---
 .cocciconfig | 3 +++
 .gitignore   | 1 +
 2 files changed, 4 insertions(+)
 create mode 100644 .cocciconfig

.cocciconfig new file +3 -0

new file mode 100644
index 000000000000..43967c6b2015
@@ -0,0 +1,3 @@
+[spatch]
+	options = --timeout 200
+	options = --use-gitgrep

.gitignore +1 -0

index 2be25f771bd8..c2ed4ecb0acd 100644
@@ -67,6 +67,7 @@ Module.symvers
 #
 !.gitignore
 !.mailmap
+!.cocciconfig
 
 #
 # Generated include files

Signature · 2 lines

Nicolas Palix
http://lig-membres.imag.fr/palix/

Quoted 24 lines · Nicolas Palix (LIG)

Le 21/06/16 ? 21:21, Luis R. Rodriguez a ?crit :

Quoted 23 lines

Help Coccinelle when used against Linux with a set of sensible defaults options for Linux. This hints to coccinelle git can be used for 'git grep' queries over coccigrep. A timeout of 200 seconds should suffice for now.

If you use idutils you can override for 'make coccicheck' by using the SPFLAGS option as follows:

First build the index, for example:
mkid -s

Pick the cocci file you wnat to work with:
export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci

Then run coccicheck:

$ make coccicheck V=1 MODE=report SPFLAGS="--use-idutils ID"

Coccinelle supports reading .cocciconfig from different directories, the later one overriding the previous reads in the following order:

o Your current user's home directory is processed first
o Your directory from which spatch is called is processed next
o The directory provided with the --dir option is processed last, if used

Quoted 2 lines

Quoted 1 line

Signed-off-by Luis R. Rodriguez <[email protected]>

Acked-by Nicolas Palix <[email protected]>

Hmm, I can see at least some advantages to encouraging people to do it the Coccinelle way, with the Coccinelle script rather than mkid directly. Then, if we need some other features specific to Coccinelle, we can just add them.

Quoted 26 lines · Julia Lawall

Quoted 24 lines · Nicolas Palix (LIG)

Le 21/06/16 ? 21:21, Luis R. Rodriguez a ?crit :

Quoted 23 lines

Help Coccinelle when used against Linux with a set of sensible defaults options for Linux. This hints to coccinelle git can be used for 'git grep' queries over coccigrep. A timeout of 200 seconds should suffice for now.

If you use idutils you can override for 'make coccicheck' by using the SPFLAGS option as follows:

First build the index, for example:
mkid -s

Pick the cocci file you wnat to work with:
export COCCI=scripts/coccinelle/misc/irqf_oneshot.cocci

Then run coccicheck:

$ make coccicheck V=1 MODE=report SPFLAGS="--use-idutils ID"

Coccinelle supports reading .cocciconfig from different directories, the later one overriding the previous reads in the following order:

o Your current user's home directory is processed first
o Your directory from which spatch is called is processed next
o The directory provided with the --dir option is processed last, if used

Quoted 7 lines

Quoted 2 lines

Quoted 1 line

Signed-off-by Luis R. Rodriguez <[email protected]>

Acked-by Nicolas Palix <[email protected]>

Hmm, I can see at least some advantages to encouraging people to do it the Coccinelle way, with the Coccinelle script rather than mkid directly. Then, if we need some other features specific to Coccinelle, we can just add them.

I can simply document that if users are used to using their own target output file, and if they wanted it to be used by coccinelle simply symlinking .id-utils.index to it would enable coccinelle to pick it up by default.

If that is done, would the new .cocciconfig not override though?

Enable Coccinelle SmPL patches to require a specific version of Coccinelle. In the event that the version does not match we just inform the user, if the user asked to go through all SmPL patches we just inform them of the need for a new version of coccinelle for the SmPL patch and continue on with the rest.

This uses the simple kernel scripts/ld-version.sh to create a weight on the version provided by spatch. The -dirty attribute is ignored if supplied, the benefit of scripts/ld-version.sh is it has a long history and well tested.

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 scripts/coccicheck | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

scripts/coccicheck +14 -0

index 998d764636e0..01b6716ea931 100755
@@ -5,6 +5,7 @@
 # version 1.0.0-rc11.
 #
 
+DIR="$(dirname $(readlink -f $0))/.."
 SPATCH="`which ${SPATCH:=spatch}`"
 
 if [ ! -x "$SPATCH" ]; then

@@ -12,6 +13,9 @@ if [ ! -x "$SPATCH" ]; then
     exit 1
 fi
 
+SPATCH_VERSION=$($SPATCH --version | head -1 | awk '{print $3}')
+SPATCH_VERSION_NUM=$(echo $SPATCH_VERSION | ${DIR}/scripts/ld-version.sh)
+
 USE_JOBS="no"
 $SPATCH --help | grep "\-\-jobs" > /dev/null && USE_JOBS="yes"
 

@@ -171,6 +175,16 @@ coccinelle () {
     COCCI="$1"
 
     OPT=`grep "Option" $COCCI | cut -d':' -f2`
+    REQ=`grep "Requires" $COCCI | cut -d':' -f2 | sed "s| ||"`
+    REQ_NUM=$(echo $REQ | ${DIR}/scripts/ld-version.sh)
+    if [ "$REQ_NUM" != "0" ] ; then
+	    if [ "$SPATCH_VERSION_NUM" -lt "$REQ_NUM" ] ; then
+		    echo "Skipping coccinele SmPL patch: $COCCI"
+		    echo "You have coccinelle:           $SPATCH_VERSION"
+		    echo "This SmPL patch requires:      $REQ"
+		    return
+	    fi
+    fi
 
 #   The option '--parse-cocci' can be used to syntactically check the SmPL files.
 #

Sprinkling *tons* of documentation on the script is not a good idea, instead refer to a wiki for further coccicheck documentation:

https://bottest.wiki.kernel.org/coccicheck

This page shall always refer to the linux-next iteration of scripts/coccicheck.

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 scripts/coccicheck | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

scripts/coccicheck +7 -2

index 01b6716ea931..adbcbbd1aad6 100755
@@ -1,9 +1,14 @@
 #!/bin/bash
-
+# Linux kernel coccicheck
+#
+# For more detailed documentation refer to:
+#
+# https://bottest.wiki.kernel.org/coccicheck
+#
+# This documentation always refers to the linux-next version of the script.
 #
 # This script requires at least spatch
 # version 1.0.0-rc11.
-#
 
 DIR="$(dirname $(readlink -f $0))/.."
 SPATCH="`which ${SPATCH:=spatch}`"

Hi,

Le 21/06/16 ? 21:21, Luis R. Rodriguez a ?crit :

Quoted 3 lines

Sprinkling *tons* of documentation on the script is not a good idea, instead refer to a wiki for further coccicheck documentation:

Quoted 4 lines

https://bottest.wiki.kernel.org/coccicheck

This page shall always refer to the linux-next iteration of scripts/coccicheck.

Can you say a word about Documentation/coccinelle.txt too ? And update it according to the script changes.

Quoted 29 lines

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 scripts/coccicheck | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

scripts/coccicheck +7 -2

index 01b6716ea931..adbcbbd1aad6 100755
@@ -1,9 +1,14 @@
 #!/bin/bash
-
+# Linux kernel coccicheck
+#
+# For more detailed documentation refer to:
+#
+# https://bottest.wiki.kernel.org/coccicheck
+#
+# This documentation always refers to the linux-next version of the script.
 #
 # This script requires at least spatch
 # version 1.0.0-rc11.
-#
 
 DIR="$(dirname $(readlink -f $0))/.."
 SPATCH="`which ${SPATCH:=spatch}`"

Signature · 2 lines

Nicolas Palix
http://lig-membres.imag.fr/palix/

Quoted 8 lines · Nicolas Palix (LIG)

Hi,

Le 21/06/16 ? 21:21, Luis R. Rodriguez a ?crit :

Quoted 5 lines

Sprinkling *tons* of documentation on the script is not a good idea, instead refer to a wiki for further coccicheck documentation:

https://bottest.wiki.kernel.org/coccicheck

Quoted 5 lines

Quoted 2 lines

This page shall always refer to the linux-next iteration of scripts/coccicheck.

Can you say a word about Documentation/coccinelle.txt too ? And update it according to the script changes.

I think this would be a good idea.

Quoted 18 lines · Julia Lawall

Quoted 13 lines · Nicolas Palix (LIG)

Hi,

Le 21/06/16 ? 21:21, Luis R. Rodriguez a ?crit :

Quoted 7 lines

Sprinkling *tons* of documentation on the script is not a good idea, instead refer to a wiki for further coccicheck documentation:

https://bottest.wiki.kernel.org/coccicheck

This page shall always refer to the linux-next iteration of scripts/coccicheck.

Can you say a word about Documentation/coccinelle.txt too ? And update it according to the script changes.

Quoted 1 line

I think this would be a good idea.

I'd much prefer to just keep docs in a wiki, and we nuke this outdated doc or refer in the Documentation/coccinelle.txt to the wiki. This lets us document things much faster and lets us add more information in a much nicer format. But that's just me...

The existing file Documentation/coccinelle.txt even provides documentation on how to build coccinelle --we're way passed that point. Note, I havne't updated the docs on the wiki to refer to any of this, and would not unless these changes get merged at least to linux-next.

Luis

Make use of the new Requires: tag to be able to specify coccinelle binary version requirements. The cocci file device_node_continue.cocci requires at least coccinelle 1.0.4.

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 scripts/coccinelle/iterators/device_node_continue.cocci | 3 +++
 1 file changed, 3 insertions(+)

scripts/coccinelle/iterators/device_node_continue.cocci +3 -0

index 38ab744a4037..a36c16db171b 100644
@@ -5,8 +5,11 @@
 // Copyright: (C) 2015 Julia Lawall, Inria. GPLv2.
 // URL: http://coccinelle.lip6.fr/
 // Options: --no-includes --include-headers
+// Requires: 1.0.4
 // Keywords: for_each_child_of_node, etc.
 
+// This uses a conjunction, which requires at least coccinelle >= 1.0.4
+
 virtual patch
 virtual context
 virtual org

Signature · 1 line

2.8.2

Quoted 6 lines

Make use of the new Requires: tag to be able to specify coccinelle binary version requirements. The cocci file device_node_continue.cocci requires at least coccinelle 1.0.4.

Signed-off-by Luis R. Rodriguez <[email protected]>

Acked-by Julia Lawall <[email protected]>

Quoted 25 lines

---
 scripts/coccinelle/iterators/device_node_continue.cocci | 3 +++
 1 file changed, 3 insertions(+)

scripts/coccinelle/iterators/device_node_continue.cocci +3 -1

index 38ab744a4037..a36c16db171b 100644
@@ -5,8 +5,11 @@
 // Copyright: (C) 2015 Julia Lawall, Inria. GPLv2.
 // URL: http://coccinelle.lip6.fr/
 // Options: --no-includes --include-headers
+// Requires: 1.0.4
 // Keywords: for_each_child_of_node, etc.
 
+// This uses a conjunction, which requires at least coccinelle >= 1.0.4
+
 virtual patch
 virtual context
 virtual org
--

2.8.2

This v3 series addresses the feedback from the last v2 series on the coccicheck enhancements [0], namely:

o it drops the indexing heuristics in favor for a .cocciconfig use
o drops glimpse support as its simply not well maintained, recommends idutils instead.
o adds a Linux .cocciconfig -- the assumption is you'd run spatch when you're at the top level of the kernel. This has not only the side effect of picking up .cocciconfig, but also that the coccicheck use of the make variables passed on are assumed to be correct given the base directory as the current directory.

o makes SPFLAGS more useful by ensuring it is always at the end of options
o rebases on top of Deepa Dinamani's patch
"coccicheck: Allow for overriding spatch flags", under the assumption this is already merged. I don't see it on linux-next but I do think Michal has merged already onto his tree. This is also rebased on to linux-next next-20160621

These changes are also visible on kernel.org, on a branch based on linux-next next-20160621 with Deepa's commit merged first.

[0] http://lkml.kernel.org/r/1466116292-21843-1-git-send-email-mcgrof at kernel.org
[1] https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20160621-cocciconfig-v3

Luis R. Rodriguez (8):
coccicheck: move spatch binary check up
coccicheck: make SPFLAGS more useful
coccicheck: enable parmap support
coccicheck: replace --very-quiet with --quit when debugging
scripts: add Linux .cocciconfig for coccinelle
coccicheck: add support for requring a coccinelle version
coccicheck: refer to coccicheck bottest wiki for documentation scripts/coccinelle: require coccinelle >= 1.0.4 on
device_node_continue.cocci

 .cocciconfig                                       |  3 +
 .gitignore                                         |  1 +
 scripts/coccicheck                                 | 98 ++++++++++++++++++++--
 .../iterators/device_node_continue.cocci           |  3 +
 4 files changed, 96 insertions(+), 9 deletions(-)
 create mode 100644 .cocciconfig

Signature · 1 line

2.8.2

Quoted 11 lines · Luis R. Rodriguez

This v3 series addresses the feedback from the last v2 series on the coccicheck enhancements [0], namely:

o it drops the indexing heuristics in favor for a .cocciconfig use
o drops glimpse support as its simply not well maintained, recommends idutils instead.
o adds a Linux .cocciconfig -- the assumption is you'd run spatch when you're at the top level of the kernel. This has not only the side effect of picking up .cocciconfig, but also that the coccicheck use of the make variables passed on are assumed to be correct given the base directory as the current directory.

I don't understand this point. Coccinelle picks up the .cocciconfig, if any, of the directory on which you want to work, not of the current one.

Quoted 16 lines · Julia Lawall

Quoted 11 lines · Luis R. Rodriguez

This v3 series addresses the feedback from the last v2 series on the coccicheck enhancements [0], namely:

o it drops the indexing heuristics in favor for a .cocciconfig use
o drops glimpse support as its simply not well maintained, recommends idutils instead.
o adds a Linux .cocciconfig -- the assumption is you'd run spatch when you're at the top level of the kernel. This has not only the side effect of picking up .cocciconfig, but also that the coccicheck use of the make variables passed on are assumed to be correct given the base directory as the current directory.

Quoted 2 lines

I don't understand this point. Coccinelle picks up the .cocciconfig, if any, of the directory on which you want to work, not of the current one.

The order of precedence for variables for .coccoconfig is as follows:

o Your current user's home directory is processed first
o Your directory from which spatch is called is processed next
o The directory provided with the --dir option is processed last, if used

Since coccicheck runs through make, it naturally runs from the kernel proper dir, as such the second rule above would be implied for picking up a .cocciconfig. That's part of the point I'm making.

Up next let us consider when M= is used or when it is not used, if used it populates KBUILD_EXTMOD.

if [ "$KBUILD_EXTMOD" = "" ] ; then
OPTIONS="--dir $srctree $COCCIINCLUDE"
else
OPTIONS="--dir $KBUILD_EXTMOD $COCCIINCLUDE"
fi

Either way --dir is used, so the third rule applies and so your .cocciconfig from there is also read if one is found. My other point was that $COCCIINCLUDE has some useful tidbits of includes for coccinelle, and that also assumes one is on the top level dir of the kernel.

That is sanitized as follows:

# spatch only allows include directories with the syntax "-I include"           
# while gcc also allows "-Iinclude" and "-include include"                      
COCCIINCLUDE=${LINUXINCLUDE//-I/-I }                                            
COCCIINCLUDE=${COCCIINCLUDE// -include/ --include} 

So the point is to annotate that the .cocconfig is picked up first due to the fact make is used and its issued from the top level makefile and starts from the top level. The fact that --dir is used is important but secondary to its introduction as well.

Quoted 6 lines · Luis R. Rodriguez

That is sanitized as follows:

# spatch only allows include directories with the syntax "-I include"           
# while gcc also allows "-Iinclude" and "-include include"                      
COCCIINCLUDE=${LINUXINCLUDE//-I/-I }                                            
COCCIINCLUDE=${COCCIINCLUDE// -include/ --include} 

I don't get the second case. Is it to replace -include by --include? Coccinelle actually supports both, although it doesn't advertise that.

Also, in LINUXINCLUDE, what is the meaning of -include? For Coccinelle, it is not the same as -I. It is for files that should be included that are not in the set of includes seen by whatever is the specified include strategy (--all-includes, etc). The argument is a specific file name, not a directory. It is a way of eg not bothering with --recursive-includes when there is one or a few key header files that each file will need.

Quoted 4 lines

So the point is to annotate that the .cocconfig is picked up first due to the fact make is used and its issued from the top level makefile and starts from the top level. The fact that --dir is used is important but secondary to its introduction as well.

OK, the original text seemed to me to imply that running from the kernel directory was essential to getting the kernels .cocciconfig,

so I wanted to point out that this is not the case.

Quoted 8 lines · Julia Lawall

Quoted 6 lines · Luis R. Rodriguez

That is sanitized as follows:

# spatch only allows include directories with the syntax "-I include"           
# while gcc also allows "-Iinclude" and "-include include"                      
COCCIINCLUDE=${LINUXINCLUDE//-I/-I }                                            
COCCIINCLUDE=${COCCIINCLUDE// -include/ --include} 

Quoted 2 lines

I don't get the second case. Is it to replace -include by --include? Coccinelle actually supports both, although it doesn't advertise that.

Oh neat, yeah. So a follow up patch later can be to remove that second line? If so as of what version of coccinelle?

Quoted 6 lines

Also, in LINUXINCLUDE, what is the meaning of -include? For Coccinelle, it is not the same as -I. It is for files that should be included that are not in the set of includes seen by whatever is the specified include strategy (--all-includes, etc). The argument is a specific file name, not a directory. It is a way of eg not bothering with --recursive-includes when there is one or a few key header files that each file will need.

Its used to force to include a single file, it is a file.

Quoted 7 lines

Quoted 4 lines

So the point is to annotate that the .cocconfig is picked up first due to the fact make is used and its issued from the top level makefile and starts from the top level. The fact that --dir is used is important but secondary to its introduction as well.

OK, the original text seemed to me to imply that running from the kernel directory was essential to getting the kernels .cocciconfig,

And what I meant to imply was that since coccicheck uses the kernel makefiles it would kick off from kernel proper.

Quoted 1 line

so I wanted to point out that this is not the case.

I should have elaborated with all these details, its perhaps best to be explicit about this so I can respin with a clearer commit log.

Quoted 9 lines · Luis R. Rodriguez

Quoted 8 lines · Julia Lawall

Quoted 6 lines · Luis R. Rodriguez

That is sanitized as follows:

# spatch only allows include directories with the syntax "-I include"           
# while gcc also allows "-Iinclude" and "-include include"                      
COCCIINCLUDE=${LINUXINCLUDE//-I/-I }                                            
COCCIINCLUDE=${COCCIINCLUDE// -include/ --include} 

Quoted 5 lines

Quoted 2 lines

I don't get the second case. Is it to replace -include by --include? Coccinelle actually supports both, although it doesn't advertise that.

Oh neat, yeah. So a follow up patch later can be to remove that second line? If so as of what version of coccinelle?

Forever. Single - has always been supported. Double - was added at some point.

Quoted 8 lines

Quoted 6 lines

Also, in LINUXINCLUDE, what is the meaning of -include? For Coccinelle, it is not the same as -I. It is for files that should be included that are not in the set of includes seen by whatever is the specified include strategy (--all-includes, etc). The argument is a specific file name, not a directory. It is a way of eg not bothering with --recursive-includes when there is one or a few key header files that each file will need.

Its used to force to include a single file, it is a file.

OK, close enough then.

Quoted 11 lines

Quoted 7 lines

Quoted 4 lines

So the point is to annotate that the .cocconfig is picked up first due to the fact make is used and its issued from the top level makefile and starts from the top level. The fact that --dir is used is important but secondary to its introduction as well.

OK, the original text seemed to me to imply that running from the kernel directory was essential to getting the kernels .cocciconfig,

And what I meant to imply was that since coccicheck uses the kernel makefiles it would kick off from kernel proper.

Quoted 4 lines

Quoted 1 line

so I wanted to point out that this is not the case.

I should have elaborated with all these details, its perhaps best to be explicit about this so I can respin with a clearer commit log.

Thanks. People may come across this message, and it could be good for it to be as helpful as possible.