[cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Markus Elfring]

Hello,

I accidentally noticed an update candidate.

c4iw_rdev_open():
https://elixir.bootlin.com/linux/v6.15.1/source/drivers/infiniband/hw/cxgb4/device.c#L908-L909

I got into the mood to construct another script variant for the semantic patch language.

@deletion@
expression e;
@@
 if (...
-    && e
    )
    kfree(e);


A corresponding patch can be generated accordingly.

Markus_Elfring@Sonne:…/Projekte/Linux/next-analyses> time /usr/bin/spatch --no-loops …/Projekte/Coccinelle/janitor/delete_redundant_checks4.cocci drivers/infiniband/hw/cxgb4/device.c
…
@@ -905,7 +905,7 @@ static int c4iw_rdev_open(struct c4iw_rd
 
        return 0;
 err_free_status_page_and_wr_log:
-       if (c4iw_wr_log && rdev->wr_log)
+       if (c4iw_wr_log)
                kfree(rdev->wr_log);
        free_page((unsigned long)rdev->status_page);
 destroy_ocqp_pool:

real    0m0,454s
user    0m0,413s
sys     0m0,038s


I tried another program call out.

Markus_Elfring@Sonne:…/Projekte/Linux/next-analyses> git checkout next-20250606 && time /usr/bin/spatch --no-loops --timeout 45 -j4 --chunksize 1 -dir . …/Projekte/Coccinelle/janitor/delete_redundant_checks4.cocci > …/Projekte/Bau/Linux/scripts/Coccinelle/deletions1/next/20250606/delete_redundant_checks4.diff 2> …/Projekte/Bau/Linux/scripts/Coccinelle/deletions1/next/20250606/delete_redundant_checks4-errors.txt
…
real    5m42,692s
user    20m26,077s
sys     0m13,532s


Now I wonder why the known generated patch was not written into the specified
diff file for this command variant.
How will the desired data output become consistent (after the software
combination “Coccinelle 1.3.0”) for such an use case?

Regards,
Markus

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Julia Lawall]

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

On Tue, 10 Jun 2025, Markus Elfring wrote:

> Hello,
>
> I accidentally noticed an update candidate.
>
> c4iw_rdev_open():
> https://elixir.bootlin.com/linux/v6.15.1/source/drivers/infiniband/hw/cxgb4/device.c#L908-L909
>
> I got into the mood to construct another script variant for the semantic patch language.
>
> @deletion@
> expression e;
> @@
>  if (...
> -    && e
>     )
>     kfree(e);
>
>
> A corresponding patch can be generated accordingly.
>
> Markus_Elfring@Sonne:…/Projekte/Linux/next-analyses> time /usr/bin/spatch --no-loops …/Projekte/Coccinelle/janitor/delete_redundant_checks4.cocci drivers/infiniband/hw/cxgb4/device.c
> …
> @@ -905,7 +905,7 @@ static int c4iw_rdev_open(struct c4iw_rd
>
>         return 0;
>  err_free_status_page_and_wr_log:
> -       if (c4iw_wr_log && rdev->wr_log)
> +       if (c4iw_wr_log)
>                 kfree(rdev->wr_log);
>         free_page((unsigned long)rdev->status_page);
>  destroy_ocqp_pool:
>
> real    0m0,454s
> user    0m0,413s
> sys     0m0,038s
>
>
> I tried another program call out.
>
> Markus_Elfring@Sonne:…/Projekte/Linux/next-analyses> git checkout next-20250606 && time /usr/bin/spatch --no-loops --timeout 45 -j4 --chunksize 1 -dir . …/Projekte/Coccinelle/janitor/delete_redundant_checks4.cocci > …/Projekte/Bau/Linux/scripts/Coccinelle/deletions1/next/20250606/delete_redundant_checks4.diff 2> …/Projekte/Bau/Linux/scripts/Coccinelle/deletions1/next/20250606/delete_redundant_checks4-errors.txt
> …
> real    5m42,692s
> user    20m26,077s
> sys     0m13,532s
>
>
> Now I wonder why the known generated patch was not written into the specified
> diff file for this command variant.
> How will the desired data output become consistent (after the software
> combination “Coccinelle 1.3.0”) for such an use case?

Could yo uplease make a small effort to make your reports understandable?

1) I don't ned to know the paths of your files?  This adds a lot of
clutter that makes it hard to find the important information.

