public inbox for cocci@systeme.lip6.fr
 help / color / mirror / Atom feed
* [Cocci] [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment
@ 2013-01-23 22:06 Peter Senna Tschudin
  2013-01-24 17:47 ` [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment) Michael Stefaniuc
  2013-02-22 10:25 ` [Cocci] [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment Michal Marek
  0 siblings, 2 replies; 10+ messages in thread
From: Peter Senna Tschudin @ 2013-01-23 22:06 UTC (permalink / raw)
  To: cocci

There are error-prone memcpy() that can be replaced by struct
assignment that are type-safe and much easier to read. This semantic
patch looks for memcpy() that can be replaced by struct assignment.

Inspired by patches sent by Ezequiel Garcia <elezegarcia@gmail.com>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
Changes from V1:
 Updated commit message
 Changed Confidence comment to High on the semantic patch

 scripts/coccinelle/misc/memcpy-assign.cocci | 103 ++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)
 create mode 100644 scripts/coccinelle/misc/memcpy-assign.cocci

diff --git a/scripts/coccinelle/misc/memcpy-assign.cocci b/scripts/coccinelle/misc/memcpy-assign.cocci
new file mode 100644
index 0000000..afd058b
--- /dev/null
+++ b/scripts/coccinelle/misc/memcpy-assign.cocci
@@ -0,0 +1,103 @@
+//
+// Replace memcpy with struct assignment.
+//
+// Confidence: High
+// Copyright: (C) 2012 Peter Senna Tschudin, INRIA/LIP6.  GPLv2.
+// URL: http://coccinelle.lip6.fr/
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual patch
+virtual report
+virtual context
+virtual org
+
+ at r1 depends on !patch@
+identifier struct_name;
+struct struct_name to;
+struct struct_name from;
+struct struct_name *top;
+struct struct_name *fromp;
+position p;
+@@
+memcpy at p(\(&(to)\|top\), \(&(from)\|fromp\), \(sizeof(to)\|sizeof(from)\|sizeof(struct struct_name)\|sizeof(*top)\|sizeof(*fromp)\))
+
+ at script:python depends on report@
+p << r1.p;
+@@
+coccilib.report.print_report(p[0],"Replace memcpy with struct assignment")
+
+ at depends on context@
+position r1.p;
+@@
+*memcpy at p(...);
+
+ at script:python depends on org@
+p << r1.p;
+@@
+cocci.print_main("Replace memcpy with struct assignment",p)
+
+ at depends on patch@
+identifier struct_name;
+struct struct_name to;
+struct struct_name from;
+@@
+(
+-memcpy(&(to), &(from), sizeof(to));
++to = from;
+|
+-memcpy(&(to), &(from), sizeof(from));
++to = from;
+|
+-memcpy(&(to), &(from), sizeof(struct struct_name));
++to = from;
+)
+
+ at depends on patch@
+identifier struct_name;
+struct struct_name to;
+struct struct_name *from;
+@@
+(
+-memcpy(&(to), from, sizeof(to));
++to = *from;
+|
+-memcpy(&(to), from, sizeof(*from));
++to = *from;
+|
+-memcpy(&(to), from, sizeof(struct struct_name));
++to = *from;
+)
+
+ at depends on patch@
+identifier struct_name;
+struct struct_name *to;
+struct struct_name from;
+@@
+(
+-memcpy(to, &(from), sizeof(*to));
++ *to = from;
+|
+-memcpy(to, &(from), sizeof(from));
++ *to = from;
+|
+-memcpy(to, &(from), sizeof(struct struct_name));
++ *to = from;
+)
+
+@depends on patch@
+identifier struct_name;
+struct struct_name *to;
+struct struct_name *from;
+@@
+(
+-memcpy(to, from, sizeof(*to));
++ *to = *from;
+|
+-memcpy(to, from, sizeof(*from));
++ *to = *from;
+|
+-memcpy(to, from, sizeof(struct struct_name));
++ *to = *from;
+)
+
-- 
1.7.11.7

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

* [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment)
  2013-01-23 22:06 [Cocci] [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment Peter Senna Tschudin
@ 2013-01-24 17:47 ` Michael Stefaniuc
  2013-01-24 18:05   ` Julia Lawall
  2013-01-26  1:28   ` Peter Senna Tschudin
  2013-02-22 10:25 ` [Cocci] [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment Michal Marek
  1 sibling, 2 replies; 10+ messages in thread
From: Michael Stefaniuc @ 2013-01-24 17:47 UTC (permalink / raw)
  To: cocci

Hello,

On 01/23/2013 11:06 PM, Peter Senna Tschudin wrote:
> There are error-prone memcpy() that can be replaced by struct
> assignment that are type-safe and much easier to read. This semantic
> patch looks for memcpy() that can be replaced by struct assignment.
thanks for the script by Peter this script as it proved useful for Wine
too. Though in one case it generated the wrong patch.

@@
struct struct_name to;
struct struct_name from;
@@

'to' and 'from' above are expressions and not identifiers; that's a
feature. The bug happened in the replacement
- memcpy(foos + count, &foo, sizeof(struct foo_t));
+ *foos + count = foo;

Shouldn't cocinelle automatically add the parenthesis when prepending a
'*' or '&' to an expression which isn't an identifier?


The script itself is "safe" as the compiler will error out on that
patch. And the people that just take generated patches and submit them
without even compiling them deserve the public tar and feathering ;)

bye
	michael

> 
> Inspired by patches sent by Ezequiel Garcia <elezegarcia@gmail.com>
> 
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
> Changes from V1:
>  Updated commit message
>  Changed Confidence comment to High on the semantic patch
> 
>  scripts/coccinelle/misc/memcpy-assign.cocci | 103 ++++++++++++++++++++++++++++
>  1 file changed, 103 insertions(+)
>  create mode 100644 scripts/coccinelle/misc/memcpy-assign.cocci
> 
> diff --git a/scripts/coccinelle/misc/memcpy-assign.cocci b/scripts/coccinelle/misc/memcpy-assign.cocci
> new file mode 100644
> index 0000000..afd058b
> --- /dev/null
> +++ b/scripts/coccinelle/misc/memcpy-assign.cocci
> @@ -0,0 +1,103 @@
> +//
> +// Replace memcpy with struct assignment.
> +//
> +// Confidence: High
> +// Copyright: (C) 2012 Peter Senna Tschudin, INRIA/LIP6.  GPLv2.
> +// URL: http://coccinelle.lip6.fr/
> +// Comments:
> +// Options: --no-includes --include-headers
> +
> +virtual patch
> +virtual report
> +virtual context
> +virtual org
> +
> + at r1 depends on !patch@
> +identifier struct_name;
> +struct struct_name to;
> +struct struct_name from;
> +struct struct_name *top;
> +struct struct_name *fromp;
> +position p;
> +@@
> +memcpy at p(\(&(to)\|top\), \(&(from)\|fromp\), \(sizeof(to)\|sizeof(from)\|sizeof(struct struct_name)\|sizeof(*top)\|sizeof(*fromp)\))
> +
> + at script:python depends on report@
> +p << r1.p;
> +@@
> +coccilib.report.print_report(p[0],"Replace memcpy with struct assignment")
> +
> + at depends on context@
> +position r1.p;
> +@@
> +*memcpy at p(...);
> +
> + at script:python depends on org@
> +p << r1.p;
> +@@
> +cocci.print_main("Replace memcpy with struct assignment",p)
> +
> + at depends on patch@
> +identifier struct_name;
> +struct struct_name to;
> +struct struct_name from;
> +@@
> +(
> +-memcpy(&(to), &(from), sizeof(to));
> ++to = from;
> +|
> +-memcpy(&(to), &(from), sizeof(from));
> ++to = from;
> +|
> +-memcpy(&(to), &(from), sizeof(struct struct_name));
> ++to = from;
> +)
> +
> + at depends on patch@
> +identifier struct_name;
> +struct struct_name to;
> +struct struct_name *from;
> +@@
> +(
> +-memcpy(&(to), from, sizeof(to));
> ++to = *from;
> +|
> +-memcpy(&(to), from, sizeof(*from));
> ++to = *from;
> +|
> +-memcpy(&(to), from, sizeof(struct struct_name));
> ++to = *from;
> +)
> +
> + at depends on patch@
> +identifier struct_name;
> +struct struct_name *to;
> +struct struct_name from;
> +@@
> +(
> +-memcpy(to, &(from), sizeof(*to));
> ++ *to = from;
> +|
> +-memcpy(to, &(from), sizeof(from));
> ++ *to = from;
> +|
> +-memcpy(to, &(from), sizeof(struct struct_name));
> ++ *to = from;
> +)
> +
> + at depends on patch@
> +identifier struct_name;
> +struct struct_name *to;
> +struct struct_name *from;
> +@@
> +(
> +-memcpy(to, from, sizeof(*to));
> ++ *to = *from;
> +|
> +-memcpy(to, from, sizeof(*from));
> ++ *to = *from;
> +|
> +-memcpy(to, from, sizeof(struct struct_name));
> ++ *to = *from;
> +)
> +

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

* [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment)
  2013-01-24 17:47 ` [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment) Michael Stefaniuc
@ 2013-01-24 18:05   ` Julia Lawall
  2013-01-26  1:28   ` Peter Senna Tschudin
  1 sibling, 0 replies; 10+ messages in thread
From: Julia Lawall @ 2013-01-24 18:05 UTC (permalink / raw)
  To: cocci

On Thu, 24 Jan 2013, Michael Stefaniuc wrote:

> Hello,
>
> On 01/23/2013 11:06 PM, Peter Senna Tschudin wrote:
> > There are error-prone memcpy() that can be replaced by struct
> > assignment that are type-safe and much easier to read. This semantic
> > patch looks for memcpy() that can be replaced by struct assignment.
> thanks for the script by Peter this script as it proved useful for Wine
> too. Though in one case it generated the wrong patch.
>
> @@
> struct struct_name to;
> struct struct_name from;
> @@
>
> 'to' and 'from' above are expressions and not identifiers; that's a
> feature. The bug happened in the replacement
> - memcpy(foos + count, &foo, sizeof(struct foo_t));
> + *foos + count = foo;
>
> Shouldn't cocinelle automatically add the parenthesis when prepending a
> '*' or '&' to an expression which isn't an identifier?

Perhaps it would be possible, but there is nothing in place to do that.

Part of a solution would be to declare the metavariable in the
unparenthesized case as an idexpression, ie idexpression struct
struct_name *to.  Then one could have a rule for an arbitrary expression
afterwards, ie with the metavariable declaration above, that would
explicitly add parentheses.

With that though you would end up with parentheses around eg x->y, because
that is not considered to be an identifier.  So one would also need a
separate rule in the middle for that case.  To ensure that the types are
OK, you should be able to do:

@@
struct struct_name *to;
expression e;
identifier f;
...
@@

  e->f at to

This should match the same term against both the pattern e->f and the
pattern to.

Thanks for testing.

julia

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

* [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment)
  2013-01-24 17:47 ` [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment) Michael Stefaniuc
  2013-01-24 18:05   ` Julia Lawall
@ 2013-01-26  1:28   ` Peter Senna Tschudin
  2013-01-27 20:46     ` Michael Stefaniuc
  1 sibling, 1 reply; 10+ messages in thread
From: Peter Senna Tschudin @ 2013-01-26  1:28 UTC (permalink / raw)
  To: cocci

Hello Michael,

Thank you for testing.

I made changes on the semantic patch. I believe it fixed the problem.
Can you test it?

[]'s

Peter

On Thu, Jan 24, 2013 at 3:47 PM, Michael Stefaniuc <mstefani@redhat.com> wrote:
> Hello,
>
> On 01/23/2013 11:06 PM, Peter Senna Tschudin wrote:
>> There are error-prone memcpy() that can be replaced by struct
>> assignment that are type-safe and much easier to read. This semantic
>> patch looks for memcpy() that can be replaced by struct assignment.
> thanks for the script by Peter this script as it proved useful for Wine
> too. Though in one case it generated the wrong patch.
>
> @@
> struct struct_name to;
> struct struct_name from;
> @@
>
> 'to' and 'from' above are expressions and not identifiers; that's a
> feature. The bug happened in the replacement
> - memcpy(foos + count, &foo, sizeof(struct foo_t));
> + *foos + count = foo;
>
> Shouldn't cocinelle automatically add the parenthesis when prepending a
> '*' or '&' to an expression which isn't an identifier?
>
>
> The script itself is "safe" as the compiler will error out on that
> patch. And the people that just take generated patches and submit them
> without even compiling them deserve the public tar and feathering ;)
>
> bye
>         michael
>
>>
>> Inspired by patches sent by Ezequiel Garcia <elezegarcia@gmail.com>
>>
>> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
>> ---
>> Changes from V1:
>>  Updated commit message
>>  Changed Confidence comment to High on the semantic patch
>>
>>  scripts/coccinelle/misc/memcpy-assign.cocci | 103 ++++++++++++++++++++++++++++
>>  1 file changed, 103 insertions(+)
>>  create mode 100644 scripts/coccinelle/misc/memcpy-assign.cocci
>>
>> diff --git a/scripts/coccinelle/misc/memcpy-assign.cocci b/scripts/coccinelle/misc/memcpy-assign.cocci
>> new file mode 100644
>> index 0000000..afd058b
>> --- /dev/null
>> +++ b/scripts/coccinelle/misc/memcpy-assign.cocci
>> @@ -0,0 +1,103 @@
>> +//
>> +// Replace memcpy with struct assignment.
>> +//
>> +// Confidence: High
>> +// Copyright: (C) 2012 Peter Senna Tschudin, INRIA/LIP6.  GPLv2.
>> +// URL: http://coccinelle.lip6.fr/
>> +// Comments:
>> +// Options: --no-includes --include-headers
>> +
>> +virtual patch
>> +virtual report
>> +virtual context
>> +virtual org
>> +
>> + at r1 depends on !patch@
>> +identifier struct_name;
>> +struct struct_name to;
>> +struct struct_name from;
>> +struct struct_name *top;
>> +struct struct_name *fromp;
>> +position p;
>> +@@
>> +memcpy at p(\(&(to)\|top\), \(&(from)\|fromp\), \(sizeof(to)\|sizeof(from)\|sizeof(struct struct_name)\|sizeof(*top)\|sizeof(*fromp)\))
>> +
>> + at script:python depends on report@
>> +p << r1.p;
>> +@@
>> +coccilib.report.print_report(p[0],"Replace memcpy with struct assignment")
>> +
>> + at depends on context@
>> +position r1.p;
>> +@@
>> +*memcpy at p(...);
>> +
>> + at script:python depends on org@
>> +p << r1.p;
>> +@@
>> +cocci.print_main("Replace memcpy with struct assignment",p)
>> +
>> + at depends on patch@
>> +identifier struct_name;
>> +struct struct_name to;
>> +struct struct_name from;
>> +@@
>> +(
>> +-memcpy(&(to), &(from), sizeof(to));
>> ++to = from;
>> +|
>> +-memcpy(&(to), &(from), sizeof(from));
>> ++to = from;
>> +|
>> +-memcpy(&(to), &(from), sizeof(struct struct_name));
>> ++to = from;
>> +)
>> +
>> + at depends on patch@
>> +identifier struct_name;
>> +struct struct_name to;
>> +struct struct_name *from;
>> +@@
>> +(
>> +-memcpy(&(to), from, sizeof(to));
>> ++to = *from;
>> +|
>> +-memcpy(&(to), from, sizeof(*from));
>> ++to = *from;
>> +|
>> +-memcpy(&(to), from, sizeof(struct struct_name));
>> ++to = *from;
>> +)
>> +
>> + at depends on patch@
>> +identifier struct_name;
>> +struct struct_name *to;
>> +struct struct_name from;
>> +@@
>> +(
>> +-memcpy(to, &(from), sizeof(*to));
>> ++ *to = from;
>> +|
>> +-memcpy(to, &(from), sizeof(from));
>> ++ *to = from;
>> +|
>> +-memcpy(to, &(from), sizeof(struct struct_name));
>> ++ *to = from;
>> +)
>> +
>> + at depends on patch@
>> +identifier struct_name;
>> +struct struct_name *to;
>> +struct struct_name *from;
>> +@@
>> +(
>> +-memcpy(to, from, sizeof(*to));
>> ++ *to = *from;
>> +|
>> +-memcpy(to, from, sizeof(*from));
>> ++ *to = *from;
>> +|
>> +-memcpy(to, from, sizeof(struct struct_name));
>> ++ *to = *from;
>> +)
>> +



-- 
Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: memcpy-assign.cocci
Type: application/octet-stream
Size: 2740 bytes
Desc: not available
URL: <https://systeme.lip6.fr/pipermail/cocci/attachments/20130125/82b7bb92/attachment.obj>

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

* [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment)
  2013-01-26  1:28   ` Peter Senna Tschudin
@ 2013-01-27 20:46     ` Michael Stefaniuc
  2013-01-28 22:04       ` Julia Lawall
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Stefaniuc @ 2013-01-27 20:46 UTC (permalink / raw)
  To: cocci

Hello Peter,

On 01/26/2013 02:28 AM, Peter Senna Tschudin wrote:
> Thank you for testing.
> 
> I made changes on the semantic patch. I believe it fixed the problem.
> Can you test it?
while it does fix the issue it makes the cocci script less readable and
works only in that particular case. Not sure it is worth the effort as
the bad generated patch can be trivially detected and easily fixed
manually. My email was more of a bug report / feature request for
coccinelle than an improvement request to the script.

Though I've noticed an other issue with the script: Any reason you use
"Options: --no-includes"? While it does speed up things it misses quite
a few cases as a lot of types are defined in the headers.

bye
	michael

> On Thu, Jan 24, 2013 at 3:47 PM, Michael Stefaniuc <mstefani@redhat.com> wrote:
>> Hello,
>>
>> On 01/23/2013 11:06 PM, Peter Senna Tschudin wrote:
>>> There are error-prone memcpy() that can be replaced by struct
>>> assignment that are type-safe and much easier to read. This semantic
>>> patch looks for memcpy() that can be replaced by struct assignment.
>> thanks for the script by Peter this script as it proved useful for Wine
>> too. Though in one case it generated the wrong patch.
>>
>> @@
>> struct struct_name to;
>> struct struct_name from;
>> @@
>>
>> 'to' and 'from' above are expressions and not identifiers; that's a
>> feature. The bug happened in the replacement
>> - memcpy(foos + count, &foo, sizeof(struct foo_t));
>> + *foos + count = foo;
>>
>> Shouldn't cocinelle automatically add the parenthesis when prepending a
>> '*' or '&' to an expression which isn't an identifier?
>>
>>
>> The script itself is "safe" as the compiler will error out on that
>> patch. And the people that just take generated patches and submit them
>> without even compiling them deserve the public tar and feathering ;)

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

* [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment)
  2013-01-27 20:46     ` Michael Stefaniuc
@ 2013-01-28 22:04       ` Julia Lawall
  2013-01-29 21:38         ` Michael Stefaniuc
  0 siblings, 1 reply; 10+ messages in thread
From: Julia Lawall @ 2013-01-28 22:04 UTC (permalink / raw)
  To: cocci

On Sun, 27 Jan 2013, Michael Stefaniuc wrote:

> Hello Peter,
>
> On 01/26/2013 02:28 AM, Peter Senna Tschudin wrote:
> > Thank you for testing.
> >
> > I made changes on the semantic patch. I believe it fixed the problem.
> > Can you test it?
> while it does fix the issue it makes the cocci script less readable and
> works only in that particular case. Not sure it is worth the effort as
> the bad generated patch can be trivially detected and easily fixed
> manually. My email was more of a bug report / feature request for
> coccinelle than an improvement request to the script.
>
> Though I've noticed an other issue with the script: Any reason you use
> "Options: --no-includes"? While it does speed up things it misses quite
> a few cases as a lot of types are defined in the headers.

I think it should be --all-includes or even --recursive-includes

julia.

>
> bye
> 	michael
>
> > On Thu, Jan 24, 2013 at 3:47 PM, Michael Stefaniuc <mstefani@redhat.com> wrote:
> >> Hello,
> >>
> >> On 01/23/2013 11:06 PM, Peter Senna Tschudin wrote:
> >>> There are error-prone memcpy() that can be replaced by struct
> >>> assignment that are type-safe and much easier to read. This semantic
> >>> patch looks for memcpy() that can be replaced by struct assignment.
> >> thanks for the script by Peter this script as it proved useful for Wine
> >> too. Though in one case it generated the wrong patch.
> >>
> >> @@
> >> struct struct_name to;
> >> struct struct_name from;
> >> @@
> >>
> >> 'to' and 'from' above are expressions and not identifiers; that's a
> >> feature. The bug happened in the replacement
> >> - memcpy(foos + count, &foo, sizeof(struct foo_t));
> >> + *foos + count = foo;
> >>
> >> Shouldn't cocinelle automatically add the parenthesis when prepending a
> >> '*' or '&' to an expression which isn't an identifier?
> >>
> >>
> >> The script itself is "safe" as the compiler will error out on that
> >> patch. And the people that just take generated patches and submit them
> >> without even compiling them deserve the public tar and feathering ;)
> _______________________________________________
> Cocci mailing list
> Cocci at systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

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

* [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment)
  2013-01-28 22:04       ` Julia Lawall
@ 2013-01-29 21:38         ` Michael Stefaniuc
  2013-01-29 21:42           ` Julia Lawall
  2013-02-06 14:37           ` Michael Stefaniuc
  0 siblings, 2 replies; 10+ messages in thread
From: Michael Stefaniuc @ 2013-01-29 21:38 UTC (permalink / raw)
  To: cocci

Hello Julia,

On 01/28/2013 11:04 PM, Julia Lawall wrote:
> On Sun, 27 Jan 2013, Michael Stefaniuc wrote:
> 
>> Hello Peter,
>>
>> On 01/26/2013 02:28 AM, Peter Senna Tschudin wrote:
>>> Thank you for testing.
>>>
>>> I made changes on the semantic patch. I believe it fixed the problem.
>>> Can you test it?
>> while it does fix the issue it makes the cocci script less readable and
>> works only in that particular case. Not sure it is worth the effort as
>> the bad generated patch can be trivially detected and easily fixed
>> manually. My email was more of a bug report / feature request for
>> coccinelle than an improvement request to the script.
>>
>> Though I've noticed an other issue with the script: Any reason you use
>> "Options: --no-includes"? While it does speed up things it misses quite
>> a few cases as a lot of types are defined in the headers.
> 
> I think it should be --all-includes or even --recursive-includes
no clue what the policy in the Kernel is but --recursive-includes is
quite heavy handed as a default. I have one --recursive-includes still
running for Wine and it accumulated until now over 1126 CPU minutes
(Intel i5 mobile CPU with 2.4 GHz). It found 5 more cases while
--no-includes found 11 and --local-includes additional 32. The
--local-includes finished in less than 30 minutes (didn't time it exactly).

bye
	michael

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

* [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment)
  2013-01-29 21:38         ` Michael Stefaniuc
@ 2013-01-29 21:42           ` Julia Lawall
  2013-02-06 14:37           ` Michael Stefaniuc
  1 sibling, 0 replies; 10+ messages in thread
From: Julia Lawall @ 2013-01-29 21:42 UTC (permalink / raw)
  To: cocci

On Tue, 29 Jan 2013, Michael Stefaniuc wrote:

> Hello Julia,
> 
> On 01/28/2013 11:04 PM, Julia Lawall wrote:
> > On Sun, 27 Jan 2013, Michael Stefaniuc wrote:
> > 
> >> Hello Peter,
> >>
> >> On 01/26/2013 02:28 AM, Peter Senna Tschudin wrote:
> >>> Thank you for testing.
> >>>
> >>> I made changes on the semantic patch. I believe it fixed the problem.
> >>> Can you test it?
> >> while it does fix the issue it makes the cocci script less readable and
> >> works only in that particular case. Not sure it is worth the effort as
> >> the bad generated patch can be trivially detected and easily fixed
> >> manually. My email was more of a bug report / feature request for
> >> coccinelle than an improvement request to the script.
> >>
> >> Though I've noticed an other issue with the script: Any reason you use
> >> "Options: --no-includes"? While it does speed up things it misses quite
> >> a few cases as a lot of types are defined in the headers.
> > 
> > I think it should be --all-includes or even --recursive-includes
> no clue what the policy in the Kernel is but --recursive-includes is
> quite heavy handed as a default. I have one --recursive-includes still
> running for Wine and it accumulated until now over 1126 CPU minutes
> (Intel i5 mobile CPU with 2.4 GHz). It found 5 more cases while
> --no-includes found 11 and --local-includes additional 32. The
> --local-includes finished in less than 30 minutes (didn't time it exactly).

OK.  It depends on how much type information is needed.  A compromise 
could be to make a rule without type information that makes clear that one 
has less confidence in the result.  Then the user could just go look up 
the type information if needed.

-all_includes could be good then.  It only includes what is specifically 
referenced in the current file.

julia

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

* [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment)
  2013-01-29 21:38         ` Michael Stefaniuc
  2013-01-29 21:42           ` Julia Lawall
@ 2013-02-06 14:37           ` Michael Stefaniuc
  1 sibling, 0 replies; 10+ messages in thread
From: Michael Stefaniuc @ 2013-02-06 14:37 UTC (permalink / raw)
  To: cocci

On 01/29/2013 10:38 PM, Michael Stefaniuc wrote:
> On 01/28/2013 11:04 PM, Julia Lawall wrote:
>> On Sun, 27 Jan 2013, Michael Stefaniuc wrote:
>>> On 01/26/2013 02:28 AM, Peter Senna Tschudin wrote:
>>>> Thank you for testing.
>>>>
>>>> I made changes on the semantic patch. I believe it fixed the problem.
>>>> Can you test it?
>>> while it does fix the issue it makes the cocci script less readable and
>>> works only in that particular case. Not sure it is worth the effort as
>>> the bad generated patch can be trivially detected and easily fixed
>>> manually. My email was more of a bug report / feature request for
>>> coccinelle than an improvement request to the script.
>>>
>>> Though I've noticed an other issue with the script: Any reason you use
>>> "Options: --no-includes"? While it does speed up things it misses quite
>>> a few cases as a lot of types are defined in the headers.
>>
>> I think it should be --all-includes or even --recursive-includes
> no clue what the policy in the Kernel is but --recursive-includes is
> quite heavy handed as a default. I have one --recursive-includes still
> running for Wine and it accumulated until now over 1126 CPU minutes
The --recursive-includes run finally finished after accumulating 4240
minutes of CPU time. There was no visible slowdown nor memory leak
during the run which is pretty cool.

> (Intel i5 mobile CPU with 2.4 GHz). It found 5 more cases while
> --no-includes found 11 and --local-includes additional 32. The
> --local-includes finished in less than 30 minutes (didn't time it exactly).

bye
	michael

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

* [Cocci] [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment
  2013-01-23 22:06 [Cocci] [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment Peter Senna Tschudin
  2013-01-24 17:47 ` [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment) Michael Stefaniuc
@ 2013-02-22 10:25 ` Michal Marek
  1 sibling, 0 replies; 10+ messages in thread
From: Michal Marek @ 2013-02-22 10:25 UTC (permalink / raw)
  To: cocci

On Wed, Jan 23, 2013 at 08:06:30PM -0200, Peter Senna Tschudin wrote:
> There are error-prone memcpy() that can be replaced by struct
> assignment that are type-safe and much easier to read. This semantic
> patch looks for memcpy() that can be replaced by struct assignment.
> 
> Inspired by patches sent by Ezequiel Garcia <elezegarcia@gmail.com>
> 
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
> Changes from V1:
>  Updated commit message
>  Changed Confidence comment to High on the semantic patch

Applied to kbuild.git#misc, thanks.

Michal

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

end of thread, other threads:[~2013-02-22 10:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-23 22:06 [Cocci] [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment Peter Senna Tschudin
2013-01-24 17:47 ` [Cocci] Prepending '*' to an expression (Was: [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment) Michael Stefaniuc
2013-01-24 18:05   ` Julia Lawall
2013-01-26  1:28   ` Peter Senna Tschudin
2013-01-27 20:46     ` Michael Stefaniuc
2013-01-28 22:04       ` Julia Lawall
2013-01-29 21:38         ` Michael Stefaniuc
2013-01-29 21:42           ` Julia Lawall
2013-02-06 14:37           ` Michael Stefaniuc
2013-02-22 10:25 ` [Cocci] [PATCH V2] scripts/coccinelle/misc/memcpy-assign.cocci: Replace memcpy with struct assignment Michal Marek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox