[PATCH 0/3] Coccinelle: pci_free_consistent: Adjustments for a SmPL script

[PATCH 0/3] Coccinelle: pci_free_consistent: Adjustments for a SmPL script

[Markus Elfring]

This script for the semantic patch language contains implementation details
which can be improved.

Markus Elfring (3):
  Use formatted strings directly in SmPL rules
  Reduce a bit of duplicate SmPL code
  Extend when constraints for two SmPL ellipses

 .../coccinelle/free/pci_free_consistent.cocci | 28 ++++++++++---------
 1 file changed, 15 insertions(+), 13 deletions(-)

--
2.21.0

[PATCH 1/3] Coccinelle: pci_free_consistent: Use formatted strings directly in SmPL rules

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 14 May 2019 16:54:40 +0200

Formatted strings were always assigned to the Python variable “msg”
before they were used in two rules of a script for the semantic
patch language.
Delete these extra variables so that the specified string objects
are directly used for the desired data output.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 scripts/coccinelle/free/pci_free_consistent.cocci | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/coccinelle/free/pci_free_consistent.cocci b/scripts/coccinelle/free/pci_free_consistent.cocci
index 43600ccb62a8..2056d6680cb8 100644
--- a/scripts/coccinelle/free/pci_free_consistent.cocci
+++ b/scripts/coccinelle/free/pci_free_consistent.cocci
@@ -38,15 +38,15 @@ return@p2 ...;
 p1 << search.p1;
 p2 << search.p2;
 @@
-
-msg = "ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s and return without freeing on line %s" % (p1[0].line,p2[0].line)
-coccilib.report.print_report(p2[0],msg)
+coccilib.report.print_report(p2[0],
+                             "ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s and return without freeing on line %s"
+                             % (p1[0].line,p2[0].line))

 @script:python depends on org@
 p1 << search.p1;
 p2 << search.p2;
 @@
-
-msg = "ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s and return without freeing on line %s" % (p1[0].line,p2[0].line)
-cocci.print_main(msg,p1)
+cocci.print_main("ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s and return without freeing on line %s"
+                 % (p1[0].line,p2[0].line),
+                 p1)
 cocci.print_secs("",p2)
--
2.21.0

Re: [PATCH 1/3] Coccinelle: pci_free_consistent: Use formatted strings directly in SmPL rules

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]



On Tue, 14 May 2019, Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 14 May 2019 16:54:40 +0200
>
> Formatted strings were always assigned to the Python variable “msg”
> before they were used in two rules of a script for the semantic
> patch language.
> Delete these extra variables so that the specified string objects
> are directly used for the desired data output.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

NACK.  If the code is going to change, I would rather it come closer to
staying within 80 characters.

julia


> ---
>  scripts/coccinelle/free/pci_free_consistent.cocci | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/coccinelle/free/pci_free_consistent.cocci b/scripts/coccinelle/free/pci_free_consistent.cocci
> index 43600ccb62a8..2056d6680cb8 100644
> --- a/scripts/coccinelle/free/pci_free_consistent.cocci
> +++ b/scripts/coccinelle/free/pci_free_consistent.cocci
> @@ -38,15 +38,15 @@ return@p2 ...;
>  p1 << search.p1;
>  p2 << search.p2;
>  @@
> -
> -msg = "ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s and return without freeing on line %s" % (p1[0].line,p2[0].line)
> -coccilib.report.print_report(p2[0],msg)
> +coccilib.report.print_report(p2[0],
> +                             "ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s and return without freeing on line %s"
> +                             % (p1[0].line,p2[0].line))
>
>  @script:python depends on org@
>  p1 << search.p1;
>  p2 << search.p2;
>  @@
> -
> -msg = "ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s and return without freeing on line %s" % (p1[0].line,p2[0].line)
> -cocci.print_main(msg,p1)
> +cocci.print_main("ERROR: missing pci_free_consistent; pci_alloc_consistent on line %s and return without freeing on line %s"
> +                 % (p1[0].line,p2[0].line),
> +                 p1)
>  cocci.print_secs("",p2)
> --
> 2.21.0
>
>

Re: [1/3] Coccinelle: pci_free_consistent: Use formatted strings directly in SmPL rules

[Markus Elfring]

> NACK.  If the code is going to change, I would rather it come closer to
> staying within 80 characters.

Do you really prefer then to deviate from the Linux coding style
at such source code places?

Would you like to split affected string literals over multiple lines?

Regards,
Markus

Re: [1/3] Coccinelle: pci_free_consistent: Use formatted strings directly in SmPL rules

[Julia Lawall]

On Wed, 15 May 2019, Markus Elfring wrote:

> > NACK.  If the code is going to change, I would rather it come closer to
> > staying within 80 characters.
>
> Do you really prefer then to deviate from the Linux coding style
> at such source code places?
>
> Would you like to split affected string literals over multiple lines?

No, I want msg = ... so that the string starts earlier and ends earlier.

julia

[PATCH 2/3] Coccinelle: pci_free_consistent: Reduce a bit of duplicate SmPL code

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 14 May 2019 17:18:24 +0200

