2019-05-13 09:15:53

by Markus Elfring

[permalink] [raw]
Subject: [PATCH 3/5] Coccinelle: put_device: Merge four SmPL when constraints into one

From: Markus Elfring <[email protected]>
Date: Sun, 12 May 2019 18:32:46 +0200

An assignment target was repeated in four SmPL when constraints.
Combine the exclusion specifications into disjunctions for the semantic
patch language so that this target is referenced only once there.

Signed-off-by: Markus Elfring <[email protected]>
---
scripts/coccinelle/free/put_device.cocci | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/scripts/coccinelle/free/put_device.cocci b/scripts/coccinelle/free/put_device.cocci
index 120921366e84..aae79c02c1e0 100644
--- a/scripts/coccinelle/free/put_device.cocci
+++ b/scripts/coccinelle/free/put_device.cocci
@@ -23,10 +23,7 @@ if (id == NULL || ...) { ... return ...; }
when != platform_device_put(id)
when != of_dev_put(id)
when != if (id) { ... put_device(&id->dev) ... }
- when != e1 = (T)id
- when != e1 = (T)(&id->dev)
- when != e1 = get_device(&id->dev)
- when != e1 = (T1)platform_get_drvdata(id)
+ when != e1 = \( (T) \( id \| (&id->dev) \) \| get_device(&id->dev) \| (T1)platform_get_drvdata(id) \)
(
return
( id
--
2.21.0


2019-05-13 10:22:22

by Julia Lawall

[permalink] [raw]
Subject: Re: [PATCH 3/5] Coccinelle: put_device: Merge four SmPL when constraints into one



On Mon, 13 May 2019, Markus Elfring wrote:

> From: Markus Elfring <[email protected]>
> Date: Sun, 12 May 2019 18:32:46 +0200
>
> An assignment target was repeated in four SmPL when constraints.
> Combine the exclusion specifications into disjunctions for the semantic
> patch language so that this target is referenced only once there.

NACK. This exceeds 80 characters and provides no readability benefit.

julia

>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
> scripts/coccinelle/free/put_device.cocci | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/scripts/coccinelle/free/put_device.cocci b/scripts/coccinelle/free/put_device.cocci
> index 120921366e84..aae79c02c1e0 100644
> --- a/scripts/coccinelle/free/put_device.cocci
> +++ b/scripts/coccinelle/free/put_device.cocci
> @@ -23,10 +23,7 @@ if (id == NULL || ...) { ... return ...; }
> when != platform_device_put(id)
> when != of_dev_put(id)
> when != if (id) { ... put_device(&id->dev) ... }
> - when != e1 = (T)id
> - when != e1 = (T)(&id->dev)
> - when != e1 = get_device(&id->dev)
> - when != e1 = (T1)platform_get_drvdata(id)
> + when != e1 = \( (T) \( id \| (&id->dev) \) \| get_device(&id->dev) \| (T1)platform_get_drvdata(id) \)
> (
> return
> ( id
> --
> 2.21.0
>
>

2019-05-13 12:44:51

by Markus Elfring

[permalink] [raw]
Subject: Re: [3/5] Coccinelle: put_device: Merge four SmPL when constraints into one

>> An assignment target was repeated in four SmPL when constraints.
>> Combine the exclusion specifications into disjunctions for the semantic
>> patch language so that this target is referenced only once there.
>
> NACK.

I find this rejection questionable.


> This exceeds 80 characters

The line became 105 characters long.
14 space characters can eventually be omitted.


> and provides no readability benefit.

I try to stress SmPL functionality in this use case.


>> +++ b/scripts/coccinelle/free/put_device.cocci
>> @@ -23,10 +23,7 @@ if (id == NULL || ...) { ... return ...; }
>> when != platform_device_put(id)
>> when != of_dev_put(id)
>> when != if (id) { ... put_device(&id->dev) ... }
>> - when != e1 = (T)id
>> - when != e1 = (T)(&id->dev)
>> - when != e1 = get_device(&id->dev)
>> - when != e1 = (T1)platform_get_drvdata(id)
>> + when != e1 = \( (T) \( id \| (&id->dev) \) \| get_device(&id->dev) \| (T1)platform_get_drvdata(id) \)

How do you think about to extend the Coccinelle software in the way
that such a detailed constraint can be specified on separate lines
(without duplicated SmPL code)?

How long will it take to reconsider corresponding software limitations?

Regards,
Markus

2019-05-13 12:47:39

by Markus Elfring

[permalink] [raw]
Subject: Re: [3/5] Coccinelle: put_device: Merge four SmPL when constraints into one

>> An assignment target was repeated in four SmPL when constraints.
>> Combine the exclusion specifications into disjunctions for the semantic
>> patch language so that this target is referenced only once there.
>
> NACK.

I find this rejection questionable.


> This exceeds 80 characters

The line became 105 characters long.
14 space characters can eventually be omitted.


> and provides no readability benefit.

I try to stress SmPL functionality in this use case.


>> +++ b/scripts/coccinelle/free/put_device.cocci
>> @@ -23,10 +23,7 @@ if (id == NULL || ...) { ... return ...; }
>> when != platform_device_put(id)
>> when != of_dev_put(id)
>> when != if (id) { ... put_device(&id->dev) ... }
>> - when != e1 = (T)id
>> - when != e1 = (T)(&id->dev)
>> - when != e1 = get_device(&id->dev)
>> - when != e1 = (T1)platform_get_drvdata(id)
>> + when != e1 = \( (T) \( id \| (&id->dev) \) \| get_device(&id->dev) \| (T1)platform_get_drvdata(id) \)

How do you think about to extend the Coccinelle software in the way
that such a detailed constraint can be specified on separate lines
(without duplicated SmPL code)?

How long will it take to reconsider corresponding software limitations?

Regards,
Markus

2019-05-13 14:40:10

by Julia Lawall

[permalink] [raw]
Subject: Re: [3/5] Coccinelle: put_device: Merge four SmPL when constraints into one



On Mon, 13 May 2019, Markus Elfring wrote:

> >> An assignment target was repeated in four SmPL when constraints.
> >> Combine the exclusion specifications into disjunctions for the semantic
> >> patch language so that this target is referenced only once there.
> >
> > NACK.
>
> I find this rejection questionable.
>
>
> > This exceeds 80 characters
>
> The line became 105 characters long.
> 14 space characters can eventually be omitted.
>
>
> > and provides no readability benefit.
>
> I try to stress SmPL functionality in this use case.

That's not the goal of the semantic patches in the kernel.

The rule is fine as it is.

> >> +++ b/scripts/coccinelle/free/put_device.cocci
> >> @@ -23,10 +23,7 @@ if (id == NULL || ...) { ... return ...; }
> >> when != platform_device_put(id)
> >> when != of_dev_put(id)
> >> when != if (id) { ... put_device(&id->dev) ... }
> >> - when != e1 = (T)id
> >> - when != e1 = (T)(&id->dev)
> >> - when != e1 = get_device(&id->dev)
> >> - when != e1 = (T1)platform_get_drvdata(id)
> >> + when != e1 = \( (T) \( id \| (&id->dev) \) \| get_device(&id->dev) \| (T1)platform_get_drvdata(id) \)
>
> How do you think about to extend the Coccinelle software in the way
> that such a detailed constraint can be specified on separate lines
> (without duplicated SmPL code)?

This causes some ambiguities in the parser. No change is likely to occur
here.

julia

>
> How long will it take to reconsider corresponding software limitations?
>
> Regards,
> Markus
>

2019-05-13 15:36:34

by Markus Elfring

[permalink] [raw]
Subject: Re: [3/5] Coccinelle: put_device: Merge four SmPL when constraints into one

>> I try to stress SmPL functionality in this use case.
>
> That's not the goal of the semantic patches in the kernel.
>
> The rule is fine as it is.

I am curious under which circumstances other software aspects
can become more relevant (as suggested).

Regards,
Markus