[Cocci] Finding unstored return values with SmPL

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

Hello,

I can find some function calls with the help of the semantic patch
language easily.
But I see further software development challenges in the following use cases.

1. The return value was not used for the initialisation of a variable.

2. The return value was not assigned to another variable.


Do I need to fiddle with SmPL position variables to determine if an assignment
is not performed together with a call within a function implementation?

I would appreciate your advices for such a source code analysis.

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

On Wed, 15 Jul 2015, SF Markus Elfring wrote:

> Hello,
>
> I can find some function calls with the help of the semantic patch
> language easily.
> But I see further software development challenges in the following use cases.
>
> 1. The return value was not used for the initialisation of a variable.
>
> 2. The return value was not assigned to another variable.
>
>
> Do I need to fiddle with SmPL position variables to determine if an assignment
> is not performed together with a call within a function implementation?
>
> I would appreciate your advices for such a source code analysis.

No idea what you are trying to do.  Write something, and then if the
result is not satisfactory, ask afterwards.

julia

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

>> I can find some function calls with the help of the semantic patch
>> language easily.
>> But I see further software development challenges in the following use cases.
>>
>> 1. The return value was not used for the initialisation of a variable.
>>
>> 2. The return value was not assigned to another variable.
>>
>>
>> Do I need to fiddle with SmPL position variables to determine if an assignment
>> is not performed together with a call within a function implementation?
>>
>> I would appreciate your advices for such a source code analysis.
> 
> No idea what you are trying to do.  Write something, and then if the
> result is not satisfactory, ask afterwards.

Now I try again with a different wording.

If I would start with top-down approach, I imagine that I will need a SmPL rule
like the following.


@show_unstored_return_values
depends on !find_calls_for_initialisations
        && !find_calls_for_designated_initialisations
        && !find_calls_for_assignments@
identifier allocation =~ "^(?x)
(?:
   ippNewRequest
|
   sigaction
)$";
position pos;
@@
 void receiver(...)
 {
  <+...
* allocation@pos(...)
  ...+>
 }


But I find that the shown dependencies will be insufficient so far.
I imagine that I should also specify additional constrains for the function
calls I am looking for.
Can extensions be better discussed with such a SmPL stub?

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

> @show_unstored_return_values
> depends on !find_calls_for_initialisations
>         && !find_calls_for_designated_initialisations
>         && !find_calls_for_assignments@
> identifier allocation =~ "^(?x)
> (?:
>    ippNewRequest
> |
>    sigaction
> )$";

Do you really need to use a regular expression here?  Why not use a 
disjunction in the pattern?

What does x represent?

> position pos;
> @@
>  void receiver(...)

What is received?  Why is the void type important?

>  {
>   <+...
> * allocation at pos(...)

How is it useful to match just thf function call.  This doesn't show 
anything about whether or not the return value is stored.

julia

>   ...+>
>  }
> 
> 
> But I find that the shown dependencies will be insufficient so far.
> I imagine that I should also specify additional constrains for the function
> calls I am looking for.
> Can extensions be better discussed with such a SmPL stub?
> 
> Regards,
> Markus
> 

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

>> @show_unstored_return_values
>> depends on !find_calls_for_initialisations
>>         && !find_calls_for_designated_initialisations
>>         && !find_calls_for_assignments@
>> identifier allocation =~ "^(?x)
>> (?:
>>    ippNewRequest
>> |
>>    sigaction
>> )$";
> 
> Do you really need to use a regular expression here?

No, not at the moment with this SmPL stub.


> Why not use a disjunction in the pattern?

The corresponding function name list will grow to some degree in the future.
Our debate around the use of SmPL disjunctions or constraints will continue
for this purpose.


> What does x represent?

Should this parameter turn the free-spacing mode on for the regular expression?
http://www.regular-expressions.info/freespacing.html


>> position pos;
>> @@
>>  void receiver(...)
> 
> What is received?

This specific function name does not really matter at the beginning.


> Why is the void type important?

It is unimportant for a start.

My SmPL approach is evolving so that the following specification might be better.


 return_type work(...)