A return statement was specified with a known value for three branches
of a SmPL disjunction.
Reduce duplicate SmPL code there by using another disjunction for
these return values.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 scripts/coccinelle/free/pci_free_consistent.cocci | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/coccinelle/free/pci_free_consistent.cocci b/scripts/coccinelle/free/pci_free_consistent.cocci
index 2056d6680cb8..45bc14ece151 100644
--- a/scripts/coccinelle/free/pci_free_consistent.cocci
+++ b/scripts/coccinelle/free/pci_free_consistent.cocci
@@ -25,11 +25,11 @@ if (id == NULL || ...) { ... return ...; }
     when != e = (T)id
     when exists
 (
-return 0;
-|
-return 1;
-|
-return id;
+ return
+(0
+|1
+|id
+);
 |
 return@p2 ...;
 )
--
2.21.0

Re: [PATCH 2/3] Coccinelle: pci_free_consistent: Reduce a bit of duplicate SmPL code

[Julia Lawall]

On Tue, 14 May 2019, Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 14 May 2019 17:18:24 +0200
>
> A return statement was specified with a known value for three branches
> of a SmPL disjunction.
> Reduce duplicate SmPL code there by using another disjunction for
> these return values.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

NACK.  The goak is not to squeeze the most information into the fewest
number of characters.  The rule was fine as it was.

julia


> ---
>  scripts/coccinelle/free/pci_free_consistent.cocci | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/coccinelle/free/pci_free_consistent.cocci b/scripts/coccinelle/free/pci_free_consistent.cocci
> index 2056d6680cb8..45bc14ece151 100644
> --- a/scripts/coccinelle/free/pci_free_consistent.cocci
> +++ b/scripts/coccinelle/free/pci_free_consistent.cocci
> @@ -25,11 +25,11 @@ if (id == NULL || ...) { ... return ...; }
>      when != e = (T)id
>      when exists
>  (
> -return 0;
> -|
> -return 1;
> -|
> -return id;
> + return
> +(0
> +|1
> +|id
> +);
>  |
>  return@p2 ...;
>  )
> --
> 2.21.0
>
>

Re: [2/3] Coccinelle: pci_free_consistent: Reduce a bit of duplicate SmPL code

[Markus Elfring]

>> A return statement was specified with a known value for three branches
>> of a SmPL disjunction.
>> Reduce duplicate SmPL code there by using another disjunction for
>> these return values.
…
> NACK.  The goak is not to squeeze the most information into the fewest
> number of characters.

Can you accept any other formatting for the adjusted SmPL code?


> The rule was fine as it was.

Can different run time characteristics become relevant here?

Regards,
Markus

Re: [2/3] Coccinelle: pci_free_consistent: Reduce a bit of duplicate SmPL code

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]



On Wed, 15 May 2019, Markus Elfring wrote:

> >> A return statement was specified with a known value for three branches
> >> of a SmPL disjunction.
> >> Reduce duplicate SmPL code there by using another disjunction for
> >> these return values.
> …
> > NACK.  The goak is not to squeeze the most information into the fewest
> > number of characters.
>
> Can you accept any other formatting for the adjusted SmPL code?

No.  It's fine as is.

>
>
> > The rule was fine as it was.
>
> Can different run time characteristics become relevant here?

No.

julia

Re: [2/3] Coccinelle: pci_free_consistent: Reduce a bit of duplicate SmPL code

[Julia Lawall]

> Can different run time characteristics become relevant here?

Internally, they should be identical.

julia

Re: [2/3] Coccinelle: pci_free_consistent: Reduce a bit of duplicate SmPL code

[Markus Elfring]

>> Can different run time characteristics become relevant here?
>
> Internally, they should be identical.

I got other imaginations in this area.

I assume that the evaluation of a separate SmPL disjunction
for the shown return values is occasionally nicer than
the repeated matching for the corresponding complete statement.
It might be that the possible run time difference will not be
big enough for you.

Regards,
Markus

Re: Coccinelle: Handling of SmPL disjunctions

[Markus Elfring]

>>>> Can different run time characteristics become relevant here?
>>>
>>> Internally, they should be identical.
>>
>> I got other imaginations in this area.
>
> You imagination doesn't matter in this case.

They might result in collateral software evolution.


> What matters is what the code does.

At the moment then …


> It expands out disjunctions on expressions to the statement level.> So the internal representation is the same.

Thanks for such additional information. Is it represented in the software
documentation (besides the source code format)?

How do you think about to increase the matching granularity
for this functionality?

Regards,
Markus

Re: Coccinelle: Handling of SmPL disjunctions

[Markus Elfring]

>> Thanks for such additional information. Is it represented in the software
>> documentation (besides the source code format)?
>
> It is not a concern of the user.

I got interested in corresponding aspects.


> The documentation is not going to contain a description of every line
> in the implementation.

Some extensions can become nice and more helpful there.


>> How do you think about to increase the matching granularity
>> for this functionality?
>
> No idea what this means.  Disjunctions are expanded up to the level of the
> nodes in the control-flow graph.

We have got different expectations for working with such nodes
for possible (data flow) analysis.

