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

[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>
---

v2:
-fix typo in copyright.
-move all the (T *) disjunction cases before (T) as (T) matches any cast
at all including (T *) ones which is not desirable.

scripts/coccinelle/api/alloc/kzalloc-simple.cocci | 373 +++++++++++++++++++++-
1 file changed, 368 insertions(+), 5 deletions(-)

diff --git a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci b/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
index 52c55e4..d08d526 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.
+// Copyright: (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 = (T)kmalloc(E1,E2);
+(
+- x = kmalloc(E1,E2);
++ x = kzalloc(E1,E2);
+|
+- x = (T *)kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
+|
+- x = (T)kmalloc(E1,E2);
++ x = (T)kzalloc(E1,E2);
+|
+- x = vmalloc(E1);
++ x = vzalloc(E1);
+|
+- x = (T *)vmalloc(E1);
++ x = vzalloc(E1);
+|
+- x = (T)vmalloc(E1);
++ x = (T)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 = 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 = kmalloc_node(E1,E2,E3);
++ x = kzalloc_node(E1,E2,E3);
+|
+- x = (T *)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 = kmem_cache_alloc(E3,E4);
++ x = kmem_cache_zalloc(E3,E4);
+|
+- x = (T *)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 = kmem_alloc(E1,E2);
++ x = kmem_zalloc(E1,E2);
+|
+- x = (T *)kmem_alloc(E1,E2);
++ x = kmem_zalloc(E1,E2);
+|
+- x = (T)kmem_alloc(E1,E2);
++ x = (T)kmem_zalloc(E1,E2);
+|
+- x = devm_kmalloc(E2,E1,E3);
++ x = devm_kzalloc(E2,E1,E3);
+|
+- x = (T *)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 = kvmalloc(E1,E2);
++ x = kvzalloc(E1,E2);
+|
+- x = (T *)kvmalloc(E1,E2);
++ x = kvzalloc(E1,E2);
+|
+- x = (T)kvmalloc(E1,E2);
++ x = (T)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 = pci_zalloc_consistent(E2,E1,E3);
+|
+- x = (T)pci_alloc_consistent(E2,E1,E3);
++ x = (T)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 = kvzalloc_node(E1,E2,E3);
+|
+- x = (T)kvmalloc_node(E1,E2,E3);
++ x = (T)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)
+

[Julia Lawall]

Post by 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.
---
-fix typo in copyright.
-move all the (T *) disjunction cases before (T) as (T) matches any cast
at all including (T *) ones which is not desirable.
scripts/coccinelle/api/alloc/kzalloc-simple.cocci | 373 +++++++++++++++++++++-
1 file changed, 368 insertions(+), 5 deletions(-)
diff --git a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci b/scripts/coccinelle/api/alloc/kzalloc-simple.cocci
index 52c55e4..d08d526 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.
+// Copyright: (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 = (T)kmalloc(E1,E2);
+(
+- x = kmalloc(E1,E2);
++ x = kzalloc(E1,E2);
+|
+- x = (T *)kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
+|
+- x = (T)kmalloc(E1,E2);
++ x = (T)kzalloc(E1,E2);
+|
+- x = vmalloc(E1);
++ x = vzalloc(E1);
+|
+- x = (T *)vmalloc(E1);
++ x = vzalloc(E1);
+|
+- x = (T)vmalloc(E1);
++ x = (T)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 = 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 = kmalloc_node(E1,E2,E3);
++ x = kzalloc_node(E1,E2,E3);
+|
+- x = (T *)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 = kmem_cache_alloc(E3,E4);
++ x = kmem_cache_zalloc(E3,E4);
+|
+- x = (T *)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 = kmem_alloc(E1,E2);
++ x = kmem_zalloc(E1,E2);
+|
+- x = (T *)kmem_alloc(E1,E2);
++ x = kmem_zalloc(E1,E2);
+|
+- x = (T)kmem_alloc(E1,E2);
++ x = (T)kmem_zalloc(E1,E2);
+|
+- x = devm_kmalloc(E2,E1,E3);
++ x = devm_kzalloc(E2,E1,E3);
+|
+- x = (T *)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 = kvmalloc(E1,E2);
++ x = kvzalloc(E1,E2);
+|
+- x = (T *)kvmalloc(E1,E2);
++ x = kvzalloc(E1,E2);
+|
+- x = (T)kvmalloc(E1,E2);
++ x = (T)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 = pci_zalloc_consistent(E2,E1,E3);
+|
+- x = (T)pci_alloc_consistent(E2,E1,E3);
++ x = (T)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 = kvzalloc_node(E1,E2,E3);
+|
+- x = (T)kvmalloc_node(E1,E2,E3);
++ x = (T)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

[Masahiro Yamada]

Post by 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.
---
-fix typo in copyright.
-move all the (T *) disjunction cases before (T) as (T) matches any cast
at all including (T *) ones which is not desirable.

...

Post by Himanshu Jha
+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)
+

I removed the blank line at EOF,
then applied to linux-kbuild/misc. Thanks!

[Julia Lawall]

Post by Masahiro Yamada

Post by 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.
---
-fix typo in copyright.
-move all the (T *) disjunction cases before (T) as (T) matches any cast
at all including (T *) ones which is not desirable.

...

Post by Himanshu Jha
+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)
+

