linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions
@ 2016-08-01  7:02 Amitoj Kaur Chawla
  2016-08-01 11:23 ` [Cocci] " SF Markus Elfring
  2017-03-03 10:34 ` [Cocci] [PATCH v3] " SF Markus Elfring
  0 siblings, 2 replies; 14+ messages in thread
From: Amitoj Kaur Chawla @ 2016-08-01  7:02 UTC (permalink / raw)
  To: Julia.Lawall, Gilles.Muller, nicolas.palix, mmarek, cocci, linux-kernel

This script finds instances of allocate and memset which can be
replaced with a direct call to zalloc equivalent of a function.

Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
---
Changes in v2:
        -Modified commit message and subject
Changes in v3:
        -Modified comment

 scripts/coccinelle/api/zalloc.cocci | 556 ++++++++++++++++++++++++++++++++++++
 1 file changed, 556 insertions(+)
 create mode 100644 scripts/coccinelle/api/zalloc.cocci

diff --git a/scripts/coccinelle/api/zalloc.cocci b/scripts/coccinelle/api/zalloc.cocci
new file mode 100644
index 0000000..4f94e43
--- /dev/null
+++ b/scripts/coccinelle/api/zalloc.cocci
@@ -0,0 +1,556 @@
+/// Prefer zalloc functions instead of using allocate and memset.  
+///
+// Confidence: High
+// Copyright: (C) 2016 Amitoj Kaur Chawla
+
+virtual patch
+virtual context
+virtual org
+virtual report
+
+@dma1 depends on patch && !context && !org && !report@
+type T;
+T *d;
+statement S;
+@@
+
+        d =
+-            dma_pool_alloc
++            dma_pool_zalloc
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(T));
+
+@dma2 depends on patch && !context && !org && !report@
+expression d;
+statement S;
+@@
+
+        d =
+-            dma_pool_alloc
++            dma_pool_zalloc
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(*d));
+@vz1 depends on patch && !context && !org && !report@
+type T;
+T *d;
+statement S;
+@@
+
+        d =
+-            vmalloc
++            vzalloc
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(T));
+
+@vz2 depends on patch && !context && !org && !report@
+expression d;
+statement S;
+@@
+
+        d =
+-            vmalloc
++            vzalloc
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(*d));
+@vzn1 depends on patch && !context && !org && !report@
+type T;
+T *d;
+statement S;
+@@
+
+        d =
+-            vmalloc_node
++            vzalloc_node
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(T));
+
+@vzn2 depends on patch && !context && !org && !report@
+expression d;
+statement S;
+@@
+
+        d =
+-            vmalloc_node
++            vzalloc_node
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(*d));
+@pci1 depends on patch && !context && !org && !report@
+type T;
+T *d;
+statement S;
+@@
+
+        d =
+-            pci_alloc_consistent
++            pci_zalloc_consistent
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(T));
+
+@pci2 depends on patch && !context && !org && !report@
+expression d;
+statement S;
+@@
+
+        d =
+-            pci_alloc_consistent
++            pci_zalloc_consistent
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(*d));
+@kmem1 depends on patch && !context && !org && !report@
+type T;
+T *d;
+statement S;
+@@
+
+        d =
+-            kmem_cache_alloc
++            kmem_cache_zalloc
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(T));
+
+@kmem2 depends on patch && !context && !org && !report@
+expression d;
+statement S;
+@@
+
+        d =
+-            kmem_cache_alloc
++            kmem_cache_zalloc
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(*d));
+@dma3 depends on patch && !context && !org && !report@
+type T;
+T *d;
+statement S;
+@@
+
+        d =
+-            dma_alloc_coherent
++            dma_zalloc_coherent
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(T));
+
+@dma4 depends on patch && !context && !org && !report@
+expression d;
+statement S;
+@@
+
+        d =
+-            dma_alloc_coherent
++            dma_zalloc_coherent
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(*d));
+@acpi1 depends on patch && !context && !org && !report@
+type T;
+T *d;
+statement S;
+@@
+
+        d =
+-            acpi_os_allocate
++            acpi_os_allocate_zeroed
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(T));
+
+@acpi2 depends on patch && !context && !org && !report@
+expression d;
+statement S;
+@@
+
+        d =
+-            acpi_os_allocate
++            acpi_os_allocate_zeroed
+             (...);
+        if (!d) S
+-       memset(d, 0, sizeof(*d));
+
+// ----------------------------------------------------------------------------
+
+@dma1_context depends on !patch && (context || org || report)@
+type T;
+statement S;
+T *d;
+position j0;
+@@
+
+        d@j0 =
+*             dma_pool_alloc
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(T));
+
+@dma2_context depends on !patch && (context || org || report)@
+statement S;
+expression d;
+position j0;
+@@
+
+        d@j0 =
+*             dma_pool_alloc
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(*d));
+
+@vz1_context depends on !patch && (context || org || report)@
+type T;
+statement S;
+T *d;
+position j0;
+@@
+
+        d@j0 =
+*             vmalloc
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(T));
+
+@vz2_context depends on !patch && (context || org || report)@
+statement S;
+expression d;
+position j0;
+@@
+
+        d@j0 =
+*             vmalloc
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(*d));
+
+@vzn1_context depends on !patch && (context || org || report)@
+type T;
+statement S;
+T *d;
+position j0;
+@@
+
+        d@j0 =
+*             vmalloc_node
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(T));
+
+@vzn2_context depends on !patch && (context || org || report)@
+statement S;
+expression d;
+position j0;
+@@
+
+        d@j0 =
+*             vmalloc_node
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(*d));
+
+@pci1_context depends on !patch && (context || org || report)@
+type T;
+statement S;
+T *d;
+position j0;
+@@
+
+        d@j0 =
+*             pci_alloc_consistent
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(T));
+
+@pci2_context depends on !patch && (context || org || report)@
+statement S;
+expression d;
+position j0;
+@@
+
+        d@j0 =
+*             pci_alloc_consistent
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(*d));
+
+@kmem1_context depends on !patch && (context || org || report)@
+type T;
+statement S;
+T *d;
+position j0;
+@@
+
+        d@j0 =
+*             kmem_cache_alloc
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(T));
+
+@kmem2_context depends on !patch && (context || org || report)@
+statement S;
+expression d;
+position j0;
+@@
+
+        d@j0 =
+*             kmem_cache_alloc
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(*d));
+
+@dma3_context depends on !patch && (context || org || report)@
+type T;
+statement S;
+T *d;
+position j0;
+@@
+
+        d@j0 =
+*             dma_alloc_coherent
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(T));
+
+@dma4_context depends on !patch && (context || org || report)@
+statement S;
+expression d;
+position j0;
+@@
+
+        d@j0 =
+*             dma_alloc_coherent
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(*d));
+
+@acpi1_context depends on !patch && (context || org || report)@
+type T;
+statement S;
+T *d;
+position j0;
+@@
+
+        d@j0 =
+*             acpi_os_allocate
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(T));
+
+@acpi2_context depends on !patch && (context || org || report)@
+statement S;
+expression d;
+position j0;
+@@
+
+        d@j0 =
+*             acpi_os_allocate
+             (...);
+        if (!d) S
+*        memset(d, 0, sizeof(*d));
+
+// ----------------------------------------------------------------------------
+
+@script:python dma1_org depends on org@
+j0 << dma1_context.j0;
+@@
+
+msg = "Replace with dma_pool_zalloc."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python dma2_org depends on org@
+j0 << dma2_context.j0;
+@@
+
+msg = "Replace with dma_pool_zalloc."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python vz1_org depends on org@
+j0 << vz1_context.j0;
+@@
+
+msg = "Replace with vzalloc."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python vz2_org depends on org@
+j0 << vz2_context.j0;
+@@
+
+msg = "Replace with vzalloc."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python vzn1_org depends on org@
+j0 << vzn1_context.j0;
+@@
+
+msg = "Replace with vzalloc_node."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python vzn2_org depends on org@
+j0 << vzn2_context.j0;
+@@
+
+msg = "Replace with vzalloc_node."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python pci1_org depends on org@
+j0 << pci1_context.j0;
+@@
+
+msg = "Replace with pci_zalloc_consistent."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python pci2_org depends on org@
+j0 << pci2_context.j0;
+@@
+
+msg = "Replace with pci_zalloc_consistent."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python kmem1_org depends on org@
+j0 << kmem1_context.j0;
+@@
+
+msg = "Replace with kmem_cache_zalloc."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python kmem2_org depends on org@
+j0 << kmem2_context.j0;
+@@
+
+msg = "Replace with kmem_cache_zalloc."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python dma3_org depends on org@
+j0 << dma3_context.j0;
+@@
+
+msg = "Replace with dma_zalloc_coherent."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python dma4_org depends on org@
+j0 << dma4_context.j0;
+@@
+
+msg = "Replace with dma_zalloc_coherent."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python acpi1_org depends on org@
+j0 << acpi1_context.j0;
+@@
+
+msg = "Replace with acpi_os_allocate_zeroed."
+coccilib.org.print_todo(j0[0], msg)
+
+@script:python acpi2_org depends on org@
+j0 << acpi2_context.j0;
+@@
+
+msg = "Replace with acpi_os_allocate_zeroed."
+coccilib.org.print_todo(j0[0], msg)
+
+// ----------------------------------------------------------------------------
+
+@script:python dma1_report depends on report@
+j0 << dma1_context.j0;
+@@
+
+msg = "Replace with dma_pool_zalloc."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python dma2_report depends on report@
+j0 << dma2_context.j0;
+@@
+
+msg = "Replace with dma_pool_zalloc."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python vz1_report depends on report@
+j0 << vz1_context.j0;
+@@
+
+msg = "Replace with vzalloc."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python vz2_report depends on report@
+j0 << vz2_context.j0;
+@@
+
+msg = "Replace with vzalloc."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python vzn1_report depends on report@
+j0 << vzn1_context.j0;
+@@
+
+msg = "Replace with vzalloc_node."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python vzn2_report depends on report@
+j0 << vzn2_context.j0;
+@@
+
+msg = "Replace with vzalloc_node."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python pci1_report depends on report@
+j0 << pci1_context.j0;
+@@
+
+msg = "Replace with pci_zalloc_consistent."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python pci2_report depends on report@
+j0 << pci2_context.j0;
+@@
+
+msg = "Replace with pci_zalloc_consistent."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python kmem1_report depends on report@
+j0 << kmem1_context.j0;
+@@
+
+msg = "Replace with kmem_cache_zalloc."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python kmem2_report depends on report@
+j0 << kmem2_context.j0;
+@@
+
+msg = "Replace with kmem_cache_zalloc."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python dma3_report depends on report@
+j0 << dma3_context.j0;
+@@
+
+msg = "Replace with dma_zalloc_coherent."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python dma4_report depends on report@
+j0 << dma4_context.j0;
+@@
+
+msg = "Replace with dma_zalloc_coherent."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python acpi1_report depends on report@
+j0 << acpi1_context.j0;
+@@
+
+msg = "Replace with acpi_os_allocate_zeroed."
+coccilib.report.print_report(j0[0], msg)
+
+@script:python acpi2_report depends on report@
+j0 << acpi2_context.j0;
+@@
+
+msg = "Replace with acpi_os_allocate_zeroed."
+coccilib.report.print_report(j0[0], msg)
+
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-01  7:02 [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions Amitoj Kaur Chawla
@ 2016-08-01 11:23 ` SF Markus Elfring
  2016-08-01 11:37   ` Julia Lawall
  2016-08-19 12:14   ` Amitoj Kaur Chawla
  2017-03-03 10:34 ` [Cocci] [PATCH v3] " SF Markus Elfring
  1 sibling, 2 replies; 14+ messages in thread
From: SF Markus Elfring @ 2016-08-01 11:23 UTC (permalink / raw)
  To: Amitoj Kaur Chawla
  Cc: LKML, Coccinelle, Gilles Muller, Julia Lawall, Michal Marek,
	Nicolas Palix

> +@vz1 depends on patch && !context && !org && !report@
> +type T;
> +T *d;
> +statement S;
> +@@
> +
> +        d =
> +-            vmalloc
> ++            vzalloc
> +             (...);
> +        if (!d) S
> +-       memset(d, 0, sizeof(T));
> +
> +@vz2 depends on patch && !context && !org && !report@
> +expression d;
> +statement S;
> +@@
> +
> +        d =
> +-            vmalloc
> ++            vzalloc
> +             (...);
> +        if (!d) S
> +-       memset(d, 0, sizeof(*d));

I suggest to take another look at a few implementation details.

1. Would it make sense to merge such SmPL rules into one
   so that code duplication could be reduced a bit
   in such a script?

2. How do you think about to extend the shown check list
   with the function "kvm_kvzalloc"?

3. Do you want to maintain a growing (?) function name list manually?

Regards,
Markus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-01 11:23 ` [Cocci] " SF Markus Elfring
@ 2016-08-01 11:37   ` Julia Lawall
  2016-08-01 11:56     ` SF Markus Elfring
  2016-08-19 12:14   ` Amitoj Kaur Chawla
  1 sibling, 1 reply; 14+ messages in thread
From: Julia Lawall @ 2016-08-01 11:37 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Amitoj Kaur Chawla, LKML, Coccinelle, Gilles Muller,
	Michal Marek, Nicolas Palix



On Mon, 1 Aug 2016, SF Markus Elfring wrote:

> > +@vz1 depends on patch && !context && !org && !report@
> > +type T;
> > +T *d;
> > +statement S;
> > +@@
> > +
> > +        d =
> > +-            vmalloc
> > ++            vzalloc
> > +             (...);
> > +        if (!d) S
> > +-       memset(d, 0, sizeof(T));
> > +
> > +@vz2 depends on patch && !context && !org && !report@
> > +expression d;
> > +statement S;
> > +@@
> > +
> > +        d =
> > +-            vmalloc
> > ++            vzalloc
> > +             (...);
> > +        if (!d) S
> > +-       memset(d, 0, sizeof(*d));
>
> I suggest to take another look at a few implementation details.
>
> 1. Would it make sense to merge such SmPL rules into one
>    so that code duplication could be reduced a bit
>    in such a script?

I think it would suffer in readability. Perhaps in performance as well.

julia


>
> 2. How do you think about to extend the shown check list
>    with the function "kvm_kvzalloc"?
>
> 3. Do you want to maintain a growing (?) function name list manually?
>
> Regards,
> Markus
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-01 11:37   ` Julia Lawall
@ 2016-08-01 11:56     ` SF Markus Elfring
  2016-08-01 12:03       ` Julia Lawall
  0 siblings, 1 reply; 14+ messages in thread
From: SF Markus Elfring @ 2016-08-01 11:56 UTC (permalink / raw)
  To: Julia Lawall, Amitoj Kaur Chawla
  Cc: LKML, Coccinelle, Gilles Muller, Michal Marek, Nicolas Palix

>> 1. Would it make sense to merge such SmPL rules into one
>>    so that code duplication could be reduced a bit
>>    in such a script?
> 
> I think it would suffer in readability.

How do you think about the following SmPL script example?

@vz_combined
 depends on patch && !context && !org && !report@
type T;
T* pointer;
+statement S;
expression express;
@@
 pointer =
-          vmalloc
+          vzalloc
           (...);
 if (!d)
    S
-memset(d, 0, sizeof(
(
-T
|
-*(express)
)
-));


> Perhaps in performance as well.

I admit that I am unsure about the run-time characteristics
for my suggestion.

Regards,
Markus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-01 11:56     ` SF Markus Elfring
@ 2016-08-01 12:03       ` Julia Lawall
  2016-08-01 12:24         ` SF Markus Elfring
  0 siblings, 1 reply; 14+ messages in thread
From: Julia Lawall @ 2016-08-01 12:03 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Amitoj Kaur Chawla, LKML, Coccinelle, Gilles Muller,
	Michal Marek, Nicolas Palix



On Mon, 1 Aug 2016, SF Markus Elfring wrote:

> >> 1. Would it make sense to merge such SmPL rules into one
> >>    so that code duplication could be reduced a bit
> >>    in such a script?
> >
> > I think it would suffer in readability.
>
> How do you think about the following SmPL script example?
>
> @vz_combined
>  depends on patch && !context && !org && !report@
> type T;
> T* pointer;
> +statement S;
> expression express;
> @@
>  pointer =
> -          vmalloc
> +          vzalloc
>            (...);
>  if (!d)
>     S
> -memset(d, 0, sizeof(
> (
> -T
> |
> -*(express)
> )
> -));

OK, I thought you meant to make a big disjunctions for all of the before
and after functions.  This is a little better because it is bounded in
size.  But I don't understand why you have introduced the variable
express.  Normally one wants the cleared space to be the allocated size,
which is normally the size of *pointer.

The performance issue is that disjunctions on expressions, eg (A | B), are
implemented as (A | (!A & B)), ie with a negation of all the previous
options &d with each option.  So it is better to avoid very large
disjunctions on expressions.

julia


>
>
> > Perhaps in performance as well.
>
> I admit that I am unsure about the run-time characteristics
> for my suggestion.
>
> Regards,
> Markus
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-01 12:03       ` Julia Lawall
@ 2016-08-01 12:24         ` SF Markus Elfring
  2016-08-01 12:32           ` Julia Lawall
  0 siblings, 1 reply; 14+ messages in thread
From: SF Markus Elfring @ 2016-08-01 12:24 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Amitoj Kaur Chawla, LKML, Coccinelle, Gilles Muller,
	Michal Marek, Nicolas Palix

>> How do you think about the following SmPL script example?
>>
>> @vz_combined
>>  depends on patch && !context && !org && !report@
>> type T;
>> T* pointer;
>> +statement S;
>> expression express;
>> @@
>>  pointer =
>> -          vmalloc
>> +          vzalloc
>>            (...);
>>  if (!d)
>>     S
>> -memset(d, 0, sizeof(
>> (
>> -T
>> |
>> -*(express)
>> )
>> -));
> 
> OK, I thought you meant to make a big disjunctions for all of the before
> and after functions.

I imagine that it would be nice if the function name pairs could be specified
in a more succinct format for the semantic patch language.
But the discussed approach can work with a recent software version already.


> This is a little better because it is bounded in size.

Thanks …


> But I don't understand why you have introduced the variable express.

I have noticed that these two SmPL rules differed only in the source code
search specification for the operator "sizeof".
So I would prefer to express this small difference in the script directly.


> The performance issue is that disjunctions on expressions, eg (A | B), are
> implemented as (A | (!A & B)), ie with a negation of all the previous
> options &d with each option.  So it is better to avoid very large
> disjunctions on expressions.

Is the suggested SmPL disjunction still small enough for this concern?

Regards,
Markus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-01 12:24         ` SF Markus Elfring
@ 2016-08-01 12:32           ` Julia Lawall
  2016-08-01 12:45             ` SF Markus Elfring
  0 siblings, 1 reply; 14+ messages in thread
From: Julia Lawall @ 2016-08-01 12:32 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Amitoj Kaur Chawla, LKML, Coccinelle, Gilles Muller,
	Michal Marek, Nicolas Palix



On Mon, 1 Aug 2016, SF Markus Elfring wrote:

> >> How do you think about the following SmPL script example?
> >>
> >> @vz_combined
> >>  depends on patch && !context && !org && !report@
> >> type T;
> >> T* pointer;
> >> +statement S;
> >> expression express;
> >> @@
> >>  pointer =
> >> -          vmalloc
> >> +          vzalloc
> >>            (...);
> >>  if (!d)
> >>     S
> >> -memset(d, 0, sizeof(
> >> (
> >> -T
> >> |
> >> -*(express)
> >> )
> >> -));

Actually, this is a mess.  pointer, d, and express are all supposed to be
the same thing, as they were in the original rule.

Furthermore, this shows a reason why the original rule was better.  If you
say T *pointer, then you require that Coccinelle can find the type
sofficiently to know that it is a pointer.  There was no such constraint
in the sizeof(*d) variant of the original rule.

> > The performance issue is that disjunctions on expressions, eg (A | B), are
> > implemented as (A | (!A & B)), ie with a negation of all the previous
> > options &d with each option.  So it is better to avoid very large
> > disjunctions on expressions.
>
> Is the suggested SmPL disjunction still small enough for this concern?

2 elements is OK.

julia

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-01 12:32           ` Julia Lawall
@ 2016-08-01 12:45             ` SF Markus Elfring
  0 siblings, 0 replies; 14+ messages in thread
From: SF Markus Elfring @ 2016-08-01 12:45 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Amitoj Kaur Chawla, LKML, Coccinelle, Gilles Muller,
	Michal Marek, Nicolas Palix

>>>> @vz_combined
>>>>  depends on patch && !context && !org && !report@
>>>> type T;
>>>> T* pointer;
>>>> +statement S;
>>>> expression express;
>>>> @@
>>>>  pointer =
>>>> -          vmalloc
>>>> +          vzalloc
>>>>            (...);
>>>>  if (!d)
>>>>     S
>>>> -memset(d, 0, sizeof(
>>>> (
>>>> -T
>>>> |
>>>> -*(express)
>>>> )
>>>> -));
> 
> Actually, this is a mess.  pointer, d, and express are all supposed to be
> the same thing, as they were in the original rule.
> 
> Furthermore, this shows a reason why the original rule was better.

There is a trade-off between several unique SmPL rules and other combined
variants which could eventually work with two SmPL disjunctions.

Regards,
Markus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-01 11:23 ` [Cocci] " SF Markus Elfring
  2016-08-01 11:37   ` Julia Lawall
@ 2016-08-19 12:14   ` Amitoj Kaur Chawla
  2016-08-19 12:41     ` Julia Lawall
  2016-08-19 13:26     ` [Cocci] " SF Markus Elfring
  1 sibling, 2 replies; 14+ messages in thread
From: Amitoj Kaur Chawla @ 2016-08-19 12:14 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: LKML, Coccinelle, Gilles Muller, Julia Lawall, Michal Marek,
	Nicolas Palix

On Mon, Aug 1, 2016 at 4:53 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>> +@vz1 depends on patch && !context && !org && !report@
>> +type T;
>> +T *d;
>> +statement S;
>> +@@
>> +
>> +        d =
>> +-            vmalloc
>> ++            vzalloc
>> +             (...);
>> +        if (!d) S
>> +-       memset(d, 0, sizeof(T));
>> +
>> +@vz2 depends on patch && !context && !org && !report@
>> +expression d;
>> +statement S;
>> +@@
>> +
>> +        d =
>> +-            vmalloc
>> ++            vzalloc
>> +             (...);
>> +        if (!d) S
>> +-       memset(d, 0, sizeof(*d));
>
> I suggest to take another look at a few implementation details.
>
> 1. Would it make sense to merge such SmPL rules into one
>    so that code duplication could be reduced a bit
>    in such a script?
>
> 2. How do you think about to extend the shown check list
>    with the function "kvm_kvzalloc"?
>

Hi Markus,

kvm_kvzalloc function doesn't fit the same pattern as the other
functions in this semantic patch, and is kvm specific, so the
semantic patch looks fine as is.

Thanks,
Amitoj

> 3. Do you want to maintain a growing (?) function name list manually?
>
> Regards,
> Markus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-19 12:14   ` Amitoj Kaur Chawla
@ 2016-08-19 12:41     ` Julia Lawall
  2016-08-19 13:26     ` [Cocci] " SF Markus Elfring
  1 sibling, 0 replies; 14+ messages in thread
From: Julia Lawall @ 2016-08-19 12:41 UTC (permalink / raw)
  To: Amitoj Kaur Chawla
  Cc: SF Markus Elfring, LKML, Coccinelle, Gilles Muller, Julia Lawall,
	Michal Marek, Nicolas Palix



On Fri, 19 Aug 2016, Amitoj Kaur Chawla wrote:

> On Mon, Aug 1, 2016 at 4:53 PM, SF Markus Elfring
> <elfring@users.sourceforge.net> wrote:
> >> +@vz1 depends on patch && !context && !org && !report@
> >> +type T;
> >> +T *d;
> >> +statement S;
> >> +@@
> >> +
> >> +        d =
> >> +-            vmalloc
> >> ++            vzalloc
> >> +             (...);
> >> +        if (!d) S
> >> +-       memset(d, 0, sizeof(T));
> >> +
> >> +@vz2 depends on patch && !context && !org && !report@
> >> +expression d;
> >> +statement S;
> >> +@@
> >> +
> >> +        d =
> >> +-            vmalloc
> >> ++            vzalloc
> >> +             (...);
> >> +        if (!d) S
> >> +-       memset(d, 0, sizeof(*d));
> >
> > I suggest to take another look at a few implementation details.
> >
> > 1. Would it make sense to merge such SmPL rules into one
> >    so that code duplication could be reduced a bit
> >    in such a script?
> >
> > 2. How do you think about to extend the shown check list
> >    with the function "kvm_kvzalloc"?
> >
>
> Hi Markus,
>
> kvm_kvzalloc function doesn't fit the same pattern as the other
> functions in this semantic patch, and is kvm specific, so the
> semantic patch looks fine as is.

Acked-by: Julia Lawall <julia.lawall@lip6.fr>

>
> Thanks,
> Amitoj
>
> > 3. Do you want to maintain a growing (?) function name list manually?
> >
> > Regards,
> > Markus
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-19 12:14   ` Amitoj Kaur Chawla
  2016-08-19 12:41     ` Julia Lawall
@ 2016-08-19 13:26     ` SF Markus Elfring
  2016-08-19 13:30       ` Julia Lawall
  1 sibling, 1 reply; 14+ messages in thread
From: SF Markus Elfring @ 2016-08-19 13:26 UTC (permalink / raw)
  To: Amitoj Kaur Chawla
  Cc: LKML, Coccinelle, Gilles Muller, Julia Lawall, Michal Marek,
	Nicolas Palix

>> I suggest to take another look at a few implementation details.
>>
>> 1. Would it make sense to merge such SmPL rules into one
>>    so that code duplication could be reduced a bit
>>    in such a script?
>>
>> 2. How do you think about to extend the shown check list
>>    with the function "kvm_kvzalloc"?
> kvm_kvzalloc function doesn't fit the same pattern as the other
> functions in this semantic patch, and is kvm specific,

Has this one got a similar function property?

Do you prefer to exclude such functions which belong to subsystems
so far generally?


> so the semantic patch looks fine as is.

How do you think about to express the shown source code repetition
as an aspect by an other script format?

Regards,
Markus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-19 13:26     ` [Cocci] " SF Markus Elfring
@ 2016-08-19 13:30       ` Julia Lawall
  2016-08-19 16:56         ` SF Markus Elfring
  0 siblings, 1 reply; 14+ messages in thread
From: Julia Lawall @ 2016-08-19 13:30 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: Amitoj Kaur Chawla, LKML, Coccinelle, Gilles Muller,
	Michal Marek, Nicolas Palix

[-- Attachment #1: Type: TEXT/PLAIN, Size: 980 bytes --]



On Fri, 19 Aug 2016, SF Markus Elfring wrote:

> >> I suggest to take another look at a few implementation details.
> >>
> >> 1. Would it make sense to merge such SmPL rules into one
> >>    so that code duplication could be reduced a bit
> >>    in such a script?
> >>
> >> 2. How do you think about to extend the shown check list
> >>    with the function "kvm_kvzalloc"?
> …
> > kvm_kvzalloc function doesn't fit the same pattern as the other
> > functions in this semantic patch, and is kvm specific,
>
> Has this one got a similar function property?

Do you have any example where XXX followed by memset is converted to this
function?

>
> Do you prefer to exclude such functions which belong to subsystems
> so far generally?

Yes, because it would introduce unwanted dependencies.

>
> > so the semantic patch looks fine as is.
>
> How do you think about to express the shown source code repetition
> as an aspect by an other script format?

It is fine as it is.

julia

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-19 13:30       ` Julia Lawall
@ 2016-08-19 16:56         ` SF Markus Elfring
  0 siblings, 0 replies; 14+ messages in thread
From: SF Markus Elfring @ 2016-08-19 16:56 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Amitoj Kaur Chawla, LKML, Coccinelle, Gilles Muller,
	Michal Marek, Nicolas Palix

>> Do you prefer to exclude such functions which belong to subsystems
>> so far generally?
> 
> Yes, because it would introduce unwanted dependencies.

Are the mentioned dependencies easier to handle for subsystems
like "acpi", "dma" and "pci" (than "kvm")?

Would anybody like to tackle any further software development
challenges there?

Regards,
Markus

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions
  2016-08-01  7:02 [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions Amitoj Kaur Chawla
  2016-08-01 11:23 ` [Cocci] " SF Markus Elfring
@ 2017-03-03 10:34 ` SF Markus Elfring
  1 sibling, 0 replies; 14+ messages in thread
From: SF Markus Elfring @ 2017-03-03 10:34 UTC (permalink / raw)
  To: Amitoj Kaur Chawla, Coccinelle
  Cc: LKML, Gilles Muller, Nicolas Palix, Michal Marek

> This script finds instances of allocate and memset which can be
> replaced with a direct call to zalloc equivalent of a function.

What is the software development status for this SmPL script in comparison
to an other directory for source code transformations?
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/scripts/coccinelle/api/alloc

Regards,
Markus

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-03-03 10:42 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-01  7:02 [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions Amitoj Kaur Chawla
2016-08-01 11:23 ` [Cocci] " SF Markus Elfring
2016-08-01 11:37   ` Julia Lawall
2016-08-01 11:56     ` SF Markus Elfring
2016-08-01 12:03       ` Julia Lawall
2016-08-01 12:24         ` SF Markus Elfring
2016-08-01 12:32           ` Julia Lawall
2016-08-01 12:45             ` SF Markus Elfring
2016-08-19 12:14   ` Amitoj Kaur Chawla
2016-08-19 12:41     ` Julia Lawall
2016-08-19 13:26     ` [Cocci] " SF Markus Elfring
2016-08-19 13:30       ` Julia Lawall
2016-08-19 16:56         ` SF Markus Elfring
2017-03-03 10:34 ` [Cocci] [PATCH v3] " SF Markus Elfring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).