[Cocci] Comparing statement lists with SmPL

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

Hello,

The Coccinelle software supports metavariables with the type ?statement list?.
Can statement lists be compared if they refer to the same source code structures?
Is this programming interface usable to determine if any code could be combined
to some degree?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[Julia Lawall]

On Tue, 15 Aug 2017, SF Markus Elfring wrote:

> Hello,
>
> The Coccinelle software supports metavariables with the type ?statement list?.
> Can statement lists be compared if they refer to the same source code structures?
> Is this programming interface usable to determine if any code could be combined
> to some degree?

I don't really understand the questions.  Statement list metavariables can
be used once they are bound like any other metavariables.  However they
can only be bound to the complete sequence of statements in a block.  So

@@
statement list sl;
@@
if (x) {
  sl
}

is ok, but

@@
statement list sl;
@@
if (x) {
  one();
  sl
  two();
}

is not.

julia

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

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> Is this programming interface usable to determine if any code could be combined
>> to some degree?
> 
> I don't really understand the questions.

I try again to describe the use case I became interested in a bit more.


> Statement list metavariables can be used once they are bound like any other metavariables.

Can it be that they will need any special computing power occasionally?


I have tried the following small SmPL script out.

@duplicated_code@
identifier work;
statement list sl;
type T;
@@
 T work(...)
 {
 ... when any
*if ((...) < 0) {
*   sl
*}
 ...
*if ((...) < 0) {
*   sl
*}
 ...
 }


The source code analysis results look promising by this command.

