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
From: Markus Elfring <[email protected]>
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 <[email protected]>
---
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
From: Markus Elfring <[email protected]>
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 <[email protected]>
---
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
From: Markus Elfring <[email protected]>
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 <[email protected]>
---
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
From: Markus Elfring <[email protected]>
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 <[email protected]>
---
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
On Tue, 14 May 2019, Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> 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 <[email protected]>
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
>
>
On Tue, 14 May 2019, Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> 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 <[email protected]>
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
>
>
On Tue, 14 May 2019, Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> 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 <[email protected]>
> ---
> 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
>
>
> 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
>> 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
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
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
> Can different run time characteristics become relevant here?
Internally, they should be identical.
julia
>> 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