I removed the blank line at EOF,
then applied to linux-kbuild/misc. Thanks!

Thanks!

julia

[Julia Lawall]

Hello,
A script for the semantic patch language was extended in significant ways.
[PATCH v2] Coccinelle: kzalloc-simple: Add “all” zero allocating functions
https://lkml.org/lkml/2017/12/26/182
https://patchwork.kernel.org/patch/10133277/
https://systeme.lip6.fr/pipermail/cocci/2017-December/004783.html
Now I find that it became more advanced than the previous version.
How do you think about to update also the corresponding file name
(instead of keeping the word “simple” there)?

Why not send a patch for it yourself?

julia

[Julia Lawall]

Post by Julia Lawall

Now I find that it became more advanced than the previous version.
How do you think about to update also the corresponding file name
(instead of keeping the word “simple” there)?

Why not send a patch for it yourself?

* I would like to check your views around renaming of such files.
* I am unsure which name will be better finally.
Would we like to achieve another permalink here?

Actually, according to th original name choice it is stillsimple, becaue
it doesn't account for the possibility of many statement between the alloc
and the memset and it doesn't account for different ways of expressing the
size between the two calls.

If you want to be more general than kzalloc, then perhaps
zalloc-simple.cocci would be ok.

julia

[Julia Lawall]

Date: Wed, 3 Jan 2018 12:43:45 +0100
A script for the semantic patch language was extended
in a significant way.
An other file name is more appropriate then to indicate
the provided functionality. Thus rename this file.
---
...kzalloc-simple.cocci => use zalloc functions with extra changes.cocci} | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename scripts/coccinelle/api/alloc/{kzalloc-simple.cocci => use zalloc functions with extra changes.cocci} (100%)
diff --git a/scripts/coccinelle/api/alloc/kzalloc-simple.cocci b/scripts/coccinelle/api/alloc/use zalloc functions with extra changes.cocci
similarity index 100%
rename from scripts/coccinelle/api/alloc/kzalloc-simple.cocci
rename to scripts/coccinelle/api/alloc/use zalloc functions with extra changes.cocci

NACK. The name is too long and should not contain spaces.

julia

[Julia Lawall]

Post by Julia Lawall

rename from scripts/coccinelle/api/alloc/kzalloc-simple.cocci
rename to scripts/coccinelle/api/alloc/use zalloc functions with extra changes.cocci

NACK.

It seems that we need a few more tries to achieve the desired consensus.

Post by Julia Lawall
The name is too long

How would you like to express the provided functionality in a
“permanent” file name?

I have not idea what a permanent file name is. The current name could be
better without the leading k, but otherwise I think it is fine.

Post by Julia Lawall
and should not contain spaces.

May names contain space characters generally for Linux files?

I have never seen a file name with spaces in the Linux kernel, but I don't
know an easy way to check for that. But personally, I really dislike
them, and I will nack any patch that proposes to use one.

julia

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Julia Lawall]

Post by Julia Lawall

How would you like to express the provided functionality in a
“permanent” file name?

I have not idea what a permanent file name is.

Are you used to the selection of permalinks?

Post by Julia Lawall
The current name could be better without the leading k,
but otherwise I think it is fine.

I suggest to express more details than to keep the word “simple” there.

Post by Julia Lawall

May names contain space characters generally for Linux files?

I have never seen a file name with spaces in the Linux kernel,
but I don't know an easy way to check for that.

Corresponding search tools can determine this if it would be desired.

Post by Julia Lawall
But personally, I really dislike them,

Interesting 


Post by Julia Lawall
and I will nack any patch that proposes to use one.

Would you insist to replace such “special characters” by dashes or underscores?

Yes. But I will also nack any very long name.

julia

[Julia Lawall]

Post by Julia Lawall

Would you insist to replace such “special characters” by dashes or underscores?

Yes. But I will also nack any very long name.

Would you find the the file name selection “zalloc-extra_changes1.cocci” acceptable?

No. I do't know what the extra changes are and I don't know what the
number 1 means.

julia

[Julia Lawall]

Post by Masahiro Yamada
I removed the blank line at EOF,
then applied to linux-kbuild/misc.

This script for the semantic patch language is using the at sign within string
literals for Python code.
It is nice when this character seems to work also with the current software.

So it works, but you are complaining anyway?

