[Cocci] Checking the “display” of last two statements in code blocks

[Cocci] Checking the “display” of last two statements in code blocks

[Markus Elfring]

Hello,

I have tried another small SmPL script out.


@display@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 ... when any
*{
    ... when any
*   s1
*   s2
*}
 ... when any
 }


I have observed then that the following diff hunk was generated.


elfring@Sonne:~/Projekte/Linux/next-patched> spatch ~/Projekte/Coccinelle/janitor/show_last_two_statements_in_a_block.cocci drivers/net/ethernet/ti/netcp_ethss.c|less
…
@@ -3015,11 +2859,6 @@ static int init_slave(struct gbe_priv *g
        }

        if (of_property_read_u32(node, "link-interface",
-                                &slave->link_interface)) {
-               dev_warn(gbe_dev->dev,
-                        "missing link-interface value defaulting to 1G mac-phy link\n");
-               slave->link_interface = SGMII_LINK_MAC_PHY;
-       }

        slave->node = node;
        slave->open = false;
…


Now I wonder about the suggested change for the if statement here.
Should any more software adjustments be considered so that it will be avoided
to delete only a questionable part from a function call within a condition check?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Checking the “display” of last two statements in code blocks

[Julia Lawall]

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



On Fri, 12 Apr 2019, Markus Elfring wrote:

> Hello,
>
> I have tried another small SmPL script out.
>
>
> @display@
> identifier work;
> statement s1, s2;
> type T;
> @@
>  T work(...)
>  {
>  ... when any
> *{
>     ... when any
> *   s1
> *   s2
> *}
>  ... when any
>  }
>
>
> I have observed then that the following diff hunk was generated.
>
>
> elfring@Sonne:~/Projekte/Linux/next-patched> spatch ~/Projekte/Coccinelle/janitor/show_last_two_statements_in_a_block.cocci drivers/net/ethernet/ti/netcp_ethss.c|less
> …
> @@ -3015,11 +2859,6 @@ static int init_slave(struct gbe_priv *g
>         }
>
>         if (of_property_read_u32(node, "link-interface",
> -                                &slave->link_interface)) {
> -               dev_warn(gbe_dev->dev,
> -                        "missing link-interface value defaulting to 1G mac-phy link\n");
> -               slave->link_interface = SGMII_LINK_MAC_PHY;
> -       }
>
>         slave->node = node;
>         slave->open = false;
> …
>
>
> Now I wonder about the suggested change for the if statement here.
> Should any more software adjustments be considered so that it will be avoided
> to delete only a questionable part from a function call within a condition check?

I don't understand the problem.  You put *s on the { } and two statements,
and you get - on the lines that contain the { } and the two statements.
Nothing is being deleted anyway.  The -s are just a concise way to
indicate the lines on which you requested a match with the *s.  If you
want something more fine grained, use position variables and implement the
display you want in python code.

julia

>
> Regards,
> Markus
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Checking the “display” of last two statements in code blocks

[Markus Elfring]

>> @@ -3015,11 +2859,6 @@ static int init_slave(struct gbe_priv *g
>>         }
>>
>>         if (of_property_read_u32(node, "link-interface",
>> -                                &slave->link_interface)) {
>> -               dev_warn(gbe_dev->dev,
>> -                        "missing link-interface value defaulting to 1G mac-phy link\n");
>> -               slave->link_interface = SGMII_LINK_MAC_PHY;
>> -       }
>>
>>         slave->node = node;
>>         slave->open = false;
>> …
>>
>>
>> Now I wonder about the suggested change for the if statement here.
>> Should any more software adjustments be considered so that it will be avoided
>> to delete only a questionable part from a function call within a condition check?
>
> I don't understand the problem.

I suggest to take another look at the details in this test result.


> You put *s on the { } and two statements, and you get - on the lines
> that contain the { } and the two statements.

This is the general analysis approach.


> Nothing is being deleted anyway.  The -s are just a concise way to
> indicate the lines on which you requested a match with the *s.

I am using the asterisk functionality of the semantic patch language
as usual.

These minus characters indicate lines for possible deletions according to
the diff format, don't they?


I doubt that such code can be still compiled after an incomplete
removal of the shown block would be applied because of the generated
update suggestion.

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Checking the “display” of last two statements in code blocks

[Julia Lawall]

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



On Fri, 12 Apr 2019, Markus Elfring wrote:

> >> @@ -3015,11 +2859,6 @@ static int init_slave(struct gbe_priv *g
> >>         }
> >>
> >>         if (of_property_read_u32(node, "link-interface",
> >> -                                &slave->link_interface)) {
> >> -               dev_warn(gbe_dev->dev,
> >> -                        "missing link-interface value defaulting to 1G mac-phy link\n");
> >> -               slave->link_interface = SGMII_LINK_MAC_PHY;
> >> -       }
> >>
> >>         slave->node = node;
> >>         slave->open = false;
> >> …
> >>
> >>
> >> Now I wonder about the suggested change for the if statement here.
> >> Should any more software adjustments be considered so that it will be avoided
> >> to delete only a questionable part from a function call within a condition check?
> >
> > I don't understand the problem.
>
> I suggest to take another look at the details in this test result.
>
>
> > You put *s on the { } and two statements, and you get - on the lines
> > that contain the { } and the two statements.
>
> This is the general analysis approach.
>
>
> > Nothing is being deleted anyway.  The -s are just a concise way to
> > indicate the lines on which you requested a match with the *s.
>
> I am using the asterisk functionality of the semantic patch language
> as usual.
>
> These minus characters indicate lines for possible deletions according to
> the diff format, don't they?
>
>
> I doubt that such code can be still compiled after an incomplete
> removal of the shown block would be applied because of the generated
> update suggestion.

The asterisk functionality has no goal of making code that compiles.  It's
goal is to show you the lines of interest.  If you use emacs, you can use
emacs diff mode to jump to those lines in the affected files.

If you want to actually remove the lines, then use - instead of asterisk.
- and + have the goal of making code that compiles.  * does not.

julia

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Clarification for SmPL asterisk functionality

[Markus Elfring]

> The asterisk functionality has no goal of making code that compiles.

How do you think about to add this information to the manual for
the semantic patch language?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] Searching for last two statements in code blocks

[Markus Elfring]

Hello,

I have tried another small SmPL script out.


@display@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 ... when any
 {
 ... when any
*s1
 s2
 }
 ... when any
 }


I have observed that no statements were shown from if branches
in my test source file.
This happened then after the addition of the SmPL specification “if (...)”
before an opening curly bracket in this analysis approach.
Now I am curious if compound statements can be handled also without
the addition of a SmPL disjunction for known C key words.
Can extra code be treated as optional by default there?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] Checking the search for duplicate statements at the end of if branches

[Markus Elfring]

Hello,

I have tried another script out for the evolving semantic patch language.
Source code information is imported into a database table based on
the following SmPL rule.


@searching@
identifier work;
statement s1, s2;
position pos;
type T;
@@
 T work(...)
 {
 ... when any
 if (...)
 {
 ... when any
 s1@pos
 s2
 }
 ... when any
 }



I reduced the original source file to a test file which contains almost
only the most interesting code for this analysis approach.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/ti/netcp_ethss.c?id=75eac7b5f68b0a0671e795ac636457ee27cc11d8

I get an expected test result then.

elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch ../janitor/list_duplicate_statement_pairs_from_if_branches3.cocci netcp_ethss-excerpt3.c
…
Using SQLAlchemy version:
1.3.2
…
statement1|statement2|"function name"|"source file"|incidence
of_node_put ( interfaces ) ;|return ret ;|gbe_probe|netcp_ethss-excerpt3.c|2


I reduced the original source file also to a test file which contains
the complete implementation of the function “gbe_probe”.
I get another unexpected test result then.


elfring@Sonne:~/Projekte/Coccinelle/Probe> spatch ../janitor/list_duplicate_statement_pairs_from_if_branches3.cocci netcp_ethss-excerpt5.c
…
Note: processing took    95.8s: netcp_ethss-excerpt5.c
Duplicate statements were not determined from 1 records.
statement1|statement2|"function name"|"source file"|line
dev_err ( dev , "device tree info unavailable\n" ) ;|return - ENODEV ;|gbe_probe|netcp_ethss-excerpt5.c|12


Now I wonder about this software behaviour.
How are the chances to clarify the shown situation further?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] Checking execution environment for Python code in SmPL scripts

[Markus Elfring]

Hello,

I constructed a SmPL script which can work as desired.


elfring@Sonne:~/Projekte/Linux/next-patched> spatch drivers/media/dvb-frontends/tda1004x.c ~/Projekte/Coccinelle/janitor/list_duplicate_statement_pairs_from_if_branches3.cocci
…
Using SQLAlchemy version:
1.3.2
…
statement1|statement2|"function name"|"source file"|incidence
kfree ( state ) ;|return NULL ;|tda10045_attach|drivers/media/dvb-frontends/tda1004x.c|2
kfree ( state ) ;|return NULL ;|tda10046_attach|drivers/media/dvb-frontends/tda1004x.c|2


Unfortunately, I get another unexpected test result by the following command.


elfring@Sonne:~/Projekte/Linux/next-patched> spatch -D URL=postgresql+psycopg2:///media_duplicates1 drivers/media/dvb-frontends/tda1004x.c ~/Projekte/Coccinelle/janitor/list_duplicate_statement_pairs_from_if_branches4.cocci
…
Traceback (most recent call last):
  File "<string>", line 4, in <module>
NameError: name 'store_statements' is not defined
exn while in timeout_function
…


The following display shows intended differences between these SmPL scripts.


 @initialize:python@