elfring@Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && spatch.opt ~/Projekte/Coccinelle/janitor/show_same_statements1.cocci sound/pci/rme9652/hdsp.c
?
@@ -793,11 +793,6 @@ static int hdsp_get_iobox_version (struc
?
@@ -5382,19 +5372,11 @@ static int snd_hdsp_probe(struct pci_dev
?


How should such update candidates be transformed further?


> However they can only be bound to the complete sequence of statements in a block.  So
> 
> @@
> statement list sl;
> @@
> if (x) {
>   sl
> }
> 
> is ok, but
> 
> @@
> statement list sl;
> @@
> if (x) {
>   one();
>   sl
>   two();
> }
> 
> is not.

Thanks for your example.

* I do not like the mentioned software restriction at the moment.

* How can be determined how many code is equivalent between two statement list variables?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[Julia Lawall]

> * How can be determined how many code is equivalent between two statement list variables?

I guess you can make special cases, for one statement, two statements,
etc.

julia

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> * How can be determined how many code is equivalent between two statement list variables?
> 
> I guess you can make special cases, for one statement, two statements, etc.

I guess that we would like to avoid such an approach for some use cases.
I hope that there are safer programming possibilities available.

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

> The source code analysis results look promising by this command.

I have just noticed that an other source file points details out for further
development considerations around the mentioned functionality from the
Coccinelle software.

elfring at Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && spatch.opt ~/Projekte/Coccinelle/janitor/show_same_statements1.cocci sound/pci/rme9652/hdspm.c
?
     (ONCE) already tagged but only removed, so safe
diff = 
?
@@ -6497,9 +6497,6 @@ static int snd_hdspm_create_alsa_devices
 	i = 0;
 	while (i < hdspm->midiPorts) {
 		err = snd_hdspm_create_midi(card, hdspm, i);
-		if (err < 0) {
-			return err;
-		}
 		i++;
 	}
 
@@ -6955,10 +6952,6 @@ static int snd_hdspm_probe(struct pci_de
 	hdspm->pci = pci;
 
 	err = snd_hdspm_create(card, hdspm);
-	if (err < 0) {
-		snd_card_free(card);
-		return err;
-	}
 
 	if (hdspm->io_type != MADIface) {
 		snprintf(card->shortname, sizeof(card->shortname), "%s_%x",
?


Now I am looking for a way to express the constraint that the statement list
metavariable should match only source code with two statements at least.
Can the search for duplicated source code be improved by the means of the
semantic patch language?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[Julia Lawall]

On Tue, 15 Aug 2017, SF Markus Elfring wrote:

> > The source code analysis results look promising by this command.
>
> I have just noticed that an other source file points details out for further
> development considerations around the mentioned functionality from the
> Coccinelle software.
>
> elfring at Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && spatch.opt ~/Projekte/Coccinelle/janitor/show_same_statements1.cocci sound/pci/rme9652/hdspm.c
> ?
>      (ONCE) already tagged but only removed, so safe
> diff =
> ?
> @@ -6497,9 +6497,6 @@ static int snd_hdspm_create_alsa_devices
>  	i = 0;
>  	while (i < hdspm->midiPorts) {
>  		err = snd_hdspm_create_midi(card, hdspm, i);
> -		if (err < 0) {
> -			return err;
> -		}
>  		i++;
>  	}
>
> @@ -6955,10 +6952,6 @@ static int snd_hdspm_probe(struct pci_de
>  	hdspm->pci = pci;
>
>  	err = snd_hdspm_create(card, hdspm);
> -	if (err < 0) {
> -		snd_card_free(card);
> -		return err;
> -	}
>
>  	if (hdspm->io_type != MADIface) {
>  		snprintf(card->shortname, sizeof(card->shortname), "%s_%x",
> ?
>
>
> Now I am looking for a way to express the constraint that the statement list
> metavariable should match only source code with two statements at least.
> Can the search for duplicated source code be improved by the means of the
> semantic patch language?

For two statements at least you could do:

(
{
s1
s2
...
}
&
{
sl
}
)

julia

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> Now I am looking for a way to express the constraint that the statement list
>> metavariable should match only source code with two statements at least.
>> Can the search for duplicated source code be improved by the means of the
>> semantic patch language?
> 
> For two statements at least you could do:
> 
> (
> {
> s1
> s2
> ...
> }
> &
> {
> sl
> }
> )

Thanks for another example.


I admit that I am unused to such a variable combination with a SmPL conjunction.
Will this programming interface evolve any further?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

> However they can only be bound to the complete sequence of statements in a block.  So
> 
> @@
> statement list sl;
> @@
> if (x) {
>   sl
> }
> 
> is ok, but
> 
> @@
> statement list sl;
> @@
> if (x) {
>   one();
>   sl
>   two();
> }
> 
> is not.

How do you think about the usage of a similar SmPL specification?

@display@
statement list sl;
@@
 if (...)
 {
    header();
    { // Extra block
*   sl
    }
    footer();
 }

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[Julia Lawall]

On Wed, 16 Aug 2017, SF Markus Elfring wrote:

> > However they can only be bound to the complete sequence of statements in a block.  So
> >
> > @@
> > statement list sl;
> > @@
> > if (x) {
> >   sl
> > }
> >
> > is ok, but
> >
> > @@
> > statement list sl;
> > @@
> > if (x) {
> >   one();
> >   sl
> >   two();
> > }
> >
> > is not.
>
> How do you think about the usage of a similar SmPL specification?
>
> @display@
> statement list sl;
> @@
>  if (...)
>  {
>     header();
>     { // Extra block
> *   sl
>     }
>     footer();
>  }

It's fine,  A statement list just needs { } around it.

julia

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

> It's fine,  A statement list just needs { } around it.

How much does this aspect influence a source code search when relevant source files
do not contain extra curly brackets?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> Can the search for duplicated source code be improved by the means of the
>> semantic patch language?
> 
> For two statements at least you could do:
> 
> (
> {
> s1
> s2
> ...
> }
> &
> {
> sl
> }
> )

I constructed another SmPL script based on this suggestion as follows.

@duplicated_code@
identifier work;
statement s1, s2;
statement list sl;
type T;
@@
 T work(...)
 {
 ... when any
*if ((...) < 0) {
*   s1
*   s2
    ...
*}
&{
    sl
 }
 ...
*if ((...) < 0) {
*   s1
*   s2
    ...
*}
&{
    sl
 }
 ...
 }


Unfortunately, the software version ?1.0.6-00186-g0acd38ee? does not like this
SmPL specification.

elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch.opt --parse-cocci show_same_statements2.cocci
?
minus: parse error: 
  File "show_same_statements2.cocci", line 15, column 0, charpos = 156
  around = '&',
  whole content = &{


How can my understanding become better also for the use of SmPL conjunctions?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

> Unfortunately, the software version ?1.0.6-00186-g0acd38ee? does not like this
> SmPL specification.

I overlooked to add parentheses in my evolving SmPL script.

@duplicated_code@
identifier work;
statement s1, s2;
statement list sl;
type T;
@@
 T work(...)
 {
 ... when any
(
*if ((...) < 0) {
*   s1
*   s2
    ...
*}
&{
    sl
 }
)
 ...
(
*if ((...) < 0) {
*   s1
*   s2
    ...
*}
&{
    sl
 }
)
 ...
 }


The following command does not show the expected source code analysis results at the moment.

elfring@Sonne:~/Projekte/Coccinelle/janitor> spatch.opt show_same_statements2.cocci ../Probe/hdsp-excerpt1.c


Is there any more clarification needed for the use of SmPL conjunctions?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> Can the search for duplicated source code be improved by the means of the
>> semantic patch language?
> 
> For two statements at least you could do:

An other SmPL script variant can work to some degree.

@duplicated_code@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 ... when any
*if ((...) < 0)
*{
*   s1
*   s2
    ...
*}
 ...
*if ((...) < 0)
*{
*   s1
*   s2
    ...
*}
 ...
 }


But matched statements will not be mapped to the discussed metavariable type
in this use case.
I am curious on how such an approach can be used for corresponding source
code transformations.

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[Julia Lawall]

On Wed, 16 Aug 2017, SF Markus Elfring wrote:

> >> Can the search for duplicated source code be improved by the means of the
> >> semantic patch language?
> >
> > For two statements at least you could do:
> >
> > (
> > {
> > s1
> > s2
> > ...
> > }
> > &
> > {
> > sl
> > }
> > )
>
> I constructed another SmPL script based on this suggestion as follows.
>
> @duplicated_code@
> identifier work;
> statement s1, s2;
> statement list sl;
> type T;
> @@
>  T work(...)
>  {
>  ... when any
> *if ((...) < 0) {
> *   s1
> *   s2
>     ...
> *}
> &{
>     sl
>  }
>  ...
> *if ((...) < 0) {
> *   s1
> *   s2
>     ...
> *}
> &{
>     sl
>  }
>  ...
>  }
>
>
> Unfortunately, the software version ?1.0.6-00186-g0acd38ee? does not like this
> SmPL specification.

The syntax is exactly the same as that of a disjunction

(
&
)

There are no outer parentheses in column 0 in your specification.

julia


>
> elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt --parse-cocci show_same_statements2.cocci
> ?
> minus: parse error:
>   File "show_same_statements2.cocci", line 15, column 0, charpos = 156
>   around = '&',
>   whole content = &{
>
>
> How can my understanding become better also for the use of SmPL conjunctions?
>
> Regards,
> Markus
>

[Cocci] Comparing statement lists with SmPL

[Julia Lawall]

On Wed, 16 Aug 2017, SF Markus Elfring wrote:

> > Unfortunately, the software version ?1.0.6-00186-g0acd38ee? does not like this
> > SmPL specification.
>
> I overlooked to add parentheses in my evolving SmPL script.
>
> @duplicated_code@
> identifier work;
> statement s1, s2;
> statement list sl;
> type T;
> @@
>  T work(...)
>  {
>  ... when any
> (
> *if ((...) < 0) {
> *   s1
> *   s2
>     ...
> *}
> &{
>     sl
>  }
> )
>  ...
> (
> *if ((...) < 0) {
> *   s1
> *   s2
>     ...
> *}
> &{
>     sl
>  }
> )
>  ...
>  }
>
>
> The following command does not show the expected source code analysis results at the moment.
>
> elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt show_same_statements2.cocci ../Probe/hdsp-excerpt1.c

There is no way that I can understand the problme without having the .c
file.

Note that most of your ...s have no annotation.  So they cannot contain
what comes before or after.  I would suggest to put when any on all of the
...s here that are at statement level.

julia

>
>
> Is there any more clarification needed for the use of SmPL conjunctions?
>
> Regards,
> Markus
>

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt show_same_statements2.cocci ../Probe/hdsp-excerpt1.c
> 
> There is no way that I can understand the problme without having the .c file.

I attached this one to a clarification request on the topic
?Improving exception handling a bit with SmPL? two days ago.

Corresponding change possibilities can also be seen in a patch with the subject
?ALSA: rme9652: Use common error handling code in two functions?
which I published on the same day.
https://patchwork.kernel.org/patch/9900021/
https://lkml.kernel.org/r/<fefeea4b-68a2-5b4c-2cfc-cb05383b5474@users.sourceforge.net>


> Note that most of your ...s have no annotation.  So they cannot contain
> what comes before or after.  I would suggest to put when any on all of the
> ...s here that are at statement level.

Thanks for this advice.

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>>> Can the search for duplicated source code be improved by the means of the
>>> semantic patch language?
>>
>> For two statements at least you could do:
> 
> An other SmPL script variant can work to some degree.

How do you think about the relevance of the SmPL construct ?<+... ...+>?
for this use case?


I tried my intermediate SmPL script variant a bit more.

Example:

elfring at Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && spatch.opt ~/Projekte/Coccinelle/janitor/show_same_statements3.cocci fs/ubifs/lpt.c
?
@@ -1974,10 +1974,6 @@ again:
 		int ret, lnum = lprops->lnum;
 
 		ret = scan_cb(c, lprops, path[h].in_tree, data);
-		if (ret < 0) {
-			err = ret;
-			goto out;
-		}
 		if (ret & LPT_SCAN_ADD) {
 			/* Add all the nodes in path to the tree in memory */
 			for (h = 1; h < c->lpt_hght; h++) {


Now I wonder how this test result should fit to my source code search pattern.
I find also more results questionable for this Linux software module.
It seems that there are only two functions which would be an acceptable match.

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[Julia Lawall]

On Thu, 17 Aug 2017, SF Markus Elfring wrote:

> >>> Can the search for duplicated source code be improved by the means of the
> >>> semantic patch language?
> >>
> >> For two statements at least you could do:
> >
> > An other SmPL script variant can work to some degree.
>
> How do you think about the relevance of the SmPL construct ?<+... ...+>?
> for this use case?
>
>
> I tried my intermediate SmPL script variant a bit more.
>
> Example:
>
> elfring at Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && spatch.opt ~/Projekte/Coccinelle/janitor/show_same_statements3.cocci fs/ubifs/lpt.c
> ?
> @@ -1974,10 +1974,6 @@ again:
>  		int ret, lnum = lprops->lnum;
>
>  		ret = scan_cb(c, lprops, path[h].in_tree, data);
> -		if (ret < 0) {
> -			err = ret;
> -			goto out;
> -		}
>  		if (ret & LPT_SCAN_ADD) {
>  			/* Add all the nodes in path to the tree in memory */
>  			for (h = 1; h < c->lpt_hght; h++) {
>
>
> Now I wonder how this test result should fit to my source code search pattern.

I guess there is a loop around this code, so it is reachable from itself.

julia

> I find also more results questionable for this Linux software module.
> It seems that there are only two functions which would be an acceptable match.
>
> Regards,
> Markus
>

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> elfring at Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && spatch.opt ~/Projekte/Coccinelle/janitor/show_same_statements3.cocci fs/ubifs/lpt.c
>> ?
>> @@ -1974,10 +1974,6 @@ again:
>>  		int ret, lnum = lprops->lnum;
>>
>>  		ret = scan_cb(c, lprops, path[h].in_tree, data);
>> -		if (ret < 0) {
>> -			err = ret;
>> -			goto out;
>> -		}
>>  		if (ret & LPT_SCAN_ADD) {
>>  			/* Add all the nodes in path to the tree in memory */
>>  			for (h = 1; h < c->lpt_hght; h++) {
>>
>>
>> Now I wonder how this test result should fit to my source code search pattern.
> 
> I guess there is a loop around this code, so it is reachable from itself.

* How can we achieve progress with such a bit of information?

* Should such false positives be avoided anyhow?

* Will answers to other questions become more helpful again?

* Are there further development challenges to consider for the safe identification
  of unique statements by metavariables?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[Julia Lawall]

On Thu, 17 Aug 2017, SF Markus Elfring wrote:

> >> elfring at Sonne:~/Projekte/Linux/next-patched> git checkout next-20170803 && spatch.opt ~/Projekte/Coccinelle/janitor/show_same_statements3.cocci fs/ubifs/lpt.c
> >> ?
> >> @@ -1974,10 +1974,6 @@ again:
> >>  		int ret, lnum = lprops->lnum;
> >>
> >>  		ret = scan_cb(c, lprops, path[h].in_tree, data);
> >> -		if (ret < 0) {
> >> -			err = ret;
> >> -			goto out;
> >> -		}
> >>  		if (ret & LPT_SCAN_ADD) {
> >>  			/* Add all the nodes in path to the tree in memory */
> >>  			for (h = 1; h < c->lpt_hght; h++) {
> >>
> >>
> >> Now I wonder how this test result should fit to my source code search pattern.
> >
> > I guess there is a loop around this code, so it is reachable from itself.
>
> * How can we achieve progress with such a bit of information?
>
> * Should such false positives be avoided anyhow?
>
> * Will answers to other questions become more helpful again?
>
> * Are there further development challenges to consider for the safe identification
>   of unique statements by metavariables?

If you want to ensure that two metavariables match things in different
places, then put a position variable on each match and use apython rule
afterwards to discard the occurrences that are both in the same position.

julia

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> * Are there further development challenges to consider for the safe identification
>>   of unique statements by metavariables?
> 
> If you want to ensure that two metavariables match things in different
> places, then put a position variable on each match and use apython rule
> afterwards to discard the occurrences that are both in the same position.

I hoped for a more convenient search specification to reduce duplicate
source code a bit.

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> * Are there further development challenges to consider for the safe identification
>>   of unique statements by metavariables?
> 
> If you want to ensure that two metavariables match things
> in different places, then put a position variable on each match
> and use apython rule afterwards to discard the occurrences that
> are both in the same position.

I imagine that such an approach can only matter if there will be
several SmPL rules involved.


I have tried another variant out for a source code analysis.

@duplicated_code@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 <+...
*if ((...) < 0)
*{
    ...
*   s1
*   s2
*}
 ...+>
 <+...
*if ((...) < 0)
*{
    ...
*   s1
*   s2
*}
 ...+>
 }


elfring at Sonne:~/Projekte/Linux/next-patched> XX=$(date) && spatch.opt -timeout 34 -j 4 --chunksize 1 --sp-file ~/Projekte/Coccinelle/janitor/show_same_statements3.cocci --dir sound > ~/Projekte/Bau/Linux/scripts/Coccinelle/tuning1/next/20170803/same_statements3+.diff 2> ~/Projekte/Bau/Linux/scripts/Coccinelle/tuning1/next/20170803/same_statements3+-errors.txt; YY=$(date) && echo "$XX | $YY"
Di 22. Aug 10:24:47 CEST 2017 | Di 22. Aug 10:26:04 CEST 2017


Now I find that a test result like the following is worth for further
development considerations.

?
--- sound/usb/6fire/firmware.c
+++ /tmp/nothing/usb/6fire/firmware.c
@@ -246,14 +246,8 @@ static int usb6fire_fw_ezusb_upload(
 	while (usb6fire_fw_ihex_next_record(rec)) { /* write firmware */
 		ret = usb6fire_fw_ezusb_write(device, 0xa0, rec->address,
 				rec->data, rec->len);
-		if (ret < 0) {
 			kfree(rec);
 			release_firmware(fw);
-			dev_err(&intf->dev,
-				"unable to upload ezusb firmware %s: data urb.\n",
-				fwname);
-			return ret;
-		}
 	}
 
 	release_firmware(fw);
@@ -319,13 +313,8 @@ static int usb6fire_fw_fpga_upload(
 			buffer[i] = bitrev8((u8)*c);
 
 		ret = usb6fire_fw_fpga_write(device, buffer, i);
-		if (ret < 0) {
 			release_firmware(fw);
 			kfree(buffer);
-			dev_err(&intf->dev,
-				"unable to upload fpga firmware: fw urb.\n");
-			return ret;
-		}
 	}
 	release_firmware(fw);
 	kfree(buffer);
?


How would you like to treat variations in such log messages?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[Julia Lawall]

On Tue, 22 Aug 2017, SF Markus Elfring wrote:

> >> * Are there further development challenges to consider for the safe identification
> >>   of unique statements by metavariables?
> >
> > If you want to ensure that two metavariables match things
> > in different places, then put a position variable on each match
> > and use apython rule afterwards to discard the occurrences that
> > are both in the same position.
>
> I imagine that such an approach can only matter if there will be
> several SmPL rules involved.
>
>
> I have tried another variant out for a source code analysis.
>
> @duplicated_code@
> identifier work;
> statement s1, s2;
> type T;
> @@
>  T work(...)
>  {
>  <+...
> *if ((...) < 0)
> *{
>     ...
> *   s1
> *   s2
> *}
>  ...+>
>  <+...
> *if ((...) < 0)
> *{
>     ...
> *   s1
> *   s2
> *}
>  ...+>
>  }
>
>
> elfring at Sonne:~/Projekte/Linux/next-patched> XX=$(date) && spatch.opt -timeout 34 -j 4 --chunksize 1 --sp-file ~/Projekte/Coccinelle/janitor/show_same_statements3.cocci --dir sound > ~/Projekte/Bau/Linux/scripts/Coccinelle/tuning1/next/20170803/same_statements3+.diff 2> ~/Projekte/Bau/Linux/scripts/Coccinelle/tuning1/next/20170803/same_statements3+-errors.txt; YY=$(date) && echo "$XX | $YY"
> Di 22. Aug 10:24:47 CEST 2017 | Di 22. Aug 10:26:04 CEST 2017
>
>
> Now I find that a test result like the following is worth for further
> development considerations.
>
> ?
> --- sound/usb/6fire/firmware.c
> +++ /tmp/nothing/usb/6fire/firmware.c
> @@ -246,14 +246,8 @@ static int usb6fire_fw_ezusb_upload(
>  	while (usb6fire_fw_ihex_next_record(rec)) { /* write firmware */
>  		ret = usb6fire_fw_ezusb_write(device, 0xa0, rec->address,
>  				rec->data, rec->len);
> -		if (ret < 0) {
>  			kfree(rec);
>  			release_firmware(fw);
> -			dev_err(&intf->dev,
> -				"unable to upload ezusb firmware %s: data urb.\n",
> -				fwname);
> -			return ret;
> -		}
>  	}
>
>  	release_firmware(fw);
> @@ -319,13 +313,8 @@ static int usb6fire_fw_fpga_upload(
>  			buffer[i] = bitrev8((u8)*c);
>
>  		ret = usb6fire_fw_fpga_write(device, buffer, i);
> -		if (ret < 0) {
>  			release_firmware(fw);
>  			kfree(buffer);
> -			dev_err(&intf->dev,
> -				"unable to upload fpga firmware: fw urb.\n");
> -			return ret;
> -		}
>  	}
>  	release_firmware(fw);
>  	kfree(buffer);
> ?
>
>
> How would you like to treat variations in such log messages?

They look fine as they are.

julia

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> How would you like to treat variations in such log messages?
> 
> They look fine as they are.

I have got other development opinions there depending on the preferred design goals.

* The mentioned small SmPL script can also help to find special differences.

* These log statements are similar (and not identical) in the code structure.
  I wonder also about other marked code places how they would fit to
  the search pattern.

* A test result was shown from two function implementations.
  It can be nice to compare several functions.
  Is it also possible that the comparison will be only performed within
  the same function bodies?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> * A test result was shown from two function implementations.
>>   It can be nice to compare several functions.
>>   Is it also possible that the comparison will be only performed within
>>   the same function bodies?
> 
> If you make a single rule then it will only applied within individual functions.

How do the presented functions ?usb6fire_fw_ezusb_upload? and ?usb6fire_fw_fpga_upload?
fit to this information?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[Julia Lawall]

On Tue, 22 Aug 2017, SF Markus Elfring wrote:

> >> * A test result was shown from two function implementations.
> >>   It can be nice to compare several functions.
> >>   Is it also possible that the comparison will be only performed within
> >>   the same function bodies?
> >
> > If you make a single rule then it will only applied within individual functions.
>
> How do the presented functions ?usb6fire_fw_ezusb_upload? and ?usb6fire_fw_fpga_upload?
> fit to this information?

The pattern matched in two different functions?

julia

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> How do the presented functions ?usb6fire_fw_ezusb_upload? and ?usb6fire_fw_fpga_upload?
>> fit to this information?
> 
> The pattern matched in two different functions?

Please look once more.

Yes. - This is one of the reasons why I ask here again.

I would like to choose if a code analysis should happen only within a single function
or on interesting parts from the complete source file.

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>>> The pattern matched in two different functions?
>>
>> Please look once more.
>>
>> Yes. - This is one of the reasons why I ask here again.
>>
>> I would like to choose if a code analysis should happen only within a single function
>> or on interesting parts from the complete source file.
> 
> Feel free to make a tool of your own if you are not satisfied with the one
> offered you.

This is also possible. - But I wonder more about specific test results based
on the currently provided software.

I got matches for places where I would expect that they should not match.


> Coccinelle applies a rule to one function at a time.

This is also fine for a while.


> That is not going to change.

I imagine that the situation could evolve more if additional development resources
will appear somehow.


> If you want to match things in different functions, make two rules.

Additional SmPL rules will search for other details.


Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

> I have tried another variant out for a source code analysis.

Does the following SmPL script variant make sense?

@duplicated_code@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 ... when any
*if ((...) < 0)
*{
    ...
*   s1
*   s2
*}
 <+...
*if ((...) < 0)
*{
    ...
*   s1
*   s2
*}
 ...+>
 }


Can this approach find interesting places in any source files?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

> You would need when any on the ...  Otherwise, it will not match anything
> (perhaps declarations). The s1 after is precludes matching any statement.
> Likewise in the next if.

The suggested variant does still not work in the way I would expect it.

@duplicated_code@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 ... when any
*if ((...) < 0)
*{
    ... when any
*   s1
*   s2
*}
 <+...
*if ((...) < 0)
*{
    ... when any
*   s1
*   s2
*}
 ...+>
 }


I got some promising test results from the following SmPL approach.

@duplicated_code@
identifier work;
statement s1, s2;
type T;
@@
 T work(...)
 {
 ... when any
*if ((...) < 0)
*{
    ... when any
*   s1
*   s2
*}
 ... when any
*if ((...) < 0)
*{
    ... when any
*   s1
*   s2
*}
 ... when any
 }


elfring@Sonne:~/Projekte/Linux/next-patched> XX=$(date) && spatch.opt --timeout 12 --sp-file ~/Projekte/Coccinelle/janitor/show_same_statements3b.cocci --dir sound > ~/Projekte/Bau/Linux/scripts/Coccinelle/tuning1/next/20170803/same_statements3b.diff 2> ~/Projekte/Bau/Linux/scripts/Coccinelle/tuning1/next/20170803/same_statements3b-errors.txt; YY=$(date) && echo "$XX | $YY"
Mi 23. Aug 17:25:54 CEST 2017 | Mi 23. Aug 17:29:15 CEST 2017


How should the source code search be improved further here?

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

>> How should the source code search be improved further here?
> 
> If you like the results from the second case, what more do you want?

* There are also several questionable transformation suggestions generated
  besides the usable ones.

* Will answers belong also to the topic ?Get the non-optional nest construct
  completely working for SmPL??
  https://github.com/coccinelle/coccinelle/issues

Regards,
Markus

[Cocci] Comparing statement lists with SmPL

[SF Markus Elfring]

> If you like the results from the second case, what more do you want?

I would like to achieve somehow that the number of presented ?false positives?
will become so low so that similar (and extended) SmPL scripts can be used for
more automatic source code transformations with higher confidence.

* How are are the chances to integrate corresponding variants into a script
  collection for which you care also?
  https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/scripts/coccinelle/

* How much does the attention for related issues fit to the software development
  capacity which is available so far?

Regards,
Markus