[PATCH 0/2] coccicheck bugfix and CI improvement

[PATCH 0/2] coccicheck bugfix and CI improvement

[Anton Eliasson]

Patch 1 is a plain bugfix. Patch 2 allows us running coccicheck on an
external kernel module in a Makefile target like this:

    MODULE_SRC_DIR := $(shell pwd)/src
    coccicheck:
        $(MAKE) --no-print-directory -C $(KERNEL_SRC) M=$(MODULE_SRC_DIR) coccicheck MODE=report
        $(MAKE) --no-print-directory -C $(KERNEL_SRC) M=$(MODULE_SRC_DIR) coccicheck MODE=report COCCI=$(MODULE_SRC_DIR)/../coccinelle/custom-spatch.cocci

The output from this target is fairly compact. After filtering out the
"Please check for false positives..." message and sorting the remaining
lines it can be diffed with/without a proposed patch in a continuous
integration system in order to highlight newly introduced spatch
warnings.

Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
---
Anton Eliasson (2):
      scripts: coccicheck: Return error from run_cmd_parmap
      scripts: coccicheck: Separate spatch stdout and stderr

 scripts/coccicheck | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
---
base-commit: 831fe284d8275987596b7d640518dddba5735f61
change-id: 20231003-coccicheck-64b270430709

Best regards,
-- 
Anton Eliasson <anton.eliasson@axis.com>

[cocci] [PATCH 0/2] coccicheck bugfix and CI improvement

[Anton Eliasson]

Patch 1 is a plain bugfix. Patch 2 allows us running coccicheck on an
external kernel module in a Makefile target like this:

    MODULE_SRC_DIR := $(shell pwd)/src
    coccicheck:
        $(MAKE) --no-print-directory -C $(KERNEL_SRC) M=$(MODULE_SRC_DIR) coccicheck MODE=report
        $(MAKE) --no-print-directory -C $(KERNEL_SRC) M=$(MODULE_SRC_DIR) coccicheck MODE=report COCCI=$(MODULE_SRC_DIR)/../coccinelle/custom-spatch.cocci

The output from this target is fairly compact. After filtering out the
"Please check for false positives..." message and sorting the remaining
lines it can be diffed with/without a proposed patch in a continuous
integration system in order to highlight newly introduced spatch
warnings.

Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
---
Anton Eliasson (2):
      scripts: coccicheck: Return error from run_cmd_parmap
      scripts: coccicheck: Separate spatch stdout and stderr

 scripts/coccicheck | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
---
base-commit: 831fe284d8275987596b7d640518dddba5735f61
change-id: 20231003-coccicheck-64b270430709

Best regards,
-- 
Anton Eliasson <anton.eliasson@axis.com>

[PATCH 1/2] scripts: coccicheck: Return error from run_cmd_parmap

[Anton Eliasson]

Exiting on error breaks the chain mode. Return the error instead in
order for the caller to propagate it or in the case of chain, try the
next mode.

Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
---
 scripts/coccicheck | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e52cb43fede6..95a312730e98 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -153,7 +153,7 @@ run_cmd_parmap() {
 	err=$?
 	if [[ $err -ne 0 ]]; then
 		echo "coccicheck failed"
-		exit $err
+		return $err
 	fi
 }
 

-- 
2.30.2

[cocci] [PATCH 1/2] scripts: coccicheck: Return error from run_cmd_parmap

[Anton Eliasson]

Exiting on error breaks the chain mode. Return the error instead in
order for the caller to propagate it or in the case of chain, try the
next mode.

Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
---
 scripts/coccicheck | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index e52cb43fede6..95a312730e98 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -153,7 +153,7 @@ run_cmd_parmap() {
 	err=$?
 	if [[ $err -ne 0 ]]; then
 		echo "coccicheck failed"
-		exit $err
+		return $err
 	fi
 }
 

-- 
2.30.2

[PATCH 2/2] scripts: coccicheck: Separate spatch stdout and stderr

[Anton Eliasson]

This helps automating coccicheck runs by discarding stderr and only
looking at the output of stdout. In report mode the only remaining
output on stdout is the initial "Please check for false positives"
message followed by each spatch warning found.

Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
---
 scripts/coccicheck | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index 95a312730e98..7e7c44125f47 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -146,8 +146,8 @@ run_cmd_parmap() {
                 echo $@>>$DEBUG_FILE
                 $@ 2>>$DEBUG_FILE
         else
-                echo $@
-                $@ 2>&1
+                echo $@ >&2
+                $@
 	fi
 
 	err=$?

-- 
2.30.2

[cocci] [PATCH 2/2] scripts: coccicheck: Separate spatch stdout and stderr

[Anton Eliasson]

This helps automating coccicheck runs by discarding stderr and only
looking at the output of stdout. In report mode the only remaining
output on stdout is the initial "Please check for false positives"
message followed by each spatch warning found.

Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
---
 scripts/coccicheck | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/coccicheck b/scripts/coccicheck
index 95a312730e98..7e7c44125f47 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -146,8 +146,8 @@ run_cmd_parmap() {
                 echo $@>>$DEBUG_FILE
                 $@ 2>>$DEBUG_FILE
         else
-                echo $@
-                $@ 2>&1
+                echo $@ >&2
+                $@
 	fi
 
 	err=$?

-- 
2.30.2

Re: [cocci] [PATCH 2/2] scripts: coccicheck: Separate spatch stdout and stderr

[Julia Lawall]

On Tue, 3 Oct 2023, Anton Eliasson wrote:

> This helps automating coccicheck runs by discarding stderr and only
> looking at the output of stdout. In report mode the only remaining
> output on stdout is the initial "Please check for false positives"
> message followed by each spatch warning found.

What is getting dropped is the spatch command lines indicating the
semantic patch.  Is this desirable?

julia

>
> Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
> ---
>  scripts/coccicheck | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/coccicheck b/scripts/coccicheck
> index 95a312730e98..7e7c44125f47 100755
> --- a/scripts/coccicheck
> +++ b/scripts/coccicheck
> @@ -146,8 +146,8 @@ run_cmd_parmap() {
>                  echo $@>>$DEBUG_FILE
>                  $@ 2>>$DEBUG_FILE
>          else
> -                echo $@
> -                $@ 2>&1
> +                echo $@ >&2
> +                $@
>  	fi
>
>  	err=$?
>
> --
> 2.30.2
>
>

Re: [cocci] [PATCH 2/2] scripts: coccicheck: Separate spatch stdout and stderr

[Anton Eliasson]

On 07/10/2023 21.41, Julia Lawall wrote:
>
> On Tue, 3 Oct 2023, Anton Eliasson wrote:
>
>> This helps automating coccicheck runs by discarding stderr and only
>> looking at the output of stdout. In report mode the only remaining
>> output on stdout is the initial "Please check for false positives"
>> message followed by each spatch warning found.
> What is getting dropped is the spatch command lines indicating the
> semantic patch.  Is this desirable?
>
> julia
It's not ideal but it's the best compromise that I have found. The 
problem I'm trying to solve is to be able to diff the output of two 
coccicheck runs and notify the developer if any new warnings were 
introduced. That requires the output to be stable. spatch is always 
invoked for each cocci file in the same order. However, the output from 
each spatch invocation is not stable as it examines each source file in 
an arbitrary order.

My workaround is to sort the output before diffing. The line-by-line 
sorted output only makes sense if the input is one line per warning 
found and that is why I try to discard all output except the single line 
per spatch warning. While the terse output doesn't tell which semantic 
patch file generated the warning, it does usually contain the offending 
file, line number and a summary of the issue.


Anton
>
>> Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
>> ---
>>   scripts/coccicheck | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/coccicheck b/scripts/coccicheck
>> index 95a312730e98..7e7c44125f47 100755
>> --- a/scripts/coccicheck
>> +++ b/scripts/coccicheck
>> @@ -146,8 +146,8 @@ run_cmd_parmap() {
>>                   echo $@>>$DEBUG_FILE
>>                   $@ 2>>$DEBUG_FILE
>>           else
>> -                echo $@
>> -                $@ 2>&1
>> +                echo $@ >&2
>> +                $@
>>   	fi
>>
>>   	err=$?
>>
>> --
>> 2.30.2
>>
>>

Re: [cocci] [PATCH 2/2] scripts: coccicheck: Separate spatch stdout and stderr

[Julia Lawall]

On Tue, 10 Oct 2023, Anton Eliasson wrote:

> On 07/10/2023 21.41, Julia Lawall wrote:
> >
> > On Tue, 3 Oct 2023, Anton Eliasson wrote:
> >
> > > This helps automating coccicheck runs by discarding stderr and only
> > > looking at the output of stdout. In report mode the only remaining
> > > output on stdout is the initial "Please check for false positives"
> > > message followed by each spatch warning found.
> > What is getting dropped is the spatch command lines indicating the
> > semantic patch.  Is this desirable?
> >
> > julia
> It's not ideal but it's the best compromise that I have found. The problem I'm
> trying to solve is to be able to diff the output of two coccicheck runs and
> notify the developer if any new warnings were introduced. That requires the
> output to be stable. spatch is always invoked for each cocci file in the same
> order. However, the output from each spatch invocation is not stable as it
> examines each source file in an arbitrary order.
>
> My workaround is to sort the output before diffing. The line-by-line sorted
> output only makes sense if the input is one line per warning found and that is
> why I try to discard all output except the single line per spatch warning.
> While the terse output doesn't tell which semantic patch file generated the
> warning, it does usually contain the offending file, line number and a summary
> of the issue.

Why does the command line pose a problem for sorting?

julia

>
>
> Anton
> >
> > > Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
> > > ---
> > >   scripts/coccicheck | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/scripts/coccicheck b/scripts/coccicheck
> > > index 95a312730e98..7e7c44125f47 100755
> > > --- a/scripts/coccicheck
> > > +++ b/scripts/coccicheck
> > > @@ -146,8 +146,8 @@ run_cmd_parmap() {
> > >                   echo $@>>$DEBUG_FILE
> > >                   $@ 2>>$DEBUG_FILE
> > >           else
> > > -                echo $@
> > > -                $@ 2>&1
> > > +                echo $@ >&2
> > > +                $@
> > >   	fi
> > >
> > >   	err=$?
> > >
> > > --
> > > 2.30.2
> > >
> > >
>
>

Re: [cocci] [PATCH 2/2] scripts: coccicheck: Separate spatch stdout and stderr

[Anton Eliasson]

On 10/10/2023 18.11, Julia Lawall wrote:
>
> On Tue, 10 Oct 2023, Anton Eliasson wrote:
>
>> On 07/10/2023 21.41, Julia Lawall wrote:
>>> On Tue, 3 Oct 2023, Anton Eliasson wrote:
>>>
>>>> This helps automating coccicheck runs by discarding stderr and only
>>>> looking at the output of stdout. In report mode the only remaining
>>>> output on stdout is the initial "Please check for false positives"
>>>> message followed by each spatch warning found.
>>> What is getting dropped is the spatch command lines indicating the
>>> semantic patch.  Is this desirable?
>>>
>>> julia
>> It's not ideal but it's the best compromise that I have found. The problem I'm
>> trying to solve is to be able to diff the output of two coccicheck runs and
>> notify the developer if any new warnings were introduced. That requires the
>> output to be stable. spatch is always invoked for each cocci file in the same
>> order. However, the output from each spatch invocation is not stable as it
>> examines each source file in an arbitrary order.
>>
>> My workaround is to sort the output before diffing. The line-by-line sorted
>> output only makes sense if the input is one line per warning found and that is
>> why I try to discard all output except the single line per spatch warning.
>> While the terse output doesn't tell which semantic patch file generated the
>> warning, it does usually contain the offending file, line number and a summary
>> of the issue.
> Why does the command line pose a problem for sorting?
>
> julia

You're right. I was overthinking it. Since the sorted command lines will 
be common for the two runs they will disappear after diffing.

So at this point I don't have any need for this patch. I'll reach out to 
you again if it turns out to be an issue after we have gotten the 
continuous integration check in place. Thanks for the feedback and I'm 
sorry about the noise.


Anton

>
>>
>> Anton
>>>> Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
>>>> ---
>>>>    scripts/coccicheck | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/scripts/coccicheck b/scripts/coccicheck
>>>> index 95a312730e98..7e7c44125f47 100755
>>>> --- a/scripts/coccicheck
>>>> +++ b/scripts/coccicheck
>>>> @@ -146,8 +146,8 @@ run_cmd_parmap() {
>>>>                    echo $@>>$DEBUG_FILE
>>>>                    $@ 2>>$DEBUG_FILE
>>>>            else
>>>> -                echo $@
>>>> -                $@ 2>&1
>>>> +                echo $@ >&2
>>>> +                $@
>>>>    	fi
>>>>
>>>>    	err=$?
>>>>
>>>> --
>>>> 2.30.2
>>>>
>>>>
>>

Re: [cocci] [PATCH 2/2] scripts: coccicheck: Separate spatch stdout and stderr

[Julia Lawall]

On Wed, 11 Oct 2023, Anton Eliasson wrote:

> On 10/10/2023 18.11, Julia Lawall wrote:
> >
> > On Tue, 10 Oct 2023, Anton Eliasson wrote:
> >
> > > On 07/10/2023 21.41, Julia Lawall wrote:
> > > > On Tue, 3 Oct 2023, Anton Eliasson wrote:
> > > >
> > > > > This helps automating coccicheck runs by discarding stderr and only
> > > > > looking at the output of stdout. In report mode the only remaining
> > > > > output on stdout is the initial "Please check for false positives"
> > > > > message followed by each spatch warning found.
> > > > What is getting dropped is the spatch command lines indicating the
> > > > semantic patch.  Is this desirable?
> > > >
> > > > julia
> > > It's not ideal but it's the best compromise that I have found. The problem
> > > I'm
> > > trying to solve is to be able to diff the output of two coccicheck runs
> > > and
> > > notify the developer if any new warnings were introduced. That requires
> > > the
> > > output to be stable. spatch is always invoked for each cocci file in the
> > > same
> > > order. However, the output from each spatch invocation is not stable as it
> > > examines each source file in an arbitrary order.
> > >
> > > My workaround is to sort the output before diffing. The line-by-line
> > > sorted
> > > output only makes sense if the input is one line per warning found and
> > > that is
> > > why I try to discard all output except the single line per spatch warning.
> > > While the terse output doesn't tell which semantic patch file generated
> > > the
> > > warning, it does usually contain the offending file, line number and a
> > > summary
> > > of the issue.
> > Why does the command line pose a problem for sorting?
> >
> > julia
>
> You're right. I was overthinking it. Since the sorted command lines will be
> common for the two runs they will disappear after diffing.
>
> So at this point I don't have any need for this patch. I'll reach out to you
> again if it turns out to be an issue after we have gotten the continuous
> integration check in place. Thanks for the feedback and I'm sorry about the
> noise.

OK, thanks for the discussion.  I was also thinking about whether it could
be possible to make the output always come out in the same order, based on
the name of the analyzed file.  Maybe it is possible.

julia


>
>
> Anton
>
> >
> > >
> > > Anton
> > > > > Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
> > > > > ---
> > > > >    scripts/coccicheck | 4 ++--
> > > > >    1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/scripts/coccicheck b/scripts/coccicheck
> > > > > index 95a312730e98..7e7c44125f47 100755
> > > > > --- a/scripts/coccicheck
> > > > > +++ b/scripts/coccicheck
> > > > > @@ -146,8 +146,8 @@ run_cmd_parmap() {
> > > > >                    echo $@>>$DEBUG_FILE
> > > > >                    $@ 2>>$DEBUG_FILE
> > > > >            else
> > > > > -                echo $@
> > > > > -                $@ 2>&1
> > > > > +                echo $@ >&2
> > > > > +                $@
> > > > >    	fi
> > > > >
> > > > >    	err=$?
> > > > >
> > > > > --
> > > > > 2.30.2
> > > > >
> > > > >
> > >
>
>

Re: [cocci] [PATCH 2/2] scripts: coccicheck: Separate spatch stdout and stderr

[Anton Eliasson]

On 11/10/2023 15.46, Julia Lawall wrote:
>
> On Wed, 11 Oct 2023, Anton Eliasson wrote:
>
>> On 10/10/2023 18.11, Julia Lawall wrote:
>>> On Tue, 10 Oct 2023, Anton Eliasson wrote:
>>>
>>>> On 07/10/2023 21.41, Julia Lawall wrote:
>>>>> On Tue, 3 Oct 2023, Anton Eliasson wrote:
>>>>>
>>>>>> This helps automating coccicheck runs by discarding stderr and only
>>>>>> looking at the output of stdout. In report mode the only remaining
>>>>>> output on stdout is the initial "Please check for false positives"
>>>>>> message followed by each spatch warning found.
>>>>> What is getting dropped is the spatch command lines indicating the
>>>>> semantic patch.  Is this desirable?
>>>>>
>>>>> julia
>>>> It's not ideal but it's the best compromise that I have found. The problem
>>>> I'm
>>>> trying to solve is to be able to diff the output of two coccicheck runs
>>>> and
>>>> notify the developer if any new warnings were introduced. That requires
>>>> the
>>>> output to be stable. spatch is always invoked for each cocci file in the
>>>> same
>>>> order. However, the output from each spatch invocation is not stable as it
>>>> examines each source file in an arbitrary order.
>>>>
>>>> My workaround is to sort the output before diffing. The line-by-line
>>>> sorted
>>>> output only makes sense if the input is one line per warning found and
>>>> that is
>>>> why I try to discard all output except the single line per spatch warning.
>>>> While the terse output doesn't tell which semantic patch file generated
>>>> the
>>>> warning, it does usually contain the offending file, line number and a
>>>> summary
>>>> of the issue.
>>> Why does the command line pose a problem for sorting?
>>>
>>> julia
>> You're right. I was overthinking it. Since the sorted command lines will be
>> common for the two runs they will disappear after diffing.
>>
>> So at this point I don't have any need for this patch. I'll reach out to you
>> again if it turns out to be an issue after we have gotten the continuous
>> integration check in place. Thanks for the feedback and I'm sorry about the
>> noise.
> OK, thanks for the discussion.  I was also thinking about whether it could
> be possible to make the output always come out in the same order, based on
> the name of the analyzed file.  Maybe it is possible.
>
> julia

Yes, that might help to preserve some context. The difference between 
two runs could be presented as a unified diff instead of just lines 
added. That would probably contain the command line invoking the 
semantic patch that caused the warning, unless there are too many 
pre-existent warnings.


Anton

>
>>
>> Anton
>>
>>>> Anton
>>>>>> Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
>>>>>> ---
>>>>>>     scripts/coccicheck | 4 ++--
>>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/scripts/coccicheck b/scripts/coccicheck
>>>>>> index 95a312730e98..7e7c44125f47 100755
>>>>>> --- a/scripts/coccicheck
>>>>>> +++ b/scripts/coccicheck
>>>>>> @@ -146,8 +146,8 @@ run_cmd_parmap() {
>>>>>>                     echo $@>>$DEBUG_FILE
>>>>>>                     $@ 2>>$DEBUG_FILE
>>>>>>             else
>>>>>> -                echo $@
>>>>>> -                $@ 2>&1
>>>>>> +                echo $@ >&2
>>>>>> +                $@
>>>>>>     	fi
>>>>>>
>>>>>>     	err=$?
>>>>>>
>>>>>> --
>>>>>> 2.30.2
>>>>>>
>>>>>>
>>

Re: [cocci] Detecting differences according to source code analysis warnings

[Markus Elfring]

>>>> The problem I'm trying to solve is to be able to diff the output of two coccicheck runs
>>>> and notify the developer if any new warnings were introduced.

How did source code analyses evolve?


>>>> That requires the output to be stable.

This seems to be a reasonable expectation.


>>>> spatch is always invoked for each cocci file in the same order.
>>>> However, the output from each spatch invocation is not stable as it
>>>> examines each source file in an arbitrary order.

Will such information need further clarifications?


>>>> My workaround is to sort the output before diffing. The line-by-line
>>>> sorted output only makes sense if the input is one line per warning found
>>>> and that is why I try to discard all output except the single line per spatch warning.

Can any other data formats become more appropriate for efficient comparisons?


> OK, thanks for the discussion.  I was also thinking about whether it could
> be possible to make the output always come out in the same order, based on
> the name of the analyzed file.

The Coccinelle software determines file lists.
How stable are corresponding file search results actually?


> Maybe it is possible.

Selected data processing variants can achieve something in this direction.

I imagine that there are possibilities like the following if a “stable” file list
can be used.

* Perform desired source code analyses only sequentially.
  I am unsure if this processing mode would fit to execution performance expectations
  depending on the size of used file selections.

* I guess that the current parallelisation technique can trigger data output variations
  because of the creation of background processes.
  https://opam.ocaml.org/packages/parmap/

  I assume that algorithms can be adjusted for the concatenation of generated output data.

  The programming language version “OCaml 5” (from 2022-12-16) got better support
  for multi-threading.
  https://v2.ocaml.org/releases/5.0/manual/parallelism.html

  The program “spatch” can eventually control the software execution then in ways
  for a consistent data processing order, can't it?


Regards,
Markus

Re: [cocci] [PATCH 1/2] scripts: coccicheck: Return error from run_cmd_parmap

[Julia Lawall]

----- Mail original -----
> De: "Anton Eliasson" <anton.eliasson@axis.com>
> À: "Julia Lawall" <Julia.Lawall@inria.fr>, "nicolas palix" <nicolas.palix@imag.fr>
> Cc: "cocci" <cocci@inria.fr>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Anton Eliasson" <anton.eliasson@axis.com>,
> kernel@axis.com
> Envoyé: Mardi 3 Octobre 2023 16:25:14
> Objet: [cocci] [PATCH 1/2] scripts: coccicheck: Return error from run_cmd_parmap

> Exiting on error breaks the chain mode. Return the error instead in
> order for the caller to propagate it or in the case of chain, try the
> next mode.
> 
> Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
> ---
> scripts/coccicheck | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/coccicheck b/scripts/coccicheck
> index e52cb43fede6..95a312730e98 100755
> --- a/scripts/coccicheck
> +++ b/scripts/coccicheck
> @@ -153,7 +153,7 @@ run_cmd_parmap() {
> 	err=$?
> 	if [[ $err -ne 0 ]]; then
> 		echo "coccicheck failed"
> -		exit $err
> +		return $err
> 	fi
> }
> 

I tried disabling OCaml in my version of Coccinelle and then ran make coccicheck with this patch.  But I didn't see any improvement.  On the other hand, it keeps going if I just remove the exit line entirely.  Is that what is wanted?  One can still see the coccicheck failed message.

julia

> --
> 2.30.2

Re: [cocci] [PATCH 1/2] scripts: coccicheck: Return error from run_cmd_parmap

[Anton Eliasson]

On 02/11/2023 22.27, Julia Lawall wrote:
>
> ----- Mail original -----
>> De: "Anton Eliasson" <anton.eliasson@axis.com>
>> À: "Julia Lawall" <Julia.Lawall@inria.fr>, "nicolas palix" <nicolas.palix@imag.fr>
>> Cc: "cocci" <cocci@inria.fr>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Anton Eliasson" <anton.eliasson@axis.com>,
>> kernel@axis.com
>> Envoyé: Mardi 3 Octobre 2023 16:25:14
>> Objet: [cocci] [PATCH 1/2] scripts: coccicheck: Return error from run_cmd_parmap
>> Exiting on error breaks the chain mode. Return the error instead in
>> order for the caller to propagate it or in the case of chain, try the
>> next mode.
>>
>> Signed-off-by: Anton Eliasson <anton.eliasson@axis.com>
>> ---
>> scripts/coccicheck | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/coccicheck b/scripts/coccicheck
>> index e52cb43fede6..95a312730e98 100755
>> --- a/scripts/coccicheck
>> +++ b/scripts/coccicheck
>> @@ -153,7 +153,7 @@ run_cmd_parmap() {
>> 	err=$?
>> 	if [[ $err -ne 0 ]]; then
>> 		echo "coccicheck failed"
>> -		exit $err
>> +		return $err
>> 	fi
>> }
>>
> I tried disabling OCaml in my version of Coccinelle and then ran make coccicheck with this patch.  But I didn't see any improvement.  On the other hand, it keeps going if I just remove the exit line entirely.  Is that what is wanted?  One can still see the coccicheck failed message.
>
> julia
>
Hi again!

It only makes a difference in chain mode, I think. Without my change 
coccicheck stops after encountering the first semantic patch that 
doesn't support the patch rule (see the console output below). With the 
change coccicheck keeps going with the next semantic patch which is what 
I expect. Do you see a different behavior?

However, I found now that with the change it gets harder to cancel 
coccicheck. A single ctrl-c is no longer enough. I basically have to 
hold down ctrl-c until all the script layers have returned and exited. 
So that's maybe not ideal.

> $ git describe
> v6.6
> $ make coccicheck MODE=chain
> You have selected the "chain" mode.
> All available modes will be tried (in that order): patch, report, 
> context, org
>
> Please check for false positives in the output before submitting a patch.
> When using "patch" mode, carefully review the patch before submitting it.
>
> /usr/bin/spatch -D patch --very-quiet --cocci-file 
> ./scripts/coccinelle/api/alloc/alloc_cast.cocci --no-includes 
> --include-headers --dir . -I ./arch/x86/include -I 
> ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi 
> -I ./arch/x86/include/generated/uapi -I ./include/uapi -I 
> ./include/generated/uapi --include ./include/linux/compiler-version.h 
> --include ./include/linux/kconfig.h --jobs 16 --chunksize 1
> 14406 files match
> /usr/bin/spatch -D patch --very-quiet --cocci-file 
> ./scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci --no-includes 
> --include-headers --dir . -I ./arch/x86/include -I 
> ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi 
> -I ./arch/x86/include/generated/uapi -I ./include/uapi -I 
> ./include/generated/uapi --include ./include/linux/compiler-version.h 
> --include ./include/linux/kconfig.h --jobs 16 --chunksize 1
> 44 files match
> /usr/bin/spatch -D patch --very-quiet --cocci-file 
> ./scripts/coccinelle/api/alloc/zalloc-simple.cocci --no-includes 
> --include-headers --dir . -I ./arch/x86/include -I 
> ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi 
> -I ./arch/x86/include/generated/uapi -I ./include/uapi -I 
> ./include/generated/uapi --include ./include/linux/compiler-version.h 
> --include ./include/linux/kconfig.h --jobs 16 --chunksize 1
> 2146 files match
> diff -u -p a/fs/direct-io.c b/fs/direct-io.c
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -1128,15 +1128,9 @@ ssize_t __blockdev_direct_IO(struct kioc
>      if (iov_iter_rw(iter) == READ && !count)
>          return 0;
>
> -    dio = kmem_cache_alloc(dio_cache, GFP_KERNEL);
> +    dio = kmem_cache_zalloc(dio_cache, GFP_KERNEL);
>      if (!dio)
>          return -ENOMEM;
> -    /*
> -     * Believe it or not, zeroing out the page array caused a .5%
> -     * performance regression in a database benchmark.  So, we take
> -     * care to only zero out what's needed.
> -     */
> -    memset(dio, 0, offsetof(struct dio, pages));
>
>      dio->flags = flags;
>      if (dio->flags & DIO_LOCKING && iov_iter_rw(iter) == READ) {
> /usr/bin/spatch -D patch --very-quiet --cocci-file 
> ./scripts/coccinelle/api/atomic_as_refcounter.cocci --include-headers 
> --very-quiet --dir . -I ./arch/x86/include -I 
> ./arch/x86/include/generated -I ./include -I ./arch/x86/include/uapi 
> -I ./arch/x86/include/generated/uapi -I ./include/uapi -I 
> ./include/generated/uapi --include ./include/linux/compiler-version.h 
> --include ./include/linux/kconfig.h --jobs 16 --chunksize 1
> virtual rule patch not supported
> coccicheck failed
> make[1]: *** [/home/antone/git/linux/Makefile:2005: coccicheck] Error 255
> make: *** [Makefile:234: __sub-make] Error 2