Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1453008 > unrolled thread

[PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions

Back to article view | Back to linux.kernel

Contents

  [PATCH v3] Coccinelle: Script to replace allocate and memset with  zalloc functions Amitoj Kaur Chawla <amitoj1606@gmail.com> - 2016-08-01 09:10 +0200
    Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and  memset with zalloc functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-01 13:30 +0200
      Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and  memset with zalloc functions Julia Lawall <julia.lawall@lip6.fr> - 2016-08-01 13:40 +0200
        Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and  memset with zalloc functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-01 14:00 +0200
          Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and  memset with zalloc functions Julia Lawall <julia.lawall@lip6.fr> - 2016-08-01 14:10 +0200
            Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and  memset with zalloc functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-01 14:30 +0200
              Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and  memset with zalloc functions Julia Lawall <julia.lawall@lip6.fr> - 2016-08-01 14:40 +0200
                Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and  memset with zalloc functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-08-01 14:50 +0200

#1453008 — [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions

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

[toc] | [next] | [standalone]

#1453145 — Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions

> +@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

[toc] | [prev] | [next] | [standalone]

#1453151 — Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions

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
>

[toc] | [prev] | [next] | [standalone]

#1453162 — Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions

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

[toc] | [prev] | [next] | [standalone]

#1453166 — Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions

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
>

[toc] | [prev] | [next] | [standalone]

#1453177 — Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions

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

[toc] | [prev] | [next] | [standalone]

#1453183 — Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions

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

[toc] | [prev] | [next] | [standalone]

#1453190 — Re: [Cocci] [PATCH v3] Coccinelle: Script to replace allocate and memset with zalloc functions

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web