>>  {
>>   <+...
>> * allocation at pos(...)
> 
> How is it useful to match just thf function call.

Do you care also for unused return values at such source code place occasionally?


> This doesn't show anything about whether or not the return value is stored.

I am trying to express this constraint by a detailed SmPL dependency specification
somehow in the rule declaration.
I am also unsure on the right way to establish a safe relationship between the position
variable "pos" there and function calls at other source code places.

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

Between the regular expression and the undefined dependencies, I have no 
idea what you are trying to do.  Maybe just give an example of code that 
you would like to match.

julia

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

> Between the regular expression and the undefined dependencies,
> I have no idea what you are trying to do.

I would present the "gap" (further SmPL rules) there if the main rule
will be clear for the suggested top-down design approach.


> Maybe just give an example of code that you would like to match.

I would like to achieve a bit of functionality which is already provided
by other popular static source code analysis tools to some degree
also by means of the Coccinelle software.

Other tools can also detect unused/unchecked return values.
Examples:
http://www.splint.org/manual/html/sec8.html#_Toc534974997
https://gcc.gnu.org/onlinedocs/gcc-5.2.0/gcc/Function-Attributes.html#index-g_t_0040code_007bwarn_005funused_005fresult_007d-function-attribute-3299


The SmPL approach that we are discussing now should point open issues out
in source code like the following small test example.

int implementation_with_an_ignored_return_value(void)
{
 malloc(123);
 return 0;
}


I imagine that a complete solution would need a detailed data flow analysis
for this purpose. But I am not familiar enough will all necessary
technology there so far.

So I hope that a smaller solution can be achieved with Coccinelle.
* I can find an ordinary function call without problems.

* The corresponding software development challenges become apparent
  when I would like to ensure by a SmPL filter that the source code position
  of this call is different from all other function calls which were detected
  at other places within a function implementation.

  @show_unstored_return_values
   depends on !find_calls_for_initialisations
           && !find_calls_for_designated_initialisations
           && !find_calls_for_assignments@

  Should this dependency specification be extended anyhow?

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

> I would like to achieve a bit of functionality which is already provided
> by other popular static source code analysis tools

Why not use them then?

> The SmPL approach that we are discussing now should point open issues out
> in source code like the following small test example.
> 
> int implementation_with_an_ignored_return_value(void)
> {
>  malloc(123);
>  return 0;
> }
> 
> 
> I imagine that a complete solution would need a detailed data flow analysis
> for this purpose. But I am not familiar enough will all necessary
> technology there so far.
> 
> So I hope that a smaller solution can be achieved with Coccinelle.
> * I can find an ordinary function call without problems.
> 
> * The corresponding software development challenges become apparent
>   when I would like to ensure by a SmPL filter that the source code position
>   of this call is different from all other function calls which were detected
>   at other places within a function implementation.
> 
>   @show_unstored_return_values
>    depends on !find_calls_for_initialisations
>            && !find_calls_for_designated_initialisations
>            && !find_calls_for_assignments@

I have the impression that you just want

f(...);

once you have determined that f has a return value.  No need for positions
or dependencies on other rules.

On the other hand, this is very liable to false positives when there is
some good reason why the return value of the particular call to f doesn't
matter.

julia

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

>> I would like to achieve a bit of functionality which is already provided
>> by other popular static source code analysis tools
> 
> Why not use them then?

Such tools have got properties or limitations which I do not like.
I would like to reuse the strengths of the Coccinelle software a bit more.
I like its interfaces for the programming languages "OCaml" and "Python"
in principle.


>>   @show_unstored_return_values
>>    depends on !find_calls_for_initialisations
>>            && !find_calls_for_designated_initialisations
>>            && !find_calls_for_assignments@
> 
> I have the impression that you just want
> 
> f(...);

I would like to find a function call which has got specific properties
at such a source code place.
I can see the affected properties as an advanced software developer
almost immediately at the call site while I struggle with a mapping
to the semantic patch language in this use case.


> once you have determined that f has a return value.

I imagine to connect a function name list with this identifier in
a SmPL constraint again.


> No need for positions or dependencies on other rules.

I got the impression for a moment that it is eventually harder to match
something when a detail is absent compared to easily visible source code.
So I tried to use individual SmPL rules as dedicated filters.
Do I fiddle with them in a way which should be better covered
by SmPL disjunctions?

Is it strange anyhow that so many special cases would need to be excluded
so that a specific function call with an unchecked return value is finally
left over for further considerations?


> On the other hand, this is very liable to false positives when there
> is some good reason why the return value of the particular call
> to f doesn't matter.

I can agree to your view to some degree.

I see some opportunities for further fine-tuning of affected source code.
How often would it make sense to mark such special places with a cast
to void?

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

Coccinelle does type inference, including the return types of functions 
when the return types are available.  The simple solution is thus just:

@@
type T;
T x;
identifier f;
void v;
@@

(
f(...)@v;
|
*f(...)@x;
)

This would need to be run with arguments like --recursive-includes, to 
include files included from other include files, --relax-include-path, to 
find uniquely named include files even when they are not in the provided 
include path, and --include-headers-for-types, to improve performance only 
collecting type information from the include files.

julia

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

> Coccinelle does type inference, including the return types of functions

This functionality is nice.


> when the return types are available.

Would you like to give any more hints for this special condition?


> The simple solution is thus just:

Your SmPL script suggestion looks interesting.


> @@
> type T;
> T x;

Would the specification "type x" be also sufficient?


> identifier f;
> void v;
> @@
> 
> (
> f(...)@v;
> |
> *f(...)@x;
> )

Does the first pattern take precedence over the last one
which should show the really interesting case by the asterisk operation?
Do I need to consider search pattern priorities here?

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

> > when the return types are available.
> 
> Would you like to give any more hints for this special condition?

The type is available if the definition or prototype is available.  It 
depends on whether the prototype is in a file that Coccinelle can see when 
it considers a given .c file.

> > @@
> > type T;
> > T x;
> 
> Would the specification "type x" be also sufficient?

Not at all.  That would make x be a type.  You want it to be an 
expression, of some type.

> > identifier f;
> > void v;
> > @@
> > 
> > (
> > f(...)@v;
> > |
> > *f(...)@x;
> > )
> 
> Does the first pattern take precedence over the last one

Yes, always.

julia

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

>>> @@
>>> type T;
>>> T x;
>>
>> Would the specification "type x" be also sufficient?
> 
> Not at all.  That would make x be a type.  You want it to be an 
> expression, of some type.

It seems that I do not understand this distinction at the moment.

I would interpret this part more in the way that a SmPL metavariable
is simply renamed from "T" to "x".


>>> identifier f;
>>> void v;
>>> @@
>>>
>>> (
>>> f(...)@v;
>>> |
>>> *f(...)@x;
>>> )
>>
>> Does the first pattern take precedence over the last one
> 
> Yes, always.

Thanks for your clarification.


I am still unfamiliar with the consequences from the use of 
variables like "v" and "x" when they are not position variables
as you suggest in your example script.

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

On Sun, 19 Jul 2015, SF Markus Elfring wrote:

> >>> @@
> >>> type T;
> >>> T x;
> >>
> >> Would the specification "type x" be also sufficient?
> > 
> > Not at all.  That would make x be a type.  You want it to be an 
> > expression, of some type.
> 
> It seems that I do not understand this distinction at the moment.
> 
> I would interpret this part more in the way that a SmPL metavariable
> is simply renamed from "T" to "x".

typedef int mytype;

mytype x;

Clearly this would not be the same as

typedef int x;

> 
> 
> >>> identifier f;
> >>> void v;
> >>> @@
> >>>
> >>> (
> >>> f(...)@v;
> >>> |
> >>> *f(...)@x;
> >>> )
> >>
> >> Does the first pattern take precedence over the last one
> > 
> > Yes, always.
> 
> Thanks for your clarification.
> 
> 
> I am still unfamiliar with the consequences from the use of 
> variables like "v" and "x" when they are not position variables
> as you suggest in your example script.

They match the closest enclosing expression.

julia

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

> typedef int mytype;
> 
> mytype x;
> 
> Clearly this would not be the same as
> 
> typedef int x;

I see a bit more here.

But my knowledge in the application of the semantic patch language
is still evolving also around such details. The corresponding distinction
is not clear enough for me so far.

Is the handling of "typedefs" another detail for later considerations?



>> I am still unfamiliar with the consequences from the use of 
>> variables like "v" and "x" when they are not position variables
>> as you suggest in your example script.
> 
> They match the closest enclosing expression.

I see that there are also a few other items mentioned in the manual.
Unfortunately, the description is terse.
https://github.com/coccinelle/coccinelle/blob/af8131522ee9aff575e4345f5068f4f138264cd6/docs/manual/cocci_syntax.tex#L258

But I am unsure about when this special SmPL construct will help
in the discussed use case.

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

On Sun, 19 Jul 2015, SF Markus Elfring wrote:

> > typedef int mytype;
> > 
> > mytype x;
> > 
> > Clearly this would not be the same as
> > 
> > typedef int x;
> 
> I see a bit more here.
> 
> But my knowledge in the application of the semantic patch language
> is still evolving also around such details. The corresponding distinction
> is not clear enough for me so far.
> 
> Is the handling of "typedefs" another detail for later considerations?

I just mentioned typedefs to make clear the difference between the name of 
a type and the type of a variable.

julia

> >> I am still unfamiliar with the consequences from the use of 
> >> variables like "v" and "x" when they are not position variables
> >> as you suggest in your example script.
> > 
> > They match the closest enclosing expression.
> 
> I see that there are also a few other items mentioned in the manual.
> Unfortunately, the description is terse.
> https://github.com/coccinelle/coccinelle/blob/af8131522ee9aff575e4345f5068f4f138264cd6/docs/manual/cocci_syntax.tex#L258
> 
> But I am unsure about when this special SmPL construct will help
> in the discussed use case.
> 
> Regards,
> Markus
> 

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

> Do you really need to use a regular expression here?
> Why not use a disjunction in the pattern?

Another source code analysis approach like the following might make
the involved software design decisions a bit more clear.


@show_unstored_return_values@
identifier allocation =~ "^(?x)
(?:
   pthread_mutex_(?:try|un)?lock
|
   sigaction
)$", structure_var, var, work;
field element;
struct structure_type;
type data_type, return_type;
@@
 return_type work(...)
 {
  <+...
(
  data_type var = allocation(...);
|
  structure_type structure_var = { ... .element = allocation(...) ... };
|
(
  var
|
  structure_var
)
  = allocation(...)
|
* (void) allocation(...);
|
* allocation(...)
)
  ...+>
 }


elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_unstored_return_values2.cocci unchecked_return_values1.c
init_defs_builtins: /usr/local/lib/coccinelle/standard.h
181 182
Fatal error: exception Failure("meta: parse error: \n = File \"show_unstored_return_values2.cocci\", line 9, column 21,  charpos = 181\n    around = ';', whole content = struct structure_type;\n")


How can this "surprise" be fixed?



> How is it useful to match just thf function call.

Does the following small source code example contain a few update candidates
for further considerations?

#include <pthread.h>

static pthread_mutex_t my_lock = PTHREAD_MUTEX_INITIALIZER;
static unsigned long my_counter = 1;

void increment_a_shared_variable(void)
{
 pthread_mutex_lock(&my_lock);
 ++my_counter;
 pthread_mutex_unlock(&my_lock);
}



> This doesn't show anything about whether or not the return value is stored.

Does the following SmPL script draft show another interesting application
of the semantic patch language?

@show_unstored_return_values@
identifier allocation =~ "^(?x)
(?:
   pthread_mutex_(?:try|un)?lock
|
   sigaction
)$", var, work;
type data_type, return_type;
@@
 return_type work(...)
 {
  <+...
(
  data_type var = allocation(...);
|
  var = allocation(...)
|
* (void) allocation(...);
|
* allocation(...)
)
  ...+>
 }


elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_unstored_return_values3.cocci unchecked_return_values1.c
init_defs_builtins: /usr/local/lib/coccinelle/standard.h
HANDLING: unchecked_return_values1.c
diff = 
--- unchecked_return_values1.c
+++ /tmp/cocci-output-7564-385170-unchecked_return_values1.c
@@ -5,7 +5,5 @@ static unsigned long my_counter = 1;
 
 void increment_a_shared_variable(void)
 {
- pthread_mutex_lock(&my_lock);
  ++my_counter;
- pthread_mutex_unlock(&my_lock);
 }


How do you think about my imaginations for corresponding software improvements?

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

On Mon, 20 Jul 2015, SF Markus Elfring wrote:

> > Do you really need to use a regular expression here?
> > Why not use a disjunction in the pattern?
>
> Another source code analysis approach like the following might make
> the involved software design decisions a bit more clear.
>
>
> @show_unstored_return_values@
> identifier allocation =~ "^(?x)
> (?:
>    pthread_mutex_(?:try|un)?lock
> |
>    sigaction
> )$", structure_var, var, work;
> field element;
> struct structure_type;

This (at best) makes an expression-typed metavariable that can only match
a structure.

> type data_type, return_type;
> @@
>  return_type work(...)
>  {
>   <+...
> (
>   data_type var = allocation(...);
> |
>   structure_type structure_var = { ... .element = allocation(...) ... };

This uses that metavariable as a type rather than as an expression.

In any case, what you want is

identifier i;

and then struct i structure_var = { ... .element = allocation(...) ... };

> |
> (
>   var
> |
>   structure_var
> )
>   = allocation(...)
> |
> * (void) allocation(...);

I don't see any need for this case.  If someone has put void, it means
that thay have thought about the code, and decided that the return value
is not important.

julia

> |
> * allocation(...)
> )
>   ...+>
>  }
>
>
> elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_unstored_return_values2.cocci unchecked_return_values1.c
> init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> 181 182
> Fatal error: exception Failure("meta: parse error: \n = File \"show_unstored_return_values2.cocci\", line 9, column 21,  charpos = 181\n    around = ';', whole content = struct structure_type;\n")
>
>
> How can this "surprise" be fixed?
>
>
>
> > How is it useful to match just thf function call.
>
> Does the following small source code example contain a few update candidates
> for further considerations?
>
> #include <pthread.h>
>
> static pthread_mutex_t my_lock = PTHREAD_MUTEX_INITIALIZER;
> static unsigned long my_counter = 1;
>
> void increment_a_shared_variable(void)
> {
>  pthread_mutex_lock(&my_lock);
>  ++my_counter;
>  pthread_mutex_unlock(&my_lock);
> }
>
>
>
> > This doesn't show anything about whether or not the return value is stored.
>
> Does the following SmPL script draft show another interesting application
> of the semantic patch language?
>
> @show_unstored_return_values@
> identifier allocation =~ "^(?x)
> (?:
>    pthread_mutex_(?:try|un)?lock
> |
>    sigaction
> )$", var, work;
> type data_type, return_type;
> @@
>  return_type work(...)
>  {
>   <+...
> (
>   data_type var = allocation(...);
> |
>   var = allocation(...)
> |
> * (void) allocation(...);
> |
> * allocation(...)
> )
>   ...+>
>  }
>
>
> elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_unstored_return_values3.cocci unchecked_return_values1.c
> init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> HANDLING: unchecked_return_values1.c
> diff =
> --- unchecked_return_values1.c
> +++ /tmp/cocci-output-7564-385170-unchecked_return_values1.c
> @@ -5,7 +5,5 @@ static unsigned long my_counter = 1;
>
>  void increment_a_shared_variable(void)
>  {
> - pthread_mutex_lock(&my_lock);
>   ++my_counter;
> - pthread_mutex_unlock(&my_lock);
>  }
>
>
> How do you think about my imaginations for corresponding software improvements?
>
> Regards,
> Markus
>

[Cocci] Finding designated initialisers with SmPL

[SF Markus Elfring]

> In any case, what you want is
> 
> identifier i;
> 
> and then struct i structure_var = { ... .element = allocation(...) ... };

Thanks for your hint.


@show_designated_initialisers@
field element;
identifier allocation, structure_type, structure_var;
@@
 <+...
*struct structure_type structure_var = { ... .element = allocation(...) ... };
 ...+>


elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_unstored_return_values2.cocci unchecked_return_values1.c
init_defs_builtins: /usr/local/lib/coccinelle/standard.h
327 328
Fatal error: exception Failure("minus: parse error: \n = File \"show_unstored_return_values2.cocci\", line 17, column 46,  charpos = 327\n    around = '.', whole content =   struct structure_type structure_var = { ... .element = allocation(...) ... };\n")


How should this small SmPL example be improved further?
http://en.cppreference.com/w/c/language/struct_initialization

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

> In any case, what you want is
> 
> identifier i;
> 
> and then struct i structure_var = { ... .element = allocation(...) ... };

Thanks for your hint.


@show_designated_initialisers@
field element;
identifier allocation, structure_type, structure_var;
@@
 <+...
*struct structure_type structure_var = { ... .element = allocation(...) ... };
 ...+>


elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_designated_initialisers1.cocci unchecked_return_values1.c
init_defs_builtins: /usr/local/lib/coccinelle/standard.h
155 156
Fatal error: exception Failure("minus: parse error: \n = File \"show_designated_initialisers1.cocci\", line 6, column 45,  charpos = 155\n    around = '.', whole content = *struct structure_type structure_var = { ... .element = allocation(...) ... };\n")


How should my small SmPL example be improved further?

http://en.cppreference.com/w/c/language/struct_initialization

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

On Mon, 20 Jul 2015, SF Markus Elfring wrote:

> > In any case, what you want is
> >
> > identifier i;
> >
> > and then struct i structure_var = { ... .element = allocation(...) ... };
>
> Thanks for your hint.
>
>
> @show_designated_initialisers@
> field element;
> identifier allocation, structure_type, structure_var;
> @@
>  <+...
> *struct structure_type structure_var = { ... .element = allocation(...) ... };

You need a comma after the ...s representing structure field declarations.

julia

>  ...+>
>
>
> elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_designated_initialisers1.cocci unchecked_return_values1.c
> init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> 155 156
> Fatal error: exception Failure("minus: parse error: \n = File \"show_designated_initialisers1.cocci\", line 6, column 45,  charpos = 155\n    around = '.', whole content = *struct structure_type structure_var = { ... .element = allocation(...) ... };\n")
>
>
> How should my small SmPL example be improved further?
>
> http://en.cppreference.com/w/c/language/struct_initialization
>
> Regards,
> Markus
>

[Cocci] Finding designated initialisers with SmPL

[SF Markus Elfring]

>> *struct structure_type structure_var = { ... .element = allocation(...) ... };
> 
> You need a comma after the ...s representing structure field declarations.

Do I overlook another needed implementation detail here?


@show_designated_initialisers@
field element;
identifier allocation, structure_type, structure_var;
@@
 <+...
*struct structure_type structure_var = { ..., .element = allocation(...), ... };
 ...+>


elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_designated_initialisers1.cocci unchecked_return_values1.c
init_defs_builtins: /usr/local/lib/coccinelle/standard.h
157 164
Fatal error: exception Failure("minus: parse error: \n = File \"show_designated_initialisers1.cocci\", line 6, column 47,  charpos = 157\n    around = 'element', whole content = *struct structure_type structure_var = { ..., .element = allocation(...), ..., };\n")


Regards,
Markus

[Cocci] Finding designated initialisers with SmPL

[Julia Lawall]

On Mon, 20 Jul 2015, SF Markus Elfring wrote:

> >> *struct structure_type structure_var = { ... .element = allocation(...) ... };
> > 
> > You need a comma after the ...s representing structure field declarations.
> 
> Do I overlook another needed implementation detail here?
> 
> 
> @show_designated_initialisers@
> field element;
> identifier allocation, structure_type, structure_var;
> @@
>  <+...
> *struct structure_type structure_var = { ..., .element = allocation(...), ... };
>  ...+>

Element should be an identifier.

julia

> 
> 
> elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_designated_initialisers1.cocci unchecked_return_values1.c
> init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> 157 164
> Fatal error: exception Failure("minus: parse error: \n = File \"show_designated_initialisers1.cocci\", line 6, column 47,  charpos = 157\n    around = 'element', whole content = *struct structure_type structure_var = { ..., .element = allocation(...), ..., };\n")
> 
> 
> Regards,
> Markus
> 
> 

[Cocci] Finding designated initialisers with SmPL

[SF Markus Elfring]

>>  <+...
>> *struct structure_type structure_var = { ..., .element = allocation(...), ... };
>>  ...+>
> 
> Element should be an identifier.

I find it not so obvious that the metavariable type "field"
was inappropriate there. Would more explanations in the manual help, too?
https://github.com/coccinelle/coccinelle/blob/af8131522ee9aff575e4345f5068f4f138264cd6/docs/manual/cocci_syntax.tex#L153


I have tried the following small SmPL script

@show_designated_initialisers@
identifier allocation, element, structure_type, structure_var;
@@
 <+...
*struct structure_type structure_var = { ..., .element = allocation(...), ... };
 ...+>


on this


char* get_default_message(short selection)
{
 static char xyz[123];
 
 switch (selection)
 {
 case 1:
  strcpy(xyz, "Test");
 default:
  strcpy(xyz, "working");
 }
 return xyz;
}

static unsigned long my_counter = 0;

static struct my_string
{
 unsigned int length;
 char* text;
} message = { .text = get_default_message(1), .length = 123 };


source code example.


elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_designated_initialisers1.cocci designated_initialiser1.c
init_defs_builtins: /usr/local/lib/coccinelle/standard.h
HANDLING: designated_initialiser1.c


Unfortunately, I do not see a generated patch there.
How should I improve my approach for the expected analysis result?

Regards,
Markus

[Cocci] Finding designated initialisers with SmPL

[SF Markus Elfring]

> I have tried the following small SmPL script
> 
> @show_designated_initialisers@
> identifier allocation, element, structure_type, structure_var;
> @@
>  <+...
> *struct structure_type structure_var = { ..., .element = allocation(...), ... };
>  ...+>
> 
> 
> on this
> 
> 
> char* get_default_message(short selection)
> {
>  static char xyz[123];
>  
>  switch (selection)
>  {
>  case 1:
>   strcpy(xyz, "Test");
>  default:
>   strcpy(xyz, "working");
>  }
>  return xyz;
> }
> 
> static unsigned long my_counter = 0;
> 
> static struct my_string
> {
>  unsigned int length;
>  char* text;
> } message = { .text = get_default_message(1), .length = 123 };
> 
> 
> source code example.
> 
> 
> elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_designated_initialisers1.cocci designated_initialiser1.c
> init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> HANDLING: designated_initialiser1.c
> 
> 
> Unfortunately, I do not see a generated patch there.
> How should I improve my approach for the expected analysis result?

How is the status for the support of data processing for designated initialisers
with the semantic patch language?

Regards,
Markus

[Cocci] Finding designated initialisers with SmPL

[Julia Lawall]

On Sat, 8 Aug 2015, SF Markus Elfring wrote:

> > I have tried the following small SmPL script
> > 
> > @show_designated_initialisers@
> > identifier allocation, element, structure_type, structure_var;
> > @@
> >  <+...
> > *struct structure_type structure_var = { ..., .element = allocation(...), ... };
> >  ...+>

Why do you have <+... ...+> around the pattern?

julia

> > 
> > 
> > on this
> > 
> > 
> > char* get_default_message(short selection)
> > {
> >  static char xyz[123];
> >  
> >  switch (selection)
> >  {
> >  case 1:
> >   strcpy(xyz, "Test");
> >  default:
> >   strcpy(xyz, "working");
> >  }
> >  return xyz;
> > }
> > 
> > static unsigned long my_counter = 0;
> > 
> > static struct my_string
> > {
> >  unsigned int length;
> >  char* text;
> > } message = { .text = get_default_message(1), .length = 123 };
> > 
> > 
> > source code example.
> > 
> > 
> > elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_designated_initialisers1.cocci designated_initialiser1.c
> > init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> > HANDLING: designated_initialiser1.c
> > 
> > 
> > Unfortunately, I do not see a generated patch there.
> > How should I improve my approach for the expected analysis result?
> 
> How is the status for the support of data processing for designated initialisers
> with the semantic patch language?
> 
> Regards,
> Markus
> _______________________________________________
> Cocci mailing list
> Cocci at systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
> 

[Cocci] Finding designated initialisers with SmPL

[SF Markus Elfring]

>>> @show_designated_initialisers@
>>> identifier allocation, element, structure_type, structure_var;
>>> @@
>>>  <+...
>>> *struct structure_type structure_var = { ..., .element = allocation(...), ... };
>>>  ...+>
> 
> Why do you have <+... ...+> around the pattern?

I tried to express that the special case "nothing found" should be excluded here.

I do also not get the expected analysis result if I omit the SmPL ellipsis
for such an use case. How should I improve this approach then?

Regards,
Markus

[Cocci] Finding designated initialisers with SmPL

[Julia Lawall]

On Sat, 8 Aug 2015, SF Markus Elfring wrote:

> > I have tried the following small SmPL script
> > 
> > @show_designated_initialisers@
> > identifier allocation, element, structure_type, structure_var;
> > @@
> >  <+...
> > *struct structure_type structure_var = { ..., .element = allocation(...), ... };
> >  ...+>
> > 
> > 
> > on this
> > 
> > 
> > char* get_default_message(short selection)
> > {
> >  static char xyz[123];
> >  
> >  switch (selection)
> >  {
> >  case 1:
> >   strcpy(xyz, "Test");
> >  default:
> >   strcpy(xyz, "working");
> >  }
> >  return xyz;
> > }
> > 
> > static unsigned long my_counter = 0;
> > 
> > static struct my_string
> > {
> >  unsigned int length;
> >  char* text;
> > } message = { .text = get_default_message(1), .length = 123 };

Actually, I don't see where you expect your rule to match your code.  I 
don't see the point of including the function withe the switch and the 
declaration of my-counter in the example at all.  The only case where 
there is any hope of a match is in the declaration of the my_string 
strcuture, but that declaration doesn't have the same form as your 
semantic patch rule.  Your code specifies the types of the fields at the 
same time, which is not done in the pattern.

julia

> > 
> > 
> > source code example.
> > 
> > 
> > elfring at Sonne:~/Projekte/Coccinelle/janitor> spatch.opt -sp-file show_designated_initialisers1.cocci designated_initialiser1.c
> > init_defs_builtins: /usr/local/lib/coccinelle/standard.h
> > HANDLING: designated_initialiser1.c
> > 
> > 
> > Unfortunately, I do not see a generated patch there.
> > How should I improve my approach for the expected analysis result?
> 
> How is the status for the support of data processing for designated initialisers
> with the semantic patch language?
> 
> Regards,
> Markus
> _______________________________________________
> Cocci mailing list
> Cocci at systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
> 

[Cocci] Finding designated initialisers with SmPL

[SF Markus Elfring]

>>> static struct my_string
>>> {
>>>  unsigned int length;
>>>  char* text;
>>> } message = { .text = get_default_message(1), .length = 123 };
> 
> Actually, I don't see where you expect your rule to match your code.

The following small SmPL script seems to work.

@show_designated_initialisers@
identifier allocation, element, structure_type, structure_var;
@@
*struct structure_type { ... } structure_var = { ..., .element = allocation(...), ... };


Do I stumble on another open issue if I would dare to reuse the SmPL
construct "<+... ...+>" for such an use case again?

Regards,
Markus

[Cocci] Finding designated initialisers with SmPL

[Julia Lawall]

> Do I stumble on another open issue if I would dare to reuse the SmPL
> construct "<+... ...+>" for such an use case again?

If there were an open issue that I knew of, I would have probably fixed it.

<+... ...+> is useful when you to match a larger top-level piece of code, and
want to see if some subterm appears in it, one or more times.  But if your
pattern matches the whole top-level piece of code itself, then there is no
point to use <+... ...+>.  Either your pattern is there or it is not.  If
itis there, it will be matched with just the pattern itself.  There is no
need for <+... ...+>, because you are not looking for a subterm of
anything.

julia

[Cocci] Finding designated initialisers with SmPL

[SF Markus Elfring]

>> Do I stumble on another open issue if I would dare to reuse the SmPL
>> construct "<+... ...+>" for such an use case again?
> 
> If there were an open issue that I knew of, I would have probably fixed it.

It seems that I observed a few software limitations again a moment ago.


> <+... ...+> is useful when you to match a larger top-level piece of code, and
> want to see if some subterm appears in it, one or more times.

Thanks for your promising feedback.

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

> Does the following SmPL script draft show another interesting application
> of the semantic patch language?

Dear Julia,

I have adjusted this approach a bit for the programming interface "udev".
https://github.com/systemd/systemd/blob/bbf35206735f97cf3fcda8d26982b35b0cad20a9/src/udev/udev.h#L68

It seems that the Coccinelle software needs remarkable execution times
for the analysis of specific source files from this library.
I am unsure again which timeout value will be appropriate here.

Do I become too impatient with my computer?

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

On Wed, 22 Jul 2015, SF Markus Elfring wrote:

> > Does the following SmPL script draft show another interesting application
> > of the semantic patch language?
>
> Dear Julia,
>
> I have adjusted this approach a bit for the programming interface "udev".
> https://github.com/systemd/systemd/blob/bbf35206735f97cf3fcda8d26982b35b0cad20a9/src/udev/udev.h#L68
>
> It seems that the Coccinelle software needs remarkable execution times
> for the analysis of specific source files from this library.
> I am unsure again which timeout value will be appropriate here.
>
> Do I become too impatient with my computer?

You can use arguments like --show-trying and --debug to see where it is
getting stuck, if that is the case.

julia

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

> You can use arguments like --show-trying and --debug to see
> where it is getting stuck, if that is the case.

How do you think about to try another analysis out with one
of your test systems?

"source file"|duration|comment|function
udev-rules.c|234.2|EXN:Common.Timeout|add_rule
udev-builtin-usb_id.c|234.4|EXN:Common.Timeout|builtin_usb_id
udev-builtin-net_id.c|13.0||
udev-builtin-input_id.c|45.1||
udevadm-monitor.c|168.5||
udevadm-control.c|240.0|EXN:Common.Timeout|adm_control
collect/collect.c|27.1||
cdrom_id/cdrom_id.c|234.4|EXN:Common.Timeout|main
ata_id/ata_id.c|252.5|EXN:Common.Timeout|main


I extended the corresponding SmPL script also with the
following specifications.

@show_unstored_return_values@
binary operator bo;
expression express;
?
statement es, is;
?
|
  if (allocation(...) bo express) is else es;
|
?


But I got the impression that this pattern extension is not matched
in my use case.
How should I improve my approach here?

Regards,
Markus

[Cocci] Finding unstored return values with SmPL

[Julia Lawall]

On Thu, 23 Jul 2015, SF Markus Elfring wrote:

> > You can use arguments like --show-trying and --debug to see
> > where it is getting stuck, if that is the case.
> 
> How do you think about to try another analysis out with one
> of your test systems?

I don't have your semantic patch.  If you want me to to try something, you 
have to send the semantic patch, even if you have sent it before.

But have you looked at the result of --show-trying and looked at the last 
function that it prints to see why that function might be time consuming?

> "source file"|duration|comment|function
> udev-rules.c|234.2|EXN:Common.Timeout|add_rule
> udev-builtin-usb_id.c|234.4|EXN:Common.Timeout|builtin_usb_id
> udev-builtin-net_id.c|13.0||
> udev-builtin-input_id.c|45.1||
> udevadm-monitor.c|168.5||
> udevadm-control.c|240.0|EXN:Common.Timeout|adm_control
> collect/collect.c|27.1||
> cdrom_id/cdrom_id.c|234.4|EXN:Common.Timeout|main
> ata_id/ata_id.c|252.5|EXN:Common.Timeout|main
> 
> 
> I extended the corresponding SmPL script also with the
> following specifications.
> 
> @show_unstored_return_values@
> binary operator bo;
> expression express;
> ?
> statement es, is;
> ?
> |
>   if (allocation(...) bo express) is else es;

There should not be a semicolon after es.  Since es is a statement, the 
semicolon represents an empty statement, which your code is unlikely to 
contain.

julia

[Cocci] Finding unstored return values with SmPL

[SF Markus Elfring]

>> How do you think about to try another analysis out with one
>> of your test systems?
> 
> I don't have your semantic patch.  If you want me to to try something,
> you have to send the semantic patch, even if you have sent it before.

I sent you two SmPL script examples for this purpose in the meantime.
Would you like to share any more experiences from a corresponding
software run time analysis?

Regards,
Markus

[Cocci] Fine-tuning for the processing of function name lists?

[SF Markus Elfring]

> It should be much better for the understandability of your code,
> for the maintainability of your code, and for the speed of processing
> your code to say
> 
> identifier allocation = {function1, function2, function3, function4, ...};

Function names from various application programming interfaces follow
a few naming patterns.
Can SmPL scripts benefit from the fact that such identifiers contain
a couple of identical name components?

Regards,
Markus