How does its usage fit to the following information in the SmPL manual?
https://github.com/coccinelle/coccinelle/blob/bf1c6a5869dd324f5faeeaa3a12d57270e478b21/docs/manual/cocci_syntax.tex#L50
“


”

I guess the conclusion is that it woks in strings (which are pretty
universal) and not in comments (which are language specific).

julia

https://github.com/coccinelle/coccinelle/issues/36
Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Julia Lawall]

Post by Masahiro Yamada
I removed the blank line at EOF,
then applied to linux-kbuild/misc.

I have taken another look at this script for the semantic patch language.
I imagined that I could refactor the shown SmPL disjunctions a bit.
But I noticed then that these SmPL rules contain a development mistake.
The deletion for a call of the function “memset” depends on the specification
that a size determination is passed by the expression “E1”.
The function “kmem_cache_alloc” was specified despite of the technical detail
that this function does not get a parameter passed which would correspond
to such a size information.
https://elixir.free-electrons.com/linux/v4.15/source/tools/testing/radix-tree/linux/slab.h#L14
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/tools/testing/radix-tree/linux/slab.h?id=537659c08a7da298aa748854f65f2aa1f31b1378#n14
Thus I suggest to remove it from the first two SmPL rules and omit the rule “r4”.
Will the rule set be more consistent then?

If E1 is not bound by the kem_cache_alloc rule, then it will match
anything. The user can check if it is appropriate.

Another option would be to use the type of the variable storing the result
of the call to compute the expected size. Feel free to send a patch.

julia

[Julia Lawall]

Date: Thu, 1 Feb 2018 10:20:47 +0100
The deletion for a call of the function "memset" depends on
the specification that a size determination is passed by
the expression "E1".
The function "kmem_cache_alloc" was specified despite of the technical
detail that this function does not get a parameter passed which would
correspond to such a size information.
Thus remove it from the first two SmPL rules and omit the rule "r4".

Nack. It should be supported by the size determined in another way.

julia

Link: https://elixir.free-electrons.com/linux/v4.15/source/tools/testing/radix-tree/linux/slab.h#L14
Link: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/tools/testing/radix-tree/linux/slab.h?id=f26e52e08ab8e56f528ac14aa7929b3477de5616#n14
Fixes: 5e2d9da5b9ba350a4f13bd3b255be046bcf86465 ("Coccinelle: kzalloc-simple: Add all zero allocating functions")
---
scripts/coccinelle/api/alloc/zalloc-simple.cocci | 41 +-----------------------
1 file changed, 1 insertion(+), 40 deletions(-)
diff --git a/scripts/coccinelle/api/alloc/zalloc-simple.cocci b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
index 92b20913055f..3bee6cdd99ea 100644
--- a/scripts/coccinelle/api/alloc/zalloc-simple.cocci
+++ b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
@@ -34,7 +34,7 @@ statement S;
@@
* x = (T)\(kmalloc(E1, ...)\|vmalloc(E1)\|dma_alloc_coherent(...,E1,...)\|
- kmalloc_node(E1, ...)\|kmem_cache_alloc(...)\|kmem_alloc(E1, ...)\|
+ kmalloc_node(E1, ...)\|kmem_alloc(E1, ...)\|
devm_kmalloc(...,E1,...)\|kvmalloc(E1, ...)\|pci_alloc_consistent(...,E1,...)\|
kvmalloc_node(E1,...)\);
if ((x==NULL) || ...) S
@@ -88,15 +88,6 @@ statement S;
- x = (T)kmalloc_node(E1,E2,E3);
+ x = (T)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 = kmem_cache_zalloc(E3,E4);
-|
-- x = (T)kmem_cache_alloc(E3,E4);
-+ x = (T)kmem_cache_zalloc(E3,E4);
-|
- x = kmem_alloc(E1,E2);
+ x = kmem_zalloc(E1,E2);
|
@@ -268,36 +259,6 @@ 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)
-
//-----------------------------------------------------------------
@r5 depends on org || report@
type T, T2;
--
2.16.1
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Julia Lawall]

Post by Julia Lawall

The function "kmem_cache_alloc" was specified despite of the technical
detail that this function does not get a parameter passed which would
correspond to such a size information.
Thus remove it from the first two SmPL rules and omit the rule "r4".

Nack.

I find such a rejection surprising once more.

Post by Julia Lawall
It should be supported by the size determined in another way.

I am curious on how our different views could be clarified further
for this special software situation.
* Do we agree that a proper size determination is essential for every
condition in the discussed SmPL rules together with forwarding
this information?

No. I don't mind a few false positives. The user can look at the answer
and see if it is a false positive or not.

Furthermore, I told you how to address this function so that the size
issue would be taken care of. That is the patch that I would accept.

* How can a name be ever relevant (within the published SmPL approach)
for a function when it was designed in the way that it should generally
work without a size parameter?

No idea what this means.

julia