Does the term “disjunction lifting” (from the SmPL manual) fit to this feedback?

Regards,
Markus

Re: Coccinelle: Handling of SmPL disjunctions

[Markus Elfring]

>>> How do you think about to increase the matching granularity
>>> for this functionality?
>>
>> No idea what this means.  Disjunctions are expanded up to the level of the
>> nodes in the control-flow graph.
>
> We have got different expectations for working with such nodes
> for possible (data flow) analysis.

Will the chances become better to clarify this functionality?

Which software components are involved so far?

Regards,
Markus

[PATCH 2/3] Coccinelle: pci_free_consistent: Reduce a bit of duplicate SmPL code

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 14 May 2019 17:18:24 +0200

A return statement was specified with a known value for three branches
of a SmPL disjunction.
Reduce duplicate SmPL code there by using another disjunction for
these return values.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 scripts/coccinelle/free/pci_free_consistent.cocci | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/coccinelle/free/pci_free_consistent.cocci b/scripts/coccinelle/free/pci_free_consistent.cocci
index 2056d6680cb8..45bc14ece151 100644
--- a/scripts/coccinelle/free/pci_free_consistent.cocci
+++ b/scripts/coccinelle/free/pci_free_consistent.cocci
@@ -25,11 +25,11 @@ if (id == NULL || ...) { ... return ...; }
     when != e = (T)id
     when exists
 (
-return 0;
-|
-return 1;
-|
-return id;
+ return
+(0
+|1
+|id
+);
 |
 return@p2 ...;
 )
--
2.21.0

[PATCH 3/3] Coccinelle: pci_free_consistent: Extend when constraints for two SmPL ellipses

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 14 May 2019 18:12:15 +0200

A SmPL ellipsis was specified for a search approach so that additional
source code would be tolerated between an assignment to a local variable
and the corresponding null pointer check.

But such code should be restricted.
* The local variable must not be reassigned there.
* It must also not be forwarded to an other assignment target.

Take additional casts for these code exclusion specifications into account
together with optional parentheses.

Fixes: f7b167113753e95ae61383e234f8d10142782ace ("scripts: Coccinelle script for pci_free_consistent()")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 scripts/coccinelle/free/pci_free_consistent.cocci | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/coccinelle/free/pci_free_consistent.cocci b/scripts/coccinelle/free/pci_free_consistent.cocci
index 45bc14ece151..48a36adfa3ce 100644
--- a/scripts/coccinelle/free/pci_free_consistent.cocci
+++ b/scripts/coccinelle/free/pci_free_consistent.cocci
@@ -13,13 +13,15 @@ virtual org
 local idexpression id;
 expression x,y,z,e;
 position p1,p2;
-type T;
+type T,T2,T3,T4;
 @@

 id = pci_alloc_consistent@p1(x,y,&z)
-... when != e = id
+ ... when != id = (T2)(e)
+     when != e = (T3)(id)
 if (id == NULL || ...) { ... return ...; }
 ... when != pci_free_consistent(x,y,id,z)
+    when != id = (T4)(e)
     when != if (id) { ... pci_free_consistent(x,y,id,z) ... }
     when != if (y) { ... pci_free_consistent(x,y,id,z) ... }
     when != e = (T)id
--
2.21.0

Re: [PATCH 3/3] Coccinelle: pci_free_consistent: Extend when constraints for two SmPL ellipses

[Julia Lawall]

On Tue, 14 May 2019, Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 14 May 2019 18:12:15 +0200
>
> A SmPL ellipsis was specified for a search approach so that additional
> source code would be tolerated between an assignment to a local variable
> and the corresponding null pointer check.
>
> But such code should be restricted.
> * The local variable must not be reassigned there.
> * It must also not be forwarded to an other assignment target.
>
> Take additional casts for these code exclusion specifications into account
> together with optional parentheses.

I leave this up to the ZTE people.

julia


>
> Fixes: f7b167113753e95ae61383e234f8d10142782ace ("scripts: Coccinelle script for pci_free_consistent()")
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  scripts/coccinelle/free/pci_free_consistent.cocci | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/coccinelle/free/pci_free_consistent.cocci b/scripts/coccinelle/free/pci_free_consistent.cocci
> index 45bc14ece151..48a36adfa3ce 100644
> --- a/scripts/coccinelle/free/pci_free_consistent.cocci
> +++ b/scripts/coccinelle/free/pci_free_consistent.cocci
> @@ -13,13 +13,15 @@ virtual org
>  local idexpression id;
>  expression x,y,z,e;
>  position p1,p2;
> -type T;
> +type T,T2,T3,T4;
>  @@
>
>  id = pci_alloc_consistent@p1(x,y,&z)
> -... when != e = id
> + ... when != id = (T2)(e)
> +     when != e = (T3)(id)
>  if (id == NULL || ...) { ... return ...; }
>  ... when != pci_free_consistent(x,y,id,z)
> +    when != id = (T4)(e)
>      when != if (id) { ... pci_free_consistent(x,y,id,z) ... }
>      when != if (y) { ... pci_free_consistent(x,y,id,z) ... }
>      when != e = (T)id
> --
> 2.21.0
>
>