2) The proper scientific method is to cange only one thing at a time.  You
seem tohave changed two things 1: in one case you give the specific file
name andin the second case you use the whole Linux kernel directory, and
2; in one case you are using linux-next from a specific date andin the
other case there is no information about what kernel version you are
using.

In the second test, can you check whether the file you expect to be
considered is included in a HANDLING line in the output?

If it is included in the HANDLING line in the output, can you resend your
report that addresses the above two issues?

julia

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Markus Elfring]

> Could yo uplease make a small effort to make your reports understandable?

Why do the presented test commands trigger undesirable understanding difficulties?


> 1) I don't ned to know the paths of your files?  This adds a lot of
> clutter that makes it hard to find the important information.

I find the mentioned test data relevant for further clarifications.


> 2) The proper scientific method is to cange only one thing at a time.

There are two data processing results to compare.


> You seem tohave changed two things 1: in one case you give the specific file name

Something looks reasonable.


> andin the second case you use the whole Linux kernel directory,

Yes.

The mentioned log file contains the following information then:
“…
7675 files match
…”

May I expect that a file name like “./drivers/infiniband/hw/cxgb4/device.c”
should occur there?


> and 2; in one case you are using linux-next from a specific date andin the
> other case there is no information about what kernel version you are
> using.

The mentioned test cases refer to the same Linux development repository.

Regards,
Markus

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Julia Lawall]

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



On Tue, 10 Jun 2025, Markus Elfring wrote:

> > Could yo uplease make a small effort to make your reports understandable?
>
> Why do the presented test commands trigger undesirable understanding difficulties?
>
>
> > 1) I don't ned to know the paths of your files?  This adds a lot of
> > clutter that makes it hard to find the important information.
>
> I find the mentioned test data relevant for further clarifications.
>
>
> > 2) The proper scientific method is to cange only one thing at a time.
>
> There are two data processing results to compare.
>
>
> > You seem tohave changed two things 1: in one case you give the specific file name
>
> Something looks reasonable.
>
>
> > andin the second case you use the whole Linux kernel directory,
>
> Yes.
>
> The mentioned log file contains the following information then:
> “…
> 7675 files match
> …”
>
> May I expect that a file name like “./drivers/infiniband/hw/cxgb4/device.c”
> should occur there?
>
>
> > and 2; in one case you are using linux-next from a specific date andin the
> > other case there is no information about what kernel version you are
> > using.
>
> The mentioned test cases refer to the same Linux development repository.

Did you check for the HANDLING comment as requested?

julia

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Markus Elfring]

>> The mentioned test cases refer to the same Linux development repository.
> 
> Did you check for the HANDLING comment as requested?

The log file “…/Projekte/Bau/Linux/scripts/Coccinelle/deletions1/next/20250606/delete_redundant_checks4-errors.txt”
contains the text “HANDLING: ./” 7675 times at the moment.

Regards,
Markus

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Markus Elfring]

>> The log file “…/Projekte/Bau/Linux/scripts/Coccinelle/deletions1/next/20250606/delete_redundant_checks4-errors.txt”
>> contains the text “HANDLING: ./” 7675 times at the moment.
> 
> Please actually look at what I asked for.  Does it contain HANDLING for
> the file you are concerned with?

I reported that I miss one file path at least from such a data processing result.


>                                   It seems obvious that I would not care
> about the number of occurrences of HANDLING in general.

I wonder about such a feedback.


>                                                          What would I do
> with that?
I would appreciate to clarify if mentioned files would be completely accessible
for the shown source code transformation approach.

Regards,
Markus

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Markus Elfring]

>> I would appreciate to clarify if mentioned files would be completely accessible
>> for the shown source code transformation approach.
> 
> The number 7675 is not going to help me understand what the problem is.
Would you expect that any diff output should appear for the source file
“drivers/infiniband/hw/cxgb4/device.c” also in the file “delete_redundant_checks4.diff”?

Which data processing results do you observe in your software test environments?

Regards,
Markus

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Markus Elfring]

>> I would appreciate to clarify if mentioned files would be completely accessible
>> for the shown source code transformation approach.
> The number 7675 is not going to help me understand what the problem is.

* Would you dare to check data processing results any more for different
  sizes of source file directory hierarchies?

* How will the set of processed files (and software run time characteristics)
  evolve further for the presented use case?


Regards,
Markus

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Markus Elfring]

> Could yo uplease make a small effort to make your reports understandable?

Will another clarification approach become more helpful for this issue?

A known information can be presented as expected.

Markus_Elfring@Sonne:…/Projekte/Coccinelle/janitor> /usr/bin/spatch --parse-cocci delete_redundant_checks4.cocci
…
Grep query
kfree