+URL << virtual.database_URL;
 @@
 import sqlalchemy, sys
…
-engine = create_engine("sqlite:///:memory:", echo=False)
+
+if URL == False or URL == "default":
+   URL = "sqlite:///result-duplicates.db"
+
+engine = create_engine(URL, echo = False)


The custom function “store_statements” should be usable in both script variants.
Now I wonder about the shown software behaviour again. How can it be fixed?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Checking execution environment for Python code in SmPL scripts

[Julia Lawall]

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



On Sat, 20 Apr 2019, Markus Elfring wrote:

> Hello,
>
> I constructed a SmPL script which can work as desired.
>
>
> elfring@Sonne:~/Projekte/Linux/next-patched> spatch drivers/media/dvb-frontends/tda1004x.c ~/Projekte/Coccinelle/janitor/list_duplicate_statement_pairs_from_if_branches3.cocci
> …
> Using SQLAlchemy version:
> 1.3.2
> …
> statement1|statement2|"function name"|"source file"|incidence
> kfree ( state ) ;|return NULL ;|tda10045_attach|drivers/media/dvb-frontends/tda1004x.c|2
> kfree ( state ) ;|return NULL ;|tda10046_attach|drivers/media/dvb-frontends/tda1004x.c|2
>
>
> Unfortunately, I get another unexpected test result by the following command.
>
>
> elfring@Sonne:~/Projekte/Linux/next-patched> spatch -D URL=postgresql+psycopg2:///media_duplicates1 drivers/media/dvb-frontends/tda1004x.c ~/Projekte/Coccinelle/janitor/list_duplicate_statement_pairs_from_if_branches4.cocci
> …
> Traceback (most recent call last):
>   File "<string>", line 4, in <module>
> NameError: name 'store_statements' is not defined
> exn while in timeout_function
> …
>
>
> The following display shows intended differences between these SmPL scripts.
>
>
>  @initialize:python@
> +URL << virtual.database_URL;

On the command line, your -D argument is URL, but here the variable is
called database_URL.  If you don't provide the variables needed by the
initialize rule, the initialize rule doesn't run.

julia


>  @@
>  import sqlalchemy, sys
> …
> -engine = create_engine("sqlite:///:memory:", echo=False)
> +
> +if URL == False or URL == "default":
> +   URL = "sqlite:///result-duplicates.db"
> +
> +engine = create_engine(URL, echo = False)
>
>
> The custom function “store_statements” should be usable in both script variants.
> Now I wonder about the shown software behaviour again. How can it be fixed?
>
> Regards,
> Markus
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Checking execution environment for Python code in SmPL scripts

[Markus Elfring]

>>  @initialize:python@
>> +URL << virtual.database_URL;
>
> On the command line, your -D argument is URL, but here the variable is
> called database_URL.

I am sorry that I have overlooked the identifier mismatch here.


> If you don't provide the variables needed by the initialize rule,
> the initialize rule doesn't run.

Thanks for your explanation.

I would appreciate a more appropriate error message for such use cases.

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Checking execution environment for Python code in SmPL scripts

[Julia Lawall]

On Sat, 20 Apr 2019, Markus Elfring wrote:

> >>  @initialize:python@
> >> +URL << virtual.database_URL;
> >
> > On the command line, your -D argument is URL, but here the variable is
> > called database_URL.
>
> I am sorry that I have overlooked the identifier mismatch here.
>
>
> > If you don't provide the variables needed by the initialize rule,
> > the initialize rule doesn't run.
>
> Thanks for your explanation.
>
> I would appreciate a more appropriate error message for such use cases.

Coccinelle doesn't know what you intend.  Executing the initialize rule is
not obligatory.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Checking execution environment for Python code in SmPL scripts

[Markus Elfring]

>> I would appreciate a more appropriate error message for such use cases.
>
> Coccinelle doesn't know what you intend.  Executing the initialize rule is
> not obligatory.

Can execution of the specified initialisation code become mandatory?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Searching for last two statements in code blocks

[Markus Elfring]

Hello,

I have tried another tiny SmPL script out.

@display@
statement s1, s2;
type T;
@@
 T nouveau_bo_alloc(...)
 {
 ... when any
 {
    ... when any
*   s1
*   s2
 }
 ... when any
 }


Such a search pattern can point source code places out which can trigger
further development considerations.
https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/gpu/drm/nouveau/nouveau_bo.c#L194
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/nouveau/nouveau_bo.c?id=3b7c59a1950c75f2c0152e5a9cd77675b09233d6#n194

System details can probably trigger also collateral evolution for
the semantic patch language (Coccinelle software).
The metavariable type “statement” supports to find C statements
according to known categories.
* Compound
* Iteration
* Selection

Can this type be restricted to C statements according to the categories
“expression” and “jump” by a way which might be simpler than the specification
of similar SmPL disjunctions?
https://en.cppreference.com/w/c/language/statements

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci