[Cocci] [PATCH] Coccinelle: kzalloc-simple: Add all zero allocating functions

[Julia Lawall]

There are many instances where memory is allocated using regular allocator
functions immediately followed by setting the allocated memory
to 0 value using memset.
We already have zero memory allocator functions to set the memory to
0 value instead of manually setting it using memset.
Therefore, use zero memory allocating functions instead of regular
memory allocators followed by memset 0 to remove redundant memset and
make the code more cleaner and also reduce the code size.
---
scripts/coccinelle/api/alloc/kzalloc-simple.cocci | 371 +++++++++++++++++++++-
1 file changed, 367 insertions(+), 4 deletions(-)
diff --git a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci b/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
index 52c55e4..f94888d 100644
--- a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
+++ b/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
@@ -1,5 +1,5 @@
///
-/// Use kzalloc rather than kmalloc followed by memset with 0
+/// Use zeroing allocator rather than allocator followed by memset with 0
///
/// This considers some simple cases that are common and easy to validate
/// Note in particular that there are no ...s in the rule, so all of the
@@ -8,6 +8,7 @@
// Confidence: High
// Copyright: (C) 2009-2010 Julia Lawall, Nicolas Palix, DIKU. GPLv2.
// Copyright: (C) 2009-2010 Gilles Muller, INRIA/LiP6. GPLv2.
+// Cpoyright: (C) 2017 Himanshu Jha GPLv2.
// URL: http://coccinelle.lip6.fr/rules/kzalloc.html
// Options: --no-includes --include-headers
//
@@ -28,11 +29,14 @@ virtual report
@depends on context@
type T, T2;
expression x;
-expression E1,E2;
+expression E1;
statement S;
@@
-* x = (T)kmalloc(E1,E2);
+* x = (T)\(kmalloc(E1, ...)\|vmalloc(E1)\|dma_alloc_coherent(...,E1,...)\|
+ kmalloc_node(E1, ...)\|kmem_cache_alloc(...)\|kmem_alloc(E1, ...)\|
+ devm_kmalloc(...,E1,...)\|kvmalloc(E1, ...)\|pci_alloc_consistent(...,E1,...)\|
+ kvmalloc_node(E1,...)\);
if ((x==NULL) || ...) S
* memset((T2)x,0,E1);
@@ -43,12 +47,101 @@ statement S;
@depends on patch@
type T, T2;
expression x;
-expression E1,E2;
+expression E1,E2,E3,E4;
statement S;
@@
+(
+- x = kmalloc(E1,E2);
++ x = kzalloc(E1,E2);
+|
- x = (T)kmalloc(E1,E2);
++ x = (T)kzalloc(E1,E2);
+|
+- x = (T *)kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
+|
+- x = vmalloc(E1);
++ x = vzalloc(E1);
+|
+- x = (T)vmalloc(E1);
++ x = (T)vzalloc(E1);
+|
+- x = (T *)vmalloc(E1);
++ x = vzalloc(E1);
+|
+- x = dma_alloc_coherent(E2,E1,E3,E4);
++ x = dma_zalloc_coherent(E2,E1,E3,E4);
+|
+- x = (T)dma_alloc_coherent(E2,E1,E3,E4);
++ x = (T)dma_zalloc_coherent(E2,E1,E3,E4);
+|
+- x = (T *)dma_alloc_coherent(E2,E1,E3,E4);
++ x = dma_zalloc_coherent(E2,E1,E3,E4);
+|
+- x = kmalloc_node(E1,E2,E3);
++ x = kzalloc_node(E1,E2,E3);
+|
+- x = (T)kmalloc_node(E1,E2,E3);
++ x = (T)kzalloc_node(E1,E2,E3);
+|
+- x = (T *)kmalloc_node(E1,E2,E3);
++ x = kzalloc_node(E1,E2,E3);
+|
+- x = kmem_cache_alloc(E3,E4);
++ x = kmem_cache_zalloc(E3,E4);
+|
+- x = (T)kmem_cache_alloc(E3,E4);
++ x = (T)kmem_cache_zalloc(E3,E4);
+|
+- x = (T *)kmem_cache_alloc(E3,E4);
++ x = kmem_cache_zalloc(E3,E4);
+|
+- x = kmem_alloc(E1,E2);
++ x = kmem_zalloc(E1,E2);
+|
+- x = (T)kmem_alloc(E1,E2);
++ x = (T)kmem_zalloc(E1,E2);
+|
+- x = (T *)kmem_alloc(E1,E2);
++ x = kmem_zalloc(E1,E2);
+|
+- x = devm_kmalloc(E2,E1,E3);
++ x = devm_kzalloc(E2,E1,E3);
+|
+- x = (T)devm_kmalloc(E2,E1,E3);
++ x = (T)devm_kzalloc(E2,E1,E3);
+|
+- x = (T *)devm_kmalloc(E2,E1,E3);
++ x = devm_kzalloc(E2,E1,E3);
+|
+- x = kvmalloc(E1,E2);
++ x = kvzalloc(E1,E2);
+|
+- x = (T)kvmalloc(E1,E2);
++ x = (T)kvzalloc(E1,E2);
+|
+- x = (T *)kvmalloc(E1,E2);
++ x = kvzalloc(E1,E2);
+|
+- x = pci_alloc_consistent(E2,E1,E3);
++ x = pci_zalloc_consistent(E2,E1,E3);
+|
+- x = (T)pci_alloc_consistent(E2,E1,E3);
++ x = (T)pci_zalloc_consistent(E2,E1,E3);
+|
+- x = (T *)pci_alloc_consistent(E2,E1,E3);
++ x = pci_zalloc_consistent(E2,E1,E3);
+|
+- x = kvmalloc_node(E1,E2,E3);
++ x = kvzalloc_node(E1,E2,E3);
+|
+- x = (T)kvmalloc_node(E1,E2,E3);
++ x = (T)kvzalloc_node(E1,E2,E3);
+|
+- x = (T *)kvmalloc_node(E1,E2,E3);
++ x = kvzalloc_node(E1,E2,E3);
+)
if ((x==NULL) || ...) S
- memset((T2)x,0,E1);
@@ -84,3 +177,273 @@ x << r.x;
msg="WARNING: kzalloc should be used for %s, instead of kmalloc/memset" % (x)
coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+type T, T2;
+expression x;
+expression E1;
+statement S;
+position p;
+
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+p << r1.p;
+x << r1.x;
+
+msg="%s" % (x)
+coccilib.org.print_todo(p[0], msg_safe)
+
+p << r1.p;
+x << r1.x;
+
+msg="WARNING: vzalloc should be used for %s, instead of vmalloc/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+type T, T2;
+expression x;
+expression E1,E2,E3,E4;
+statement S;
+position p;
+
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+p << r2.p;
+x << r2.x;
+
+msg="%s" % (x)
+coccilib.org.print_todo(p[0], msg_safe)
+
+p << r2.p;
+x << r2.x;
+
+msg="WARNING: dma_zalloc_coherent should be used for %s, instead of dma_alloc_coherent/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+type T, T2;
+expression x;
+expression E1,E2,E3;
+statement S;
+position p;
+
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+p << r3.p;
+x << r3.x;
+
+msg="%s" % (x)
+coccilib.org.print_todo(p[0], msg_safe)
+
+p << r3.p;
+x << r3.x;
+
+msg="WARNING: kzalloc_node should be used for %s, instead of kmalloc_node/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+type T, T2;
+expression x;
+expression E1,E2,E3;
+statement S;
+position p;
+
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+p << r4.p;
+x << r4.x;
+
+msg="%s" % (x)
+coccilib.org.print_todo(p[0], msg_safe)
+
+p << r4.p;
+x << r4.x;
+
+msg="WARNING: kmem_cache_zalloc should be used for %s, instead of kmem_cache_alloc/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+type T, T2;
+expression x;
+expression E1,E2;
+statement S;
+position p;
+
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+p << r5.p;
+x << r5.x;
+
+msg="%s" % (x)
+coccilib.org.print_todo(p[0], msg_safe)
+
+p << r5.p;
+x << r5.x;
+
+msg="WARNING: kmem_zalloc should be used for %s, instead of kmem_alloc/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+type T, T2;
+expression x;
+expression E1,E2,E3;
+statement S;
+position p;
+
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+p << r6.p;
+x << r6.x;
+
+msg="%s" % (x)
+coccilib.org.print_todo(p[0], msg_safe)
+
+p << r6.p;
+x << r6.x;
+
+msg="WARNING: devm_kzalloc should be used for %s, instead of devm_kmalloc/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+type T, T2;
+expression x;
+expression E1,E2;
+statement S;
+position p;
+
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+p << r7.p;
+x << r7.x;
+
+msg="%s" % (x)
+coccilib.org.print_todo(p[0], msg_safe)
+
+p << r7.p;
+x << r7.x;
+
+msg="WARNING: kvzalloc should be used for %s, instead of kvmalloc/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+type T, T2;
+expression x;
+expression E1,E2,E3;
+statement S;
+position p;
+
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+p << r8.p;
+x << r8.x;
+
+msg="%s" % (x)
+coccilib.org.print_todo(p[0], msg_safe)
+
+p << r8.p;
+x << r8.x;
+
+msg="WARNING: pci_zalloc_consistent should be used for %s, instead of pci_alloc_consistent/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+//-----------------------------------------------------------------
+type T, T2;
+expression x;
+expression E1,E2,E3;
+statement S;
+position p;
+
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+p << r9.p;
+x << r9.x;
+
+msg="%s" % (x)
+coccilib.org.print_todo(p[0], msg_safe)
+
+p << r9.p;
+x << r9.x;
+
+msg="WARNING: kvzalloc_node should be used for %s, instead of kvmalloc_node/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
--
2.7.4