> 1) I don't ned to know the paths of your files?  This adds a lot of
> clutter that makes it hard to find the important information.

Please reconsider such a response a bit more.


> 2) The proper scientific method is to cange only one thing at a time.

Can any software developers determine the concrete command which would perform
the indicated “Grep query”?
https://gitlab.inria.fr/search?group_id=10912&project_id=18465&repository_ref=master&scope=blobs&search=grep


> In the second test, can you check whether the file you expect to be
> considered is included in a HANDLING line in the output?

Can any software build configuration be achieved which can answer
such a question directly (also according to a special test case)?

Regards,
Markus

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Markus Elfring]

> The number 7675 is not going to help me understand what the problem is.

Can the corresponding message “files match” (which would be provided by a function like “gitgrep_filter”)
be essential for further development considerations?
https://gitlab.inria.fr/coccinelle/coccinelle/-/blob/e892609c351f889e013b9977a413d5b97415e4de/enter.ml#L974-983
https://github.com/coccinelle/coccinelle/blob/4093aad2da23ea5c86916de0a71f43032fc2bb2f/enter.ml#L974-L983

Regards,
Markus

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Markus Elfring]

>> 2) The proper scientific method is to cange only one thing at a time.
> Can any software developers determine the concrete command which would perform
> the indicated “Grep query”?

I got the impression that a function like “interpret” contains interesting
implementation details.
https://gitlab.inria.fr/coccinelle/coccinelle/-/blob/582d2ebd4543dd7febf0670f4a104679ec8bbc2d/parsing_cocci/git_grep.ml#L9-35
https://github.com/coccinelle/coccinelle/blob/4093aad2da23ea5c86916de0a71f43032fc2bb2f/parsing_cocci/git_grep.ml#L9-L23

Some data processing results can be produced accordingly.

Markus_Elfring@Sonne:/home/altes_Heim2/elfring/Projekte/Linux/next-analyses/drivers/infiniband/hw/cxgb4> git grep -l -w kfree '*.c' | rg -v .in | rg device
device.c

Markus_Elfring@Sonne:/home/altes_Heim2/elfring/Projekte/Linux/next-analyses/drivers/infiniband> git grep -l -w kfree '*.c' | rg -v .in | rg cxgb4/device
hw/cxgb4/device.c

Markus_Elfring@Sonne:/home/altes_Heim2/elfring/Projekte/Linux/next-analyses/drivers> git grep -l -w kfree '*.c' | rg -v .in | rg cxgb4/device


Would you become interested to clarify why I do not see another search result here
(while the program “spatch” would still generate an expected diff output for
the selected directory)?

Regards,
Markus

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Julia Lawall]

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



On Thu, 12 Jun 2025, Markus Elfring wrote:

> >> 2) The proper scientific method is to cange only one thing at a time.
> > Can any software developers determine the concrete command which would perform
> > the indicated “Grep query”?
>
> I got the impression that a function like “interpret” contains interesting
> implementation details.
> https://gitlab.inria.fr/coccinelle/coccinelle/-/blob/582d2ebd4543dd7febf0670f4a104679ec8bbc2d/parsing_cocci/git_grep.ml#L9-35
> https://github.com/coccinelle/coccinelle/blob/4093aad2da23ea5c86916de0a71f43032fc2bb2f/parsing_cocci/git_grep.ml#L9-L23
>
> Some data processing results can be produced accordingly.
>
> Markus_Elfring@Sonne:/home/altes_Heim2/elfring/Projekte/Linux/next-analyses/drivers/infiniband/hw/cxgb4> git grep -l -w kfree '*.c' | rg -v .in | rg device
> device.c
>
> Markus_Elfring@Sonne:/home/altes_Heim2/elfring/Projekte/Linux/next-analyses/drivers/infiniband> git grep -l -w kfree '*.c' | rg -v .in | rg cxgb4/device
> hw/cxgb4/device.c
>
> Markus_Elfring@Sonne:/home/altes_Heim2/elfring/Projekte/Linux/next-analyses/drivers> git grep -l -w kfree '*.c' | rg -v .in | rg cxgb4/device
>
>
> Would you become interested to clarify why I do not see another search result here
> (while the program “spatch” would still generate an expected diff output for
> the selected directory)?

I have no idea what is the program rg, but the problem is already fixed.

julia

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Markus Elfring]

>> Would you become interested to clarify why I do not see another search result here
>> (while the program “spatch” would still generate an expected diff output for
>> the selected directory)?
> 
> I have no idea what is the program rg,

