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 <[email protected]>
---
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)vmalloc@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)dma_alloc_coherent@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)kmalloc_node@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)kmem_cache_alloc@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)kmem_alloc@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)devm_kmalloc@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)kvmalloc@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)pci_alloc_consistent@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)kvmalloc_node@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)
+
--
2.7.4
On Wed, 27 Dec 2017, Himanshu Jha wrote:
> 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 <[email protected]>
Acked-by: Julia Lawall <[email protected]>
> ---
>
> 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)vmalloc@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)dma_alloc_coherent@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)kmalloc_node@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)kmem_cache_alloc@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)kmem_alloc@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)devm_kmalloc@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)kvmalloc@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)pci_alloc_consistent@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)kvmalloc_node@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)
> +
> --
> 2.7.4
>
>
2017-12-27 6:40 GMT+09:00 Himanshu Jha <[email protected]>:
> 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 <[email protected]>
> ---
>
> 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.
>
...
> +@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)
> +
I removed the blank line at EOF,
then applied to linux-kbuild/misc. Thanks!
--
Best Regards
Masahiro Yamada
On Sat, 30 Dec 2017, Masahiro Yamada wrote:
> 2017-12-27 6:40 GMT+09:00 Himanshu Jha <[email protected]>:
> > 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 <[email protected]>
> > ---
> >
> > 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.
> >
>
> ...
>
> > +@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)
> > +
>
>
> I removed the blank line at EOF,
> then applied to linux-kbuild/misc. Thanks!
Thanks!
julia
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://lkml.kernel.org/r/<[email protected]>
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)?
Regards,
Markus
On Tue, 2 Jan 2018, SF Markus Elfring wrote:
> 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://lkml.kernel.org/r/<[email protected]>
> 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
>> 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?
Regards,
Markus
On Tue, 2 Jan 2018, SF Markus Elfring wrote:
> >> 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
>> * 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,
The involved contributors have got different views if the available script
remains “simple” enough at the moment.
> becaue it doesn't account for the possibility of many statement between
> the alloc and the memset
* How close should these function call be kept together?
* Which additional statements would you tolerate between them?
> and it doesn't account for different ways of expressing the size between
> the two calls.
Would you like to get any extensions there?
> If you want to be more general than kzalloc, then perhaps
> zalloc-simple.cocci would be ok.
Will other suffixes be safer for a permanent file name so that confusion
could be avoided around different expectations for “simplicity”?
Regards,
Markus
From: Markus Elfring <[email protected]>
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.
Signed-off-by: Markus Elfring <[email protected]>
---
...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
--
2.15.1
On Wed, 3 Jan 2018, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> 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.
>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> ...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
>> 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.
> The name is too long
How would you like to express the provided functionality in a “permanent” file name?
> and should not contain spaces.
May names contain space characters generally for Linux files?
Regards,
Markus
On Wed, 3 Jan 2018, SF Markus Elfring wrote:
> >> 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.
>
>
> > 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.
>
>
> > 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
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>> 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?
> 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.
>> 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.
> But personally, I really dislike them,
Interesting …
> and I will nack any patch that proposes to use one.
Would you insist to replace such “special characters” by dashes or underscores?
Regards,
Markus
On Wed, 3 Jan 2018, SF Markus Elfring wrote:
> >> 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?
>
>
> > 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.
>
>
> >> 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.
>
>
> > But personally, I really dislike them,
>
> Interesting …
>
>
> > 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
>> Would you insist to replace such “special characters” by dashes or underscores?
>
> Yes. But I will also nack any very long name.
Will any other artificial identifier become appropriate if it would be too hard
to achieve consensus on a more meaningful description in the file name?
Regards,
Markus
>> 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?
Regards,
Markus
On Thu, 4 Jan 2018, SF Markus Elfring wrote:
> >> 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
>> Would you find the file name selection “zalloc-extra_changes1.cocci” acceptable?
>
> No.
Next try then …
> I do't know what the extra changes are
This script for the semantic patch language was designed in the way
that more source code would be adjusted (or just reformatted)
than I see as an essential transformation for the replacement
of function calls in the shown case.
Example:
…
(
- x = kmalloc(E1,E2);
+ x = kzalloc(E1,E2);
|
…
We are discussing consequences from the specification of a different
change granularity for a while.
I hope that another SmPL script variant could be integrated
which will work with a higher precision then so that involved
software developers can choose the preferred transformation approach
also on their own.
> and I don't know what the number 1 means.
How do you think about to start with numbering for subsequent
SmPL scripts?
Regards,
Markus
> If you want to be more general than kzalloc, then perhaps
> zalloc-simple.cocci would be ok.
Are you going to commit such a file name adjustment when it seems
that you would not like to accept any other suggestion there?
Regards,
Markus
> I removed the blank line at EOF,
> then applied to linux-kbuild/misc.
This script for the semantic patch language was extended. Now I find
that this development version contains some update candidates.
Implementation details can become safer in a few software design directions.
I suggest to start with another look at the distinction between pointer data types
and the other types. How much can corresponding source code review result in
further software improvements?
Regards,
Markus
> 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.
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
“…
Furthermore, @ should not be used in this code.
Spatch scans the script code for the next @ and considers that to be the
beginning of the next rule, even if @ occurs within e.g., a comment.
…”
See also:
Configuration or escaping of @ characters for embedded programming language scripts
https://github.com/coccinelle/coccinelle/issues/36
Regards,
Markus
On Fri, 19 Jan 2018, SF Markus Elfring wrote:
> > 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
>
> “…
> Furthermore, @ should not be used in this code.
> Spatch scans the script code for the next @ and considers that to be the
> beginning of the next rule, even if @ occurs within e.g., a comment.
> …”
I guess the conclusion is that it woks in strings (which are pretty
universal) and not in comments (which are language specific).
julia
>
> See also:
> Configuration or escaping of @ characters for embedded programming language scripts
> https://github.com/coccinelle/coccinelle/issues/36
>
> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> So it works, but you are complaining anyway?
I dare to point a contradiction out between two information sources
once more.
Is this SmPL script really working in intended way already?
> I guess the conclusion is that it woks in strings (which are pretty
> universal) and not in comments (which are language specific).
Does the documentation need another update for the Coccinelle software
to achieve the desired correctness?
Which software design direction will get priority in such an use case?
Regards,
Markus
> So it works, but you are complaining anyway?
How serious do you interpret such information in the SmPL manual?
>> https://github.com/coccinelle/coccinelle/blob/bf1c6a5869dd324f5faeeaa3a12d57270e478b21/docs/manual/cocci_syntax.tex#L50
>>
>> “…
>> Furthermore, @ should not be used in this code.
…
>> …”
>
> I guess the conclusion is that it woks in strings (which are pretty
> universal) and not in comments (which are language specific).
Would you like to achieve any safer data processing for potentially “reserved” characters?
Regards,
Markus
>> Will the rule set be more consistent then?
>
> If E1 is not bound by the kem_cache_alloc rule, then it will match anything.
How much was such a software behaviour intended by the discussed SmPL script?
> The user can check if it is appropriate.
How does such an information fit to expectations for safe source code analysis?
> Another option would be to use the type of the variable storing the result
> of the call to compute the expected size.
How would this suggestion help here?
Regards,
Markus
> 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?
Regards,
Markus
On Wed, 31 Jan 2018, SF Markus Elfring wrote:
> > 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
From: Markus Elfring <[email protected]>
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".
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")
Signed-off-by: Markus Elfring <[email protected]>
---
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)
-//-----------------------------------------------------------------
-@r4 depends on org || report@
-type T, T2;
-expression x;
-expression E1,E2,E3;
-statement S;
-position p;
-@@
-
- x = (T)kmem_cache_alloc@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;
--
2.16.1
On Thu, 1 Feb 2018, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> 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")
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> 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)
>
> -//-----------------------------------------------------------------
> -@r4 depends on org || report@
> -type T, T2;
> -expression x;
> -expression E1,E2,E3;
> -statement S;
> -position p;
> -@@
> -
> - x = (T)kmem_cache_alloc@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;
> --
> 2.16.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>> 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.
> 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?
* 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?
Regards,
Markus
On Thu, 1 Feb 2018, SF Markus Elfring wrote:
> >> 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.
>
>
> > 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
>> * 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.
I have got other source code analysis expectations there.
This SmPL script contains the tag “Confidence: High”.
> The user can look at the answer and see if it is a false positive or not.
The situation is questionable because of a specific software design detail.
Unsafe source code search patterns could be stored under other script names
(or even different directories).
How do you think about to move the SmPL code (which I proposed for deletion)
to another script if you would insist to preserve it?
> Furthermore, I told you how to address this function so that the size
> issue would be taken care of.
You offered another bit of information.
I find your interpretation of this aspect also unclear at the moment.
> That is the patch that I would accept.
Are there any better development solutions left over?
>> * 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.
I am trying again to resolve corresponding communication difficulties.
Thus I suggest to take another look at the following SmPL code fragment.
…
kmalloc_node(E1, ...)\|kmem_cache_alloc(...)\|kmem_alloc(E1, ...)\|
…
* memset((T2)x,0,E1);
…
How many constraints should be considered for function parameters here?
Regards,
Markus
>> * 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.
Do you care to split SmPL rules by their confidence category in such an use case?
Regards,
Markus