[SF Markus Elfring]

We already have zero memory allocator functions to set the memory to
0 value instead of manually setting it using memset.

Thanks for your extension of this script for the semantic patch language.

Will this update suggestion get any better chances than the approach
“Script to replace allocate and memset with zalloc functions”?
https://systeme.lip6.fr/pipermail/cocci/2016-August/003510.html

+/// Use zeroing allocator rather than allocator followed by memset with 0

Do you find the shown function name list complete now?

Did you omit a name like “kvm_kvzalloc” intentionally?


How do you think about the possibility to analyse relevant source files for
functions with the mentioned property?

+(
+- x = kmalloc(E1,E2);
++ x = kzalloc(E1,E2);
+|

You suggest to use another application for the SmPL disjunction.
How do you think about to refactor this specification a bit like the following?

+(
+ x =
+- kmalloc
++ kzalloc
+ (E1, E2);
+|

+|
+- x = (T *)kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
+|

Why do you find it appropriate to omit a cast at this place while it is
preserved at other places?

Regards,
Markus

[Julia Lawall]

Post by SF Markus Elfring

We already have zero memory allocator functions to set the memory to
0 value instead of manually setting it using memset.

Thanks for your extension of this script for the semantic patch language.
Will this update suggestion get any better chances than the approach
“Script to replace allocate and memset with zalloc functions”?
https://systeme.lip6.fr/pipermail/cocci/2016-August/003510.html

I don't know why the previous version was not accepted, since I acked it.
I wonder if it had anything to do with your mostly useless comments. The
current version, however, is better because it deals with casts.

Post by SF Markus Elfring

+/// Use zeroing allocator rather than allocator followed by memset with 0

Do you find the shown function name list complete now?
Did you omit a name like “kvm_kvzalloc” intentionally?

There seems to be no such function currently.

Post by SF Markus Elfring
How do you think about the possibility to analyse relevant source files for
functions with the mentioned property?

+(
+- x = kmalloc(E1,E2);
++ x = kzalloc(E1,E2);
+|

You suggest to use another application for the SmPL disjunction.
How do you think about to refactor this specification a bit like the following?
+(
+ x =
+- kmalloc
++ kzalloc
+ (E1, E2);
+|

This would be OK here, since kzalloc has the same length as kmalloc. But
it doesn't work well when the function name changes size and the arguments
are on multiple lines.

Post by SF Markus Elfring

+|
+- x = (T *)kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
+|

Why do you find it appropriate to omit a cast at this place while it is
preserved at other places?

Because a void * value doesn't have to be explicitly casted to another
pointer type.

julia

[Himanshu Jha]

Hello Markus,

Post by SF Markus Elfring

We already have zero memory allocator functions to set the memory to
0 value instead of manually setting it using memset.

Thanks for your extension of this script for the semantic patch language.
Will this update suggestion get any better chances than the approach
“Script to replace allocate and memset with zalloc functions”?
https://systeme.lip6.fr/pipermail/cocci/2016-August/003510.html

Yes! You can check it yourself. And I didn't knew someone previously
worked on this. I was assigned the task of scrapping vmalloc/meset with
vzalloc by Luis R. Rodriguez but when I made a new rule and sent it
usptream, Julia told me find all instances and group into one.
https://lkml.org/lkml/2017/11/4/171

Post by SF Markus Elfring

+/// Use zeroing allocator rather than allocator followed by memset with 0

Do you find the shown function name list complete now?

Perhaps yes! If you find anything new then please send to patch out when
it gets merged.
You are most welcome!

Post by SF Markus Elfring
Did you omit a name like “kvm_kvzalloc” intentionally?

Hmm...I don't anything in my linux-next latest

Post by SF Markus Elfring
How do you think about the possibility to analyse relevant source files for
functions with the mentioned property?

Three rules for one functions :

(
- x = kmalloc(E1,E2);
+ x = kzalloc(E1,E2);

It is most basic case.

|
- x = (T)kmalloc(E1,E2);
+ x = (T)kzalloc(E1,E2);

This for useless pointer cast which is done implicitily.

Post by SF Markus Elfring

+(
+- x = kmalloc(E1,E2);
++ x = kzalloc(E1,E2);
+|

You suggest to use another application for the SmPL disjunction.
How do you think about to refactor this specification a bit like the following?
+(
+ x =
+- kmalloc
++ kzalloc
+ (E1, E2);
+|

Julia answered this better!

Post by SF Markus Elfring

+|
+- x = (T *)kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
+|

Why do you find it appropriate to omit a cast at this place while it is
preserved at other places?

What we can do is your best to make a perfect rule with least number of
false positives but can we ensure it to be fully perfect. The coccinelle
tool find can do its best but we are the ones to ensure that the patch
generated is absolutely correct and if it's not, then we change and
improve the existing rule perhaps!

Thanks for the feedback! I am not an experienced developer like you and
used to send out checkpatch 2 months ago and now I work under the
mentorship of Luis. Just to let you know I am just another
*kernelnewbie* ;-)

If found any queries then you can too omit/change rule to see why I
exactly did that!

Lastly, I got it 0-day tested with no errors :-)
https://github.com/himanshujha199640/linux-next/commit/24c13fe24c21a5997cbdb099d1da9f5e8e23c100

I already shared the 0-day test report with Julia and if you wish I can
send it to you too!

Please while replying also cc this to lkml mainling list so that other
relevant people can also put their opinion.

Also, Julia who is going to get it merged ? Please get it merged soon,
and after that I will start sending out the patches!

Thanks
Himanshu Jha

[Julia Lawall]

Post by Himanshu Jha
Hello Markus,

Post by SF Markus Elfring

We already have zero memory allocator functions to set the memory to
0 value instead of manually setting it using memset.

Thanks for your extension of this script for the semantic patch language.
Will this update suggestion get any better chances than the approach
“Script to replace allocate and memset with zalloc functions”?
https://systeme.lip6.fr/pipermail/cocci/2016-August/003510.html

Yes! You can check it yourself. And I didn't knew someone previously
worked on this. I was assigned the task of scrapping vmalloc/meset with
vzalloc by Luis R. Rodriguez but when I made a new rule and sent it
usptream, Julia told me find all instances and group into one.
https://lkml.org/lkml/2017/11/4/171

Post by SF Markus Elfring

+/// Use zeroing allocator rather than allocator followed by memset with 0

Do you find the shown function name list complete now?

Perhaps yes! If you find anything new then please send to patch out when
it gets merged.
You are most welcome!

Post by SF Markus Elfring
Did you omit a name like “kvm_kvzalloc” intentionally?

Hmm...I don't anything in my linux-next latest

Post by SF Markus Elfring
How do you think about the possibility to analyse relevant source files for
functions with the mentioned property?

(
- x = kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
It is most basic case.
|
- x = (T)kmalloc(E1,E2);
+ x = (T)kzalloc(E1,E2);
This for useless pointer cast which is done implicitily.

Actually, the above rule is for the case where the cast is useful. The (T
*) rule should be above this one, because that is for the case where the
cast is not needed. I don't remember if this is done correctly. Please
check.

Post by Himanshu Jha
Also, Julia who is going to get it merged ? Please get it merged soon,
and after that I will start sending out the patches!

Masahiro Yamada merges the patches. Please check also that he was in CC
in your submission.

julia

[Himanshu Jha]

Post by Julia Lawall

Post by Himanshu Jha
Hello Markus,

Post by SF Markus Elfring

We already have zero memory allocator functions to set the memory to
0 value instead of manually setting it using memset.

Thanks for your extension of this script for the semantic patch language.
Will this update suggestion get any better chances than the approach
“Script to replace allocate and memset with zalloc functions”?
https://systeme.lip6.fr/pipermail/cocci/2016-August/003510.html

Yes! You can check it yourself. And I didn't knew someone previously
worked on this. I was assigned the task of scrapping vmalloc/meset with
vzalloc by Luis R. Rodriguez but when I made a new rule and sent it
usptream, Julia told me find all instances and group into one.
https://lkml.org/lkml/2017/11/4/171

Post by SF Markus Elfring

+/// Use zeroing allocator rather than allocator followed by memset with 0

Do you find the shown function name list complete now?

Perhaps yes! If you find anything new then please send to patch out when
it gets merged.
You are most welcome!

Post by SF Markus Elfring
Did you omit a name like “kvm_kvzalloc” intentionally?

Hmm...I don't anything in my linux-next latest

Post by SF Markus Elfring
How do you think about the possibility to analyse relevant source files for
functions with the mentioned property?

(
- x = kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
It is most basic case.
|
- x = (T)kmalloc(E1,E2);
+ x = (T)kzalloc(E1,E2);
This for useless pointer cast which is done implicitily.

Actually, the above rule is for the case where the cast is useful. The (T
*) rule should be above this one, because that is for the case where the
cast is not needed. I don't remember if this is done correctly. Please
check.

Oops! Yes, I somehow hurriedly made the typo.

I already verified this! This case handles for eg:

drivers/scsi/fnic/fnic_trace.c +471

fnic_trace_buf_p = (unsigned long)vmalloc((trace_max_pages *
PAGE_SIZE));

Here, the cast it is necessary and we can't omit this cast.

Post by Julia Lawall

Post by Himanshu Jha
Also, Julia who is going to get it merged ? Please get it merged soon,
and after that I will start sending out the patches!

Masahiro Yamada merges the patches. Please check also that he was in CC
in your submission.

No, he is not!
get_maintainer didn't mention his name!
Fine I am sending that again cc'ing him!

Thanks
Himanshu Jha

[Julia Lawall]

Post by Julia Lawall

- x = (T)kmalloc(E1,E2);
+ x = (T)kzalloc(E1,E2);
This for useless pointer cast which is done implicitily.

Actually, the above rule is for the case where the cast is useful.

* Have you got any special aspects in mind here?
* How do you think about to restrict it for pointer data types?

The cast is useful when it is to a non-pointer type.

julia

[SF Markus Elfring]

Post by Julia Lawall

Post by Julia Lawall

- x = (T)kmalloc(E1,E2);
+ x = (T)kzalloc(E1,E2);
This for useless pointer cast which is done implicitily.

Actually, the above rule is for the case where the cast is useful.

* Have you got any special aspects in mind here?
* How do you think about to restrict it for pointer data types?

The cast is useful when it is to a non-pointer type.

Will it be needed then to use an other metavariable for the assignment target?

How much would you like to distinguish if an item should handle a pointer
(or not)?

Regards,
Markus

[Julia Lawall]

Post by SF Markus Elfring

Post by Julia Lawall

Post by Julia Lawall

- x = (T)kmalloc(E1,E2);
+ x = (T)kzalloc(E1,E2);
This for useless pointer cast which is done implicitily.

Actually, the above rule is for the case where the cast is useful.

* Have you got any special aspects in mind here?
* How do you think about to restrict it for pointer data types?

The cast is useful when it is to a non-pointer type.

Will it be needed then to use an other metavariable for the assignment target?
How much would you like to distinguish if an item should handle a pointer
(or not)?

The compiler will complain about an assignment between an integer and a
pointer, so one can assume that if the code is

x = (T *)kmalloc();

then x has pointer type. If it doesn't have type T *, it doesn't matter.
Casting from void * into its type or from T * into its type will have the
same effect. In other cases the cast is preserved.

julia

[SF Markus Elfring]

Post by SF Markus Elfring

Post by Julia Lawall
The cast is useful when it is to a non-pointer type.

Will it be needed then to use an other metavariable for the assignment target?
How much would you like to distinguish if an item should handle a pointer
(or not)?

The compiler will complain about an assignment between an integer and a pointer,

This tool distinguishes data types better.

so one can assume that if the code is
x = (T *)kmalloc();
then x has pointer type.

We “hope” so.

How do you think about restrict the type for an acceptable target expression?

If it doesn't have type T *, it doesn't matter.

I have got an other opinion for the corresponding specification in SmPL scripts.
I would prefer to avoid another bit of logical confusion.

Regards,
Markus

[Julia Lawall]

Post by SF Markus Elfring

Post by SF Markus Elfring

Post by Julia Lawall
The cast is useful when it is to a non-pointer type.

Will it be needed then to use an other metavariable for the assignment target?
How much would you like to distinguish if an item should handle a pointer
(or not)?

The compiler will complain about an assignment between an integer and a pointer,

This tool distinguishes data types better.

so one can assume that if the code is
x = (T *)kmalloc();
then x has pointer type.

We “hope” so.
How do you think about restrict the type for an acceptable target expression?

It's pointless:

1. Code should not be committed into the kernel if it causes build
problems, even warnings.
2. Adding type restrictions will only allow the code to be match if the
types are available. Having the types available will often require
including header files. This will massively slow down the execution time
of Coccinelle, in this case for no benefit.

julia

[Amitoj Kaur Chawla]

On Wed, Dec 27, 2017 at 12:41 AM, Himanshu Jha

Post by Himanshu Jha
Hello Markus,

Post by SF Markus Elfring

We already have zero memory allocator functions to set the memory to
0 value instead of manually setting it using memset.

Thanks for your extension of this script for the semantic patch language.
Will this update suggestion get any better chances than the approach
“Script to replace allocate and memset with zalloc functions”?
https://systeme.lip6.fr/pipermail/cocci/2016-August/003510.html

Yes! You can check it yourself. And I didn't knew someone previously
worked on this. I was assigned the task of scrapping vmalloc/meset with
vzalloc by Luis R. Rodriguez but when I made a new rule and sent it
usptream, Julia told me find all instances and group into one.
https://lkml.org/lkml/2017/11/4/171

Since Himanshu's patch is a extension to my patch, it seems to be a
better idea to go on ahead with his version.

Amitoj

Post by Himanshu Jha

Post by SF Markus Elfring

+/// Use zeroing allocator rather than allocator followed by memset with 0

Do you find the shown function name list complete now?

Perhaps yes! If you find anything new then please send to patch out when
it gets merged.
You are most welcome!

Post by SF Markus Elfring
Did you omit a name like “kvm_kvzalloc” intentionally?

Hmm...I don't anything in my linux-next latest

Post by SF Markus Elfring
How do you think about the possibility to analyse relevant source files for
functions with the mentioned property?

(
- x = kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
It is most basic case.
|
- x = (T)kmalloc(E1,E2);
+ x = (T)kzalloc(E1,E2);
This for useless pointer cast which is done implicitily.

Post by SF Markus Elfring

+(
+- x = kmalloc(E1,E2);
++ x = kzalloc(E1,E2);
+|

You suggest to use another application for the SmPL disjunction.
How do you think about to refactor this specification a bit like the following?
+(
+ x =
+- kmalloc
++ kzalloc
+ (E1, E2);
+|

Julia answered this better!

Post by SF Markus Elfring

+|
+- x = (T *)kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
+|

Why do you find it appropriate to omit a cast at this place while it is
preserved at other places?

What we can do is your best to make a perfect rule with least number of
false positives but can we ensure it to be fully perfect. The coccinelle
tool find can do its best but we are the ones to ensure that the patch
generated is absolutely correct and if it's not, then we change and
improve the existing rule perhaps!
Thanks for the feedback! I am not an experienced developer like you and
used to send out checkpatch 2 months ago and now I work under the
mentorship of Luis. Just to let you know I am just another
*kernelnewbie* ;-)
If found any queries then you can too omit/change rule to see why I
exactly did that!
Lastly, I got it 0-day tested with no errors :-)
https://github.com/himanshujha199640/linux-next/commit/24c13fe24c21a5997cbdb099d1da9f5e8e23c100
I already shared the 0-day test report with Julia and if you wish I can
send it to you too!
Please while replying also cc this to lkml mainling list so that other
relevant people can also put their opinion.
Also, Julia who is going to get it merged ? Please get it merged soon,
and after that I will start sending out the patches!
Thanks
Himanshu Jha

[Himanshu Jha]

There are many instances where memory is allocated using regular allocator
functions immediately followed by setting the allocated memory
to 0 value using memset.

We already have zero memory allocator functions to set the memory to
0 value instead of manually setting it using memset.

Therefore, use zero memory allocating functions instead of regular
memory allocators followed by memset 0 to remove redundant memset and
make the code more cleaner and also reduce the code size.

Signed-off-by: Himanshu Jha <***@gmail.com>
---
scripts/coccinelle/api/alloc/kzalloc-simple.cocci | 371 +++++++++++++++++++++-
1 file changed, 367 insertions(+), 4 deletions(-)

diff --git a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci b/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
index 52c55e4..f94888d 100644
--- a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
+++ b/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
@@ -1,5 +1,5 @@
///
-/// Use kzalloc rather than kmalloc followed by memset with 0
+/// Use zeroing allocator rather than allocator followed by memset with 0
///
/// This considers some simple cases that are common and easy to validate
/// Note in particular that there are no ...s in the rule, so all of the
@@ -8,6 +8,7 @@
// Confidence: High
// Copyright: (C) 2009-2010 Julia Lawall, Nicolas Palix, DIKU. GPLv2.
// Copyright: (C) 2009-2010 Gilles Muller, INRIA/LiP6. GPLv2.
+// Cpoyright: (C) 2017 Himanshu Jha GPLv2.
// URL: http://coccinelle.lip6.fr/rules/kzalloc.html
// Options: --no-includes --include-headers
//
@@ -28,11 +29,14 @@ virtual report
@depends on context@
type T, T2;
expression x;
-expression E1,E2;
+expression E1;
statement S;
@@

-* x = (T)kmalloc(E1,E2);
+* x = (T)\(kmalloc(E1, ...)\|vmalloc(E1)\|dma_alloc_coherent(...,E1,...)\|
+ kmalloc_node(E1, ...)\|kmem_cache_alloc(...)\|kmem_alloc(E1, ...)\|
+ devm_kmalloc(...,E1,...)\|kvmalloc(E1, ...)\|pci_alloc_consistent(...,E1,...)\|
+ kvmalloc_node(E1,...)\);
if ((x==NULL) || ...) S
* memset((T2)x,0,E1);

@@ -43,12 +47,101 @@ statement S;
@depends on patch@
type T, T2;
expression x;
-expression E1,E2;
+expression E1,E2,E3,E4;
statement S;
@@

+(
+- x = kmalloc(E1,E2);
++ x = kzalloc(E1,E2);
+|
- x = (T)kmalloc(E1,E2);
++ x = (T)kzalloc(E1,E2);
+|
+- x = (T *)kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
+|
+- x = vmalloc(E1);
++ x = vzalloc(E1);
+|
+- x = (T)vmalloc(E1);
++ x = (T)vzalloc(E1);
+|
+- x = (T *)vmalloc(E1);
++ x = vzalloc(E1);
+|
+- x = dma_alloc_coherent(E2,E1,E3,E4);
++ x = dma_zalloc_coherent(E2,E1,E3,E4);
+|
+- x = (T)dma_alloc_coherent(E2,E1,E3,E4);
++ x = (T)dma_zalloc_coherent(E2,E1,E3,E4);
+|
+- x = (T *)dma_alloc_coherent(E2,E1,E3,E4);
++ x = dma_zalloc_coherent(E2,E1,E3,E4);
+|
+- x = kmalloc_node(E1,E2,E3);
++ x = kzalloc_node(E1,E2,E3);
+|
+- x = (T)kmalloc_node(E1,E2,E3);
++ x = (T)kzalloc_node(E1,E2,E3);
+|
+- x = (T *)kmalloc_node(E1,E2,E3);
++ x = kzalloc_node(E1,E2,E3);
+|
+- x = kmem_cache_alloc(E3,E4);
++ x = kmem_cache_zalloc(E3,E4);
+|
+- x = (T)kmem_cache_alloc(E3,E4);
++ x = (T)kmem_cache_zalloc(E3,E4);
+|
+- x = (T *)kmem_cache_alloc(E3,E4);
++ x = kmem_cache_zalloc(E3,E4);
+|
+- x = kmem_alloc(E1,E2);
++ x = kmem_zalloc(E1,E2);
+|
+- x = (T)kmem_alloc(E1,E2);
++ x = (T)kmem_zalloc(E1,E2);
+|
+- x = (T *)kmem_alloc(E1,E2);
++ x = kmem_zalloc(E1,E2);
+|
+- x = devm_kmalloc(E2,E1,E3);
++ x = devm_kzalloc(E2,E1,E3);
+|
+- x = (T)devm_kmalloc(E2,E1,E3);
++ x = (T)devm_kzalloc(E2,E1,E3);
+|
+- x = (T *)devm_kmalloc(E2,E1,E3);
++ x = devm_kzalloc(E2,E1,E3);
+|
+- x = kvmalloc(E1,E2);
++ x = kvzalloc(E1,E2);
+|
+- x = (T)kvmalloc(E1,E2);
++ x = (T)kvzalloc(E1,E2);
+|
+- x = (T *)kvmalloc(E1,E2);
++ x = kvzalloc(E1,E2);
+|
+- x = pci_alloc_consistent(E2,E1,E3);
++ x = pci_zalloc_consistent(E2,E1,E3);
+|
+- x = (T)pci_alloc_consistent(E2,E1,E3);
++ x = (T)pci_zalloc_consistent(E2,E1,E3);
+|
+- x = (T *)pci_alloc_consistent(E2,E1,E3);
++ x = pci_zalloc_consistent(E2,E1,E3);
+|
+- x = kvmalloc_node(E1,E2,E3);
++ x = kvzalloc_node(E1,E2,E3);
+|
+- x = (T)kvmalloc_node(E1,E2,E3);
++ x = (T)kvzalloc_node(E1,E2,E3);
+|
+- x = (T *)kvmalloc_node(E1,E2,E3);
++ x = kvzalloc_node(E1,E2,E3);
+)
if ((x==NULL) || ...) S
- memset((T2)x,0,E1);

@@ -84,3 +177,273 @@ x << r.x;

msg="WARNING: kzalloc should be used for %s, instead of kmalloc/memset" % (x)
coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+@r1 depends on org || report@
+type T, T2;
+expression x;
+expression E1;
+statement S;
+position p;
+@@
+
+ x = (T)***@p(E1);
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+@script:python depends on org@
+p << r1.p;
+x << r1.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r1.p;
+x << r1.x;
+@@
+
+msg="WARNING: vzalloc should be used for %s, instead of vmalloc/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+@r2 depends on org || report@
+type T, T2;
+expression x;
+expression E1,E2,E3,E4;
+statement S;
+position p;
+@@
+
+ x = (T)***@p(E2,E1,E3,E4);
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+@script:python depends on org@
+p << r2.p;
+x << r2.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r2.p;
+x << r2.x;
+@@
+
+msg="WARNING: dma_zalloc_coherent should be used for %s, instead of dma_alloc_coherent/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+@r3 depends on org || report@
+type T, T2;
+expression x;
+expression E1,E2,E3;
+statement S;
+position p;
+@@
+
+ x = (T)***@p(E1,E2,E3);
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+@script:python depends on org@
+p << r3.p;
+x << r3.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r3.p;
+x << r3.x;
+@@
+
+msg="WARNING: kzalloc_node should be used for %s, instead of kmalloc_node/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+@r4 depends on org || report@
+type T, T2;
+expression x;
+expression E1,E2,E3;
+statement S;
+position p;
+@@
+
+ x = (T)***@p(E2,E3);
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+@script:python depends on org@
+p << r4.p;
+x << r4.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r4.p;
+x << r4.x;
+@@
+
+msg="WARNING: kmem_cache_zalloc should be used for %s, instead of kmem_cache_alloc/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+@r5 depends on org || report@
+type T, T2;
+expression x;
+expression E1,E2;
+statement S;
+position p;
+@@
+
+ x = (T)***@p(E1,E2);
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+@script:python depends on org@
+p << r5.p;
+x << r5.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r5.p;
+x << r5.x;
+@@
+
+msg="WARNING: kmem_zalloc should be used for %s, instead of kmem_alloc/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+@r6 depends on org || report@
+type T, T2;
+expression x;
+expression E1,E2,E3;
+statement S;
+position p;
+@@
+
+ x = (T)***@p(E2,E1,E3);
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+@script:python depends on org@
+p << r6.p;
+x << r6.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r6.p;
+x << r6.x;
+@@
+
+msg="WARNING: devm_kzalloc should be used for %s, instead of devm_kmalloc/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+@r7 depends on org || report@
+type T, T2;
+expression x;
+expression E1,E2;
+statement S;
+position p;
+@@
+
+ x = (T)***@p(E1,E2);
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+@script:python depends on org@
+p << r7.p;
+x << r7.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r7.p;
+x << r7.x;
+@@
+
+msg="WARNING: kvzalloc should be used for %s, instead of kvmalloc/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+
+//-----------------------------------------------------------------
+@r8 depends on org || report@
+type T, T2;
+expression x;
+expression E1,E2,E3;
+statement S;
+position p;
+@@
+
+ x = (T)***@p(E2,E1,E3);
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+@script:python depends on org@
+p << r8.p;
+x << r8.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r8.p;
+x << r8.x;
+@@
+
+msg="WARNING: pci_zalloc_consistent should be used for %s, instead of pci_alloc_consistent/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+//-----------------------------------------------------------------
+@r9 depends on org || report@
+type T, T2;
+expression x;
+expression E1,E2,E3;
+statement S;
+position p;
+@@
+
+ x = (T)***@p(E1,E2,E3);
+ if ((x==NULL) || ...) S
+ memset((T2)x,0,E1);
+
+@script:python depends on org@
+p << r9.p;
+x << r9.x;
+@@
+
+msg="%s" % (x)
+msg_safe=msg.replace("[","@(").replace("]",")")
+coccilib.org.print_todo(p[0], msg_safe)
+
+@script:python depends on report@
+p << r9.p;
+x << r9.x;
+@@
+
+msg="WARNING: kvzalloc_node should be used for %s, instead of kvmalloc_node/memset" % (x)
+coccilib.report.print_report(p[0], msg)
+

[Fabio Estevam]

On Tue, Dec 26, 2017 at 5:49 PM, Himanshu Jha

diff --git a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci b/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
index 52c55e4..f94888d 100644
--- a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
+++ b/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
@@ -1,5 +1,5 @@
///
-/// Use kzalloc rather than kmalloc followed by memset with 0
+/// Use zeroing allocator rather than allocator followed by memset with 0
///
/// This considers some simple cases that are common and easy to validate
/// Note in particular that there are no ...s in the rule, so all of the
@@ -8,6 +8,7 @@
// Confidence: High
// Copyright: (C) 2009-2010 Julia Lawall, Nicolas Palix, DIKU. GPLv2.
// Copyright: (C) 2009-2010 Gilles Muller, INRIA/LiP6. GPLv2.
+// Cpoyright: (C) 2017 Himanshu Jha GPLv2.

Typo here in "Copyright"

[Himanshu Jha]

Post by Fabio Estevam
On Tue, Dec 26, 2017 at 5:49 PM, Himanshu Jha

diff --git a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci b/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
index 52c55e4..f94888d 100644
--- a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
+++ b/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
@@ -1,5 +1,5 @@
///
-/// Use kzalloc rather than kmalloc followed by memset with 0
+/// Use zeroing allocator rather than allocator followed by memset with 0
///
/// This considers some simple cases that are common and easy to validate
/// Note in particular that there are no ...s in the rule, so all of the
@@ -8,6 +8,7 @@
// Confidence: High
// Copyright: (C) 2009-2010 Julia Lawall, Nicolas Palix, DIKU. GPLv2.
// Copyright: (C) 2009-2010 Gilles Muller, INRIA/LiP6. GPLv2.
+// Cpoyright: (C) 2017 Himanshu Jha GPLv2.

Typo here in "Copyright"

Oops!
Thank you so much!

I will send v2 patch with other necessary changes that Julia pointed me
out in the SmPL rule.

Himanshu Jha