See also:
Make program selection configurable for “grep”
2023-02-04
https://github.com/coccinelle/coccinelle/issues/299


> but the problem is already fixed.
Which change would you like to point out here?

Regards,
Markus

RE: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Keller, Jacob E]

> -----Original Message-----
> From: cocci-request@inria.fr <cocci-request@inria.fr> On Behalf Of Julia Lawall
> Sent: Thursday, June 12, 2025 6:03 AM
> To: Markus Elfring <Markus.Elfring@web.de>
> Cc: cocci@inria.fr
> Subject: Re: [cocci] Checking another deletion of redundant null pointer checks
> (with SmPL)?
> I have no idea what is the program rg, but the problem is already fixed.
> 
> Julia

Ripgrep, https://github.com/BurntSushi/ripgrep

It’s a rust-based alternative to grep. Its very fast, always recursive, and has a bunch of nice quality of life features for scanning as it will obey common repository ignores (and doesn't attempt to traverse below .git, .hg, etc).

RE: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Julia Lawall]

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



On Thu, 12 Jun 2025, Keller, Jacob E wrote:

>
>
> > -----Original Message-----
> > From: cocci-request@inria.fr <cocci-request@inria.fr> On Behalf Of Julia Lawall
> > Sent: Thursday, June 12, 2025 6:03 AM
> > To: Markus Elfring <Markus.Elfring@web.de>
> > Cc: cocci@inria.fr
> > Subject: Re: [cocci] Checking another deletion of redundant null pointer checks
> > (with SmPL)?
> > I have no idea what is the program rg, but the problem is already fixed.
> >
> > Julia
>
> Ripgrep, https://github.com/BurntSushi/ripgrep
>
> It’s a rust-based alternative to grep. Its very fast, always recursive, and has a bunch of nice quality of life features for scanning as it will obey common repository ignores (and doesn't attempt to traverse below .git, .hg, etc).

Thanks.

It may be that we are now only using git grep for performance critical
things (at least for git repositories), so perhaps git lets you choose the
grep you want?

julia

RE: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Keller, Jacob E]

> -----Original Message-----
> From: cocci-request@inria.fr <cocci-request@inria.fr> On Behalf Of Julia Lawall
> Sent: Thursday, June 12, 2025 9:05 AM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: Markus Elfring <Markus.Elfring@web.de>; cocci@inria.fr
> Subject: RE: [cocci] Checking another deletion of redundant null pointer checks
> (with SmPL)?
> 
> 
> 
> On Thu, 12 Jun 2025, Keller, Jacob E wrote:
> 
> >
> >
> > > -----Original Message-----
> > > From: cocci-request@inria.fr <cocci-request@inria.fr> On Behalf Of Julia Lawall
> > > Sent: Thursday, June 12, 2025 6:03 AM
> > > To: Markus Elfring <Markus.Elfring@web.de>
> > > Cc: cocci@inria.fr
> > > Subject: Re: [cocci] Checking another deletion of redundant null pointer checks
> > > (with SmPL)?
> > > I have no idea what is the program rg, but the problem is already fixed.
> > >
> > > Julia
> >
> > Ripgrep, https://github.com/BurntSushi/ripgrep
> >
> > It’s a rust-based alternative to grep. Its very fast, always recursive, and has a
> bunch of nice quality of life features for scanning as it will obey common repository
> ignores (and doesn't attempt to traverse below .git, .hg, etc).
> 
> Thanks.
> 
> It may be that we are now only using git grep for performance critical
> things (at least for git repositories), so perhaps git lets you choose the
> grep you want?
> 
> Julia

Git grep has its own builtin implementation and implements several fancy features which are useful, but its not general purpose for searching outside of git repositories. I don't know if it can search using other tools. Ripgrep is nice as a general purpose search tool that happens to also be decent at searching code repositories as well.

Re: [cocci] Checking another deletion of redundant null pointer checks (with SmPL)?

[Markus Elfring]

> Markus_Elfring@Sonne:…/Projekte/Linux/next-analyses/drivers> git grep -l -w kfree '*.c' | rg -v .in | rg cxgb4/device

Further collateral evolution will happen somehow.

See also:
* More precise exclusion of .in files
  2025-06-12
  https://gitlab.inria.fr/coccinelle/coccinelle/-/commit/c717dff248a7fcc308c007bf3c8a705cd2c39c26

* Remaining open issues according to the software component “git grep”?


Regards,
Markus