Warns or generates patch for NULL check before the following functions:
kfree
usb_free_urb
debugfs_remove
debugfs_remove_recursive
Cc: Julia Lawall <[email protected]>
Cc: Gilles Muller <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Andrew Morton <[email protected]>
Signed-off-by: Fabian Frederick <[email protected]>
---
V3:
-Update print_main message.
-Add patch mode (suggested by Julia Lawall)
V2:
-Add 3 more functions to kfree (suggested by Joe Perches)
-Update warning message to involve code analysis (suggested by Julia Lawall)
scripts/coccinelle/free/ifnullfree.cocci | 53 ++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100644 scripts/coccinelle/free/ifnullfree.cocci
diff --git a/scripts/coccinelle/free/ifnullfree.cocci b/scripts/coccinelle/free/ifnullfree.cocci
new file mode 100644
index 0000000..c826d98
--- /dev/null
+++ b/scripts/coccinelle/free/ifnullfree.cocci
@@ -0,0 +1,53 @@
+/// NULL check before some freeing functions is not needed.
+///
+/// Based on checkpatch warning
+/// "kfree(NULL) is safe this check is probably not required"
+/// and kfreeaddr.cocci by Julia Lawall.
+///
+/// Comments: -
+/// Options: --no-includes --include-headers
+
+virtual patch
+virtual org
+virtual report
+virtual context
+
+@r2 depends on patch@
+expression E;
+@@
+- if (E)
+(
+- kfree(E);
++ kfree(E);
+|
+- debugfs_remove(E);
++ debugfs_remove(E);
+|
+- debugfs_remove_recursive(E);
++ debugfs_remove_recursive(E);
+|
+- usb_free_urb(E);
++ usb_free_urb(E);
+)
+
+@r depends on context || report || org @
+expression E;
+position p;
+@@
+
+* if (E)
+* \(kfree@p\|debugfs_remove@p\|debugfs_remove_recursive@p\|usb_free_urb\)(E);
+
+@script:python depends on org@
+p << r.p;
+@@
+
+cocci.print_main("NULL check before that freeing function is not needed", p)
+
+@script:python depends on report@
+p << r.p;
+@@
+
+msg = "WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values."
+coccilib.report.print_report(p[0], msg)
+
--
1.8.4.5
> V3:
> -Update print_main message.
Does the discussion topic need also an adjustment?
How do you think about my previous update suggestion "Deletion of
unnecessary checks before specific function calls"?
https://systeme.lip6.fr/pipermail/cocci/2014-March/000675.html
https://lkml.org/lkml/2014/3/5/344
Regards,
Markus
On Sat, 28 Jun 2014 11:08:35 +0200
SF Markus Elfring <[email protected]> wrote:
>
> > V3:
> > -Update print_main message.
>
> Does the discussion topic need also an adjustment?
>
> How do you think about my previous update suggestion "Deletion of
> unnecessary checks before specific function calls"?
> https://systeme.lip6.fr/pipermail/cocci/2014-March/000675.html
> https://lkml.org/lkml/2014/3/5/344
Hello Markus,
I didn't see you made the same kind of script.
Ok, you can send yours and we forget this one :)
Regards,
Fabian
>
> Regards,
> Markus
>> How do you think about my previous update suggestion "Deletion of
>> unnecessary checks before specific function calls"?
>> https://systeme.lip6.fr/pipermail/cocci/2014-March/000675.html
>> https://lkml.org/lkml/2014/3/5/344
> I didn't see you made the same kind of script.
Will my approach become a bit more complete in the near future?
http://article.gmane.org/gmane.comp.version-control.coccinelle/3512/
Regards,
Markus
On Sat, 28 Jun 2014 13:38:15 +0200
SF Markus Elfring <[email protected]> wrote:
>
> >> How do you think about my previous update suggestion "Deletion of
> >> unnecessary checks before specific function calls"?
> >> https://systeme.lip6.fr/pipermail/cocci/2014-March/000675.html
> >> https://lkml.org/lkml/2014/3/5/344
> > I didn't see you made the same kind of script.
>
> Will my approach become a bit more complete in the near future?
> http://article.gmane.org/gmane.comp.version-control.coccinelle/3512/
>
Sorry but I'm not working on coccinelle ; just did a small script for kernel tree. I guess you'll have an answer from coccinelle people.
Regards,
Fabian
> Regards,
> Markus
On Sat, 28 Jun 2014, Fabian Frederick wrote:
> Warns or generates patch for NULL check before the following functions:
>
> kfree
> usb_free_urb
> debugfs_remove
> debugfs_remove_recursive
>
> Cc: Julia Lawall <[email protected]>
> Cc: Gilles Muller <[email protected]>
> Cc: Joe Perches <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Signed-off-by: Fabian Frederick <[email protected]>
Acked-by: Julia Lawall <[email protected]>
> ---
>
> V3:
> -Update print_main message.
> -Add patch mode (suggested by Julia Lawall)
>
> V2:
> -Add 3 more functions to kfree (suggested by Joe Perches)
> -Update warning message to involve code analysis (suggested by Julia Lawall)
>
> scripts/coccinelle/free/ifnullfree.cocci | 53 ++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
> create mode 100644 scripts/coccinelle/free/ifnullfree.cocci
>
> diff --git a/scripts/coccinelle/free/ifnullfree.cocci b/scripts/coccinelle/free/ifnullfree.cocci
> new file mode 100644
> index 0000000..c826d98
> --- /dev/null
> +++ b/scripts/coccinelle/free/ifnullfree.cocci
> @@ -0,0 +1,53 @@
> +/// NULL check before some freeing functions is not needed.
> +///
> +/// Based on checkpatch warning
> +/// "kfree(NULL) is safe this check is probably not required"
> +/// and kfreeaddr.cocci by Julia Lawall.
> +///
> +/// Comments: -
> +/// Options: --no-includes --include-headers
> +
> +virtual patch
> +virtual org
> +virtual report
> +virtual context
> +
> +@r2 depends on patch@
> +expression E;
> +@@
> +- if (E)
> +(
> +- kfree(E);
> ++ kfree(E);
> +|
> +- debugfs_remove(E);
> ++ debugfs_remove(E);
> +|
> +- debugfs_remove_recursive(E);
> ++ debugfs_remove_recursive(E);
> +|
> +- usb_free_urb(E);
> ++ usb_free_urb(E);
> +)
> +
> +@r depends on context || report || org @
> +expression E;
> +position p;
> +@@
> +
> +* if (E)
> +* \(kfree@p\|debugfs_remove@p\|debugfs_remove_recursive@p\|usb_free_urb\)(E);
> +
> +@script:python depends on org@
> +p << r.p;
> +@@
> +
> +cocci.print_main("NULL check before that freeing function is not needed", p)
> +
> +@script:python depends on report@
> +p << r.p;
> +@@
> +
> +msg = "WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values."
> +coccilib.report.print_report(p[0], msg)
> +
> --
> 1.8.4.5
>
>
On Sat, 28 Jun 2014, Julia Lawall wrote:
> On Sat, 28 Jun 2014, Fabian Frederick wrote:
>
> > Warns or generates patch for NULL check before the following functions:
> >
> > kfree
> > usb_free_urb
> > debugfs_remove
> > debugfs_remove_recursive
> >
> > Cc: Julia Lawall <[email protected]>
> > Cc: Gilles Muller <[email protected]>
> > Cc: Joe Perches <[email protected]>
> > Cc: Andrew Morton <[email protected]>
> > Signed-off-by: Fabian Frederick <[email protected]>
>
> Acked-by: Julia Lawall <[email protected]>
>
> > ---
> >
> > V3:
> > -Update print_main message.
> > -Add patch mode (suggested by Julia Lawall)
> >
> > V2:
> > -Add 3 more functions to kfree (suggested by Joe Perches)
> > -Update warning message to involve code analysis (suggested by Julia Lawall)
> >
> > scripts/coccinelle/free/ifnullfree.cocci | 53 ++++++++++++++++++++++++++++++++
> > 1 file changed, 53 insertions(+)
> > create mode 100644 scripts/coccinelle/free/ifnullfree.cocci
> >
> > diff --git a/scripts/coccinelle/free/ifnullfree.cocci b/scripts/coccinelle/free/ifnullfree.cocci
> > new file mode 100644
> > index 0000000..c826d98
> > --- /dev/null
> > +++ b/scripts/coccinelle/free/ifnullfree.cocci
> > @@ -0,0 +1,53 @@
> > +/// NULL check before some freeing functions is not needed.
> > +///
> > +/// Based on checkpatch warning
> > +/// "kfree(NULL) is safe this check is probably not required"
> > +/// and kfreeaddr.cocci by Julia Lawall.
> > +///
> > +/// Comments: -
> > +/// Options: --no-includes --include-headers
> > +
> > +virtual patch
> > +virtual org
> > +virtual report
> > +virtual context
> > +
> > +@r2 depends on patch@
> > +expression E;
> > +@@
> > +- if (E)
> > +(
> > +- kfree(E);
> > ++ kfree(E);
> > +|
> > +- debugfs_remove(E);
> > ++ debugfs_remove(E);
> > +|
> > +- debugfs_remove_recursive(E);
> > ++ debugfs_remove_recursive(E);
> > +|
> > +- usb_free_urb(E);
> > ++ usb_free_urb(E);
> > +)
> > +
> > +@r depends on context || report || org @
> > +expression E;
> > +position p;
> > +@@
> > +
> > +* if (E)
> > +* \(kfree@p\|debugfs_remove@p\|debugfs_remove_recursive@p\|usb_free_urb\)(E);
> > +
> > +@script:python depends on org@
> > +p << r.p;
> > +@@
> > +
> > +cocci.print_main("NULL check before that freeing function is not needed", p)
> > +
> > +@script:python depends on report@
> > +p << r.p;
> > +@@
> > +
> > +msg = "WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values."
> > +coccilib.report.print_report(p[0], msg)
> > +
> > --
> > 1.8.4.5
> >
> >
>
On Sat, 28 Jun 2014, SF Markus Elfring wrote:
>
> > V3:
> > -Update print_main message.
>
> Does the discussion topic need also an adjustment?
>
> How do you think about my previous update suggestion "Deletion of
> unnecessary checks before specific function calls"?
> https://systeme.lip6.fr/pipermail/cocci/2014-March/000675.html
> https://lkml.org/lkml/2014/3/5/344
I would prefer to accept Fabian's semantic patch for the moment, as it
considers a more restricted, well thought out, set of functions,
julia
> Sorry but I'm not working on coccinelle ; just did a small script for kernel tree.
I would appreciate a more constructive feedback for my update suggestions around
the topic "Deletion of unnecessary checks before specific function calls".
- Did you look at the concrete patches?
- How do you think about the general approach?
Examples:
https://systeme.lip6.fr/pipermail/cocci/2014-March/000676.html
http://marc.info/?l=kernel-janitors&m=139405971927100&w=2
https://systeme.lip6.fr/pipermail/cocci/2014-March/000677.html
http://marc.info/?l=kernel-janitors&m=139405983727148&w=2
Regards,
Markus
On Sat, 28 Jun 2014, SF Markus Elfring wrote:
> > Sorry but I'm not working on coccinelle ; just did a small script for kernel tree.
>
> I would appreciate a more constructive feedback for my update suggestions around
> the topic "Deletion of unnecessary checks before specific function calls".
> - Did you look at the concrete patches?
> - How do you think about the general approach?
>
> Examples:
> https://systeme.lip6.fr/pipermail/cocci/2014-March/000676.html
> http://marc.info/?l=kernel-janitors&m=139405971927100&w=2
Regular expressions do not allow taking advantage of the optimizations
provided by Coccinelle and are not easy for a reader to understand.
> https://systeme.lip6.fr/pipermail/cocci/2014-March/000677.html
> http://marc.info/?l=kernel-janitors&m=139405983727148&w=2
This is not safe due to the use of when any. You have no guarantee that
there is not a dereference of input in the matched region, or that the
value of input is still the argument value at the point of the test.
julia
On Sat, 28 Jun 2014 23:00:22 +0200
SF Markus Elfring <[email protected]> wrote:
> > Sorry but I'm not working on coccinelle ; just did a small script for kernel tree.
>
> I would appreciate a more constructive feedback for my update suggestions around
> the topic "Deletion of unnecessary checks before specific function calls".
> - Did you look at the concrete patches?
> - How do you think about the general approach?
>
> Examples:
> https://systeme.lip6.fr/pipermail/cocci/2014-March/000676.html
> http://marc.info/?l=kernel-janitors&m=139405971927100&w=2
>
> https://systeme.lip6.fr/pipermail/cocci/2014-March/000677.html
> http://marc.info/?l=kernel-janitors&m=139405983727148&w=2
>
> Regards,
> Markus
Hello Markus,
I hope you had information you wanted from Julia ?
If you want more feedback, maybe you can create a new RFC (Request For Comment) in Linux Kernel mailing.
Regards,
Fabian
>> https://systeme.lip6.fr/pipermail/cocci/2014-March/000676.html
>> http://marc.info/?l=kernel-janitors&m=139405971927100&w=2
> Regular expressions do not allow taking advantage of the optimizations
> provided by Coccinelle and are not easy for a reader to understand.
I find that the application of regexes is appropriate here.
Would you like to optimise any implementation details from my general
approach?
>> https://systeme.lip6.fr/pipermail/cocci/2014-March/000677.html
>> http://marc.info/?l=kernel-janitors&m=139405983727148&w=2
> This is not safe due to the use of when any. You have no guarantee that
> there is not a dereference of input in the matched region, or that the
> value of input is still the argument value at the point of the test.
Do you suggest to make the desired detection of input parameter
validation a bit safer?
Regards,
Markus
> I hope you had information you wanted from Julia ?
Yes. - I am waiting on another Coccinelle software release which will
contain improvements for my feature requests and bug reports.
I'm curious when I should retry proposed filter patterns.
Regards,
Markus