[Cocci] gboolean -> bool conversion

[Cocci] gboolean -> bool conversion

[Daniel Wagner]

Hi,

I'd like to convert a bunch of gboolean decleration to stdbool in our 
ConnMan code base. I am a noob with coccinelle. I have partial success 
so far but I do not thing I am doing the right thing. So any advice is 
welcome :)


This here is my current coccinelle script for converting gboolean used 
as stack variable. I have also one for structs, which gave my good 
results. Another one is needed then for the function arguments. But 
let's first have a look on this part.

@r1@
position p;
typedef gboolean;
identifier func,x;
@@
func(...) {
<...
	gboolean@p x;
...>
}

@r2@
position r1.p;
typedef bool;
@@
- gboolean at p
+ bool

@r3@
identifier r1.x;
@@
(
- x = FALSE
+ x = false
|
- x = TRUE
+ x = true
)

int main(int argc, char *argv[])
{
	gboolean b = TRUE;

	return 0;
}

results in

int main(int argc, char *argv[])
{
-       gboolean b = TRUE;
+       bool b = true;

         return 0;
}

which is what I wanted. Now the problem is that I really have no clue to 
mach on things like

int main(int argc, char *argv[])
{
	gboolean b = TRUE, c = FALSE;

	return 0;
}

Any idea?

cheers,
daniel

[Cocci] gboolean -> bool conversion

[Julia Lawall]

On Thu, 11 Jul 2013, Daniel Wagner wrote:

> Hi,
>
> I'd like to convert a bunch of gboolean decleration to stdbool in our ConnMan
> code base. I am a noob with coccinelle. I have partial success so far but I do
> not thing I am doing the right thing. So any advice is welcome :)
>
>
> This here is my current coccinelle script for converting gboolean used as
> stack variable. I have also one for structs, which gave my good results.
> Another one is needed then for the function arguments. But let's first have a
> look on this part.
>
> @r1@
> position p;
> typedef gboolean;
> identifier func,x;
> @@
> func(...) {
> <...
> 	gboolean at p x;
> ...>
> }
>
> @r2@
> position r1.p;
> typedef bool;
> @@
> - gboolean at p
> + bool
>
> @r3@
> identifier r1.x;
> @@
> (
> - x = FALSE
> + x = false
> |
> - x = TRUE
> + x = true
> )
>
> int main(int argc, char *argv[])
> {
> 	gboolean b = TRUE;
>
> 	return 0;
> }
>
> results in
>
> int main(int argc, char *argv[])
> {
> -       gboolean b = TRUE;
> +       bool b = true;
>
>         return 0;
> }
>
> which is what I wanted. Now the problem is that I really have no clue to mach
> on things like
>
> int main(int argc, char *argv[])
> {
> 	gboolean b = TRUE, c = FALSE;
>
> 	return 0;
> }
>
> Any idea?

Did you try just

@@
@@

- gboolean
+ bool

Also, for yoru TRUE -> true rule, you could consider

@@
gboolean x;
@@

x =
- TRUE
+ true

That is, you don't have to find the type declaration and then the
identifier.  You can describe the type of an arbitrary expression x.

julia

[Cocci] gboolean -> bool conversion

[Daniel Wagner]

Hi Julia,

On 07/11/2013 10:29 AM, Julia Lawall wrote:
>> I'd like to convert a bunch of gboolean decleration to stdbool in our ConnMan
>> code base. I am a noob with coccinelle. I have partial success so far but I do
>> not thing I am doing the right thing. So any advice is welcome :)
>>
>>
>> This here is my current coccinelle script for converting gboolean used as
>> stack variable. I have also one for structs, which gave my good results.
>> Another one is needed then for the function arguments. But let's first have a
>> look on this part.
>>
>> @r1@
>> position p;
>> typedef gboolean;
>> identifier func,x;
>> @@
>> func(...) {
>> <...
>> 	gboolean at p x;
>> ...>
>> }
>>
>> @r2@
>> position r1.p;
>> typedef bool;
>> @@
>> - gboolean at p
>> + bool
>>
>> @r3@
>> identifier r1.x;
>> @@
>> (
>> - x = FALSE
>> + x = false
>> |
>> - x = TRUE
>> + x = true
>> )
>>
>> int main(int argc, char *argv[])
>> {
>> 	gboolean b = TRUE;
>>
>> 	return 0;
>> }
>>
>> results in
>>
>> int main(int argc, char *argv[])
>> {
>> -       gboolean b = TRUE;
>> +       bool b = true;
>>
>>          return 0;
>> }
>>
>> which is what I wanted. Now the problem is that I really have no clue to mach
>> on things like
>>
>> int main(int argc, char *argv[])
>> {
>> 	gboolean b = TRUE, c = FALSE;
>>
>> 	return 0;
>> }
>>
>> Any idea?
>
> Did you try just
>
> @@
> @@
>
> - gboolean
> + bool
>
> Also, for yoru TRUE -> true rule, you could consider

Yes, the problem is that I can't replace all gbooleans because we are 
GLib, e.g. we are using quite a few timers


guint g_timeout_add (guint interval, GSourceFunc function,
			 gpointer data);

gboolean (*GSourceFunc) (gpointer user_data);

https://developer.gnome.org/glib/2.36/glib-The-Main-Event-Loop.html#g-timeout-add

So these gboolean should not be changed. BTW, when I tried your probosal 
it would not change this here

	gboolean b,c;

changing the rules to

	- gboolean
	++ bool

resulted int

	bool bool b,c;

Also not what I wanted :)

Anyway, the main problem I think is that I do not want to change all 
gboolean, only the ones in structs and the ones declared on the stack. 
Maybe a few in fucntion signatures but that I can do by hand. I just 
dont want to change 1000 lines by hand, that is far to dangerous :)

> @@
> gboolean x;
> @@
>
> x =
> - TRUE
> + true
>
> That is, you don't have to find the type declaration and then the
> identifier.  You can describe the type of an arbitrary expression x.

Ah okay. I wanted to make sure I only change those assignments which did 
change.

cheers,
daniel

[Cocci] gboolean -> bool conversion

[Julia Lawall]

On Thu, 11 Jul 2013, Daniel Wagner wrote:

> Hi Julia,
>
> On 07/11/2013 10:29 AM, Julia Lawall wrote:
> > > I'd like to convert a bunch of gboolean decleration to stdbool in our
> > > ConnMan
> > > code base. I am a noob with coccinelle. I have partial success so far but
> > > I do
> > > not thing I am doing the right thing. So any advice is welcome :)
> > >
> > >
> > > This here is my current coccinelle script for converting gboolean used as
> > > stack variable. I have also one for structs, which gave my good results.
> > > Another one is needed then for the function arguments. But let's first
> > > have a
> > > look on this part.
> > >
> > > @r1@
> > > position p;
> > > typedef gboolean;
> > > identifier func,x;
> > > @@
> > > func(...) {
> > > <...
> > > 	gboolean at p x;
> > > ...>
> > > }
> > >
> > > @r2@
> > > position r1.p;
> > > typedef bool;
> > > @@
> > > - gboolean at p
> > > + bool
> > >
> > > @r3@
> > > identifier r1.x;
> > > @@
> > > (
> > > - x = FALSE
> > > + x = false
> > > |
> > > - x = TRUE
> > > + x = true
> > > )
> > >
> > > int main(int argc, char *argv[])
> > > {
> > > 	gboolean b = TRUE;
> > >
> > > 	return 0;
> > > }
> > >
> > > results in
> > >
> > > int main(int argc, char *argv[])
> > > {
> > > -       gboolean b = TRUE;
> > > +       bool b = true;
> > >
> > >          return 0;
> > > }
> > >
> > > which is what I wanted. Now the problem is that I really have no clue to
> > > mach
> > > on things like
> > >
> > > int main(int argc, char *argv[])
> > > {
> > > 	gboolean b = TRUE, c = FALSE;
> > >
> > > 	return 0;
> > > }
> > >
> > > Any idea?
> >
> > Did you try just
> >
> > @@
> > @@
> >
> > - gboolean
> > + bool
> >
> > Also, for yoru TRUE -> true rule, you could consider
>
> Yes, the problem is that I can't replace all gbooleans because we are GLib,
> e.g. we are using quite a few timers
>
>
> guint g_timeout_add (guint interval, GSourceFunc function,
> 			 gpointer data);
>
> gboolean (*GSourceFunc) (gpointer user_data);
>
> https://developer.gnome.org/glib/2.36/glib-The-Main-Event-Loop.html#g-timeout-add
>
> So these gboolean should not be changed. BTW, when I tried your probosal it
> would not change this here
>
> 	gboolean b,c;
>
> changing the rules to
>
> 	- gboolean
> 	++ bool
>
> resulted int
>
> 	bool bool b,c;
>
> Also not what I wanted :)

OK, I will get back to you.

julia

[Cocci] gboolean -> bool conversion

[Daniel Wagner]

Just for the record, I played a bit more around.

@@
@@

- gboolean
+ bool

@@
gboolean x;
@@

x =
(
- TRUE
+ true
|
- FALSE
+ false
)

applied on

static gboolean is_enabled(gboolean var)
{
	return var;
}

int main(int argc, char *argv[])
{
	gboolean b = TRUE;
	gboolean c, d;
	gboolean e = FALSE, g, f = TRUE;
	gboolean h = is_ensbled(b);

	return 0;
}

results in

+++ /tmp/cocci-output-20339-560145-test.c
@@ -5,9 +5,9 @@ static gboolean is_enabled(gboolean var)

  int main(int argc, char *argv[])
  {
-       gboolean b = TRUE;
+       gboolean b = true;
         gboolean c, d;
-       gboolean e = FALSE, g, f = TRUE;
+       gboolean e = false, g, f = true;
         gboolean h = is_ensbled(b);

         return 0;

[Cocci] gboolean -> bool conversion

[Julia Lawall]

How is the following:

@@
gboolean x;
@@

x =
(
- TRUE
+ true
|
- FALSE
+ false
)

@@
typedef bool;
@@

- gboolean
+ bool

Note that the order of the rules is important.  In your rules, you
converted gboolean to bool first, so the second rule would not find any
gbooleans.

Another problem with your rules was that it thought that gboolean and bool
were expressions.  So the first rule needed typedef declaration. In my
rule, I use gboolean as the type of a metavariable in the first rule, so
Coccinelle considers it to be a type thereafter.  But I need the typedef
on bool in the second rule.

Is that good enough for your purpose, or are too many values and
declarations being converted?

julia

[Cocci] gboolean -> bool conversion

[Daniel Wagner]

On 07/11/2013 12:24 PM, Julia Lawall wrote:
> How is the following:
>
> @@
> gboolean x;
> @@
>
> x =
> (
> - TRUE
> + true
> |
> - FALSE
> + false
> )
>
> @@
> typedef bool;
> @@
>
> - gboolean
> + bool
>
> Note that the order of the rules is important.

Yeah, I should have noted that.

> In your rules, you
> converted gboolean to bool first, so the second rule would not find any
> gbooleans.

Yep, that works better, though now I hit the 'already tagged' problem.
Changing the second rule to

-gboolean
++ bool

Let's coccinelle run but the result is not perfect.

> Another problem with your rules was that it thought that gboolean and bool
> were expressions.  So the first rule needed typedef declaration. In my
> rule, I use gboolean as the type of a metavariable in the first rule, so
> Coccinelle considers it to be a type thereafter.  But I need the typedef
> on bool in the second rule.

Thanks for the explanation. It really helps!

> Is that good enough for your purpose, or are too many values and
> declarations being converted?

I have updated my sample program, which contains all our usage pattern 
of gboolean.

struct info {
	gboolean data;
};

static gboolean timer_cb(gpointer user_data)
{
	struct info *info = user_data;

	info->data = FALSE;

	return FALSE;
}

static gboolean is_enabled(gboolean flag)
{
	return flag != TRUE;
}

int main(int argc, char *argv[])
{
	gboolean b = TRUE;
	gboolean c, d;
	gboolean e = FALSE, g, f = TRUE;
	gboolean h = is_ensbled(b);
	struct info *info;

	info = g_new0(struct info, 1);
	info->data = TRUE;

	g_timeout(delay, timer_cb, &info);

	return 0;
}


The resulting patch (with the ++bool version) is:

--- test.c
+++ /tmp/cocci-output-21088-1cd7bc-test.c
@@ -1,31 +1,31 @@
  struct info {
-       gboolean data;
+       bool data;
  };

-static gboolean timer_cb(gpointer user_data)
+static bool timer_cb(gpointer user_data)
  {
         struct info *info = user_data;

-       info->data = FALSE;
+       info->data = false;

         return FALSE;
  }

-static gboolean is_enabled(gboolean flag)
+static bool is_enabled(bool flag)
  {
         return !flag;
  }

  int main(int argc, char *argv[])
  {
-       gboolean b = TRUE;
-       gboolean c, d;
-       gboolean e = FALSE, g, f = TRUE;
-       gboolean h = is_ensbled(b);
+       bool b = true;
+       bool bool c, d;
+       bool bool bool e = false, g, f = true;
+       bool h = is_ensbled(b);
         struct info *info;

         info = g_new0(struct info, 1);
-       info->data = TRUE;
+       info->data = true;

         g_timeout(delay, timer_cb, &info);


Obviously, the 'bool bool bool' stuff is not correct, but I can catch 
that via the compiler.

timer_cb() should not be changed. Is there maybe a way of blacklisting 
certain matches? I see there are some 'when' examples but I do not 
really growk them right now. Sorry for beeing so noobish...

cheers,
daniel

[Cocci] gboolean -> bool conversion

[Julia Lawall]

Maybe the patch below will get rid of the already tagged token problem and
allow you to get rid of the ++..

julia

diff --git a/engine/transformation_c.ml b/engine/transformation_c.ml
index 3368ca7..348ec17 100644
--- a/engine/transformation_c.ml
+++ b/engine/transformation_c.ml
@@ -340,6 +340,28 @@ module XTRANS = struct
 	    Some(Ast_cocci.CONTEXT(old_pos,old_modif),
 		 (clean_env tin.binding)::oldenvs)

+    (* non ++ but same modif for two reasons *)
+    | (Ast_cocci.MINUS(old_pos,old_inst,old_adj,old_modif),
+       Ast_cocci.MINUS(new_pos,new_inst,new_adj,new_modif))
+	when old_pos = new_pos &&
+	  old_modif = strip_minus_code new_modif &&
+	  List.mem (clean_env tin.binding) oldenvs ->
+
+          cocciinforef :=
+	    Some(Ast_cocci.MINUS(old_pos,Common.union_set old_inst new_inst,
+				 old_adj,old_modif),
+		 oldenvs)
+
+    | (Ast_cocci.CONTEXT(old_pos,old_modif),
+       Ast_cocci.CONTEXT(new_pos,new_modif))
+	when old_pos = new_pos &&
+	  old_modif = strip_context_code new_modif &&
+	  List.mem (clean_env tin.binding) oldenvs ->
+	    (* iteration only allowed on context; no way to replace something
+	       more than once; now no need for iterable; just check a flag *)
+
+          cocciinforef := Some(Ast_cocci.CONTEXT(old_pos,old_modif),oldenvs)
+
     | _ ->
           (* coccionly:
           if !Flag.sgrep_mode2

[Cocci] gboolean -> bool conversion

[Daniel Wagner]

Hi Julia,

On 07/11/2013 02:17 PM, Julia Lawall wrote:
> Maybe the patch below will get rid of the already tagged token problem and
> allow you to get rid of the ++..

Thanks for the quick fix. Indeed, this fixes the 'already tagged' 
problem. The result is now:

  struct info {
-       gboolean data;
+       bool data;
  };

-static gboolean timer_cb(gpointer user_data)
+static bool timer_cb(gpointer user_data)
  {
         struct info *info = user_data;

-       info->data = FALSE;
+       info->data = false;

         return FALSE;
  }

-static gboolean is_enabled(gboolean flag)
+static bool is_enabled(bool flag)
  {
         return !flag;
  }

  int main(int argc, char *argv[])
  {
-       gboolean b = TRUE;
-       gboolean c, d;
-       gboolean e = FALSE, g, f = TRUE;
-       gboolean h = is_ensbled(b);
+       bool b = true;
+       bool c, d;
+       bool e = false, g, f = true;
+       bool h = is_ensbled(b);
         struct info *info;

         info = g_new0(struct info, 1);
-       info->data = TRUE;
+       info->data = true;

         g_timeout(delay, timer_cb, &info);


I am pondering if it would be possible to say

- gboolean
	when != 'static gboolean func(gpointer)'
+ bool


cheers,
daniel

ps: I the install step didn't work for me:

$ make install
cp: cannot create regular file ?ocaml/coccilib/coccilib.cmi?: No such 
file or directory
make: *** [ocaml/coccilib/coccilib.cmi] Error 1

Doing a mkdir before the install step fixed it for me.

$ mkdir ocaml/coccili
$ make install

[Cocci] gboolean -> bool conversion

[Julia Lawall]

> I am pondering if it would be possible to say
>
> - gboolean
> 	when != 'static gboolean func(gpointer)'
> + bool

No, this is not possible.  Is the idea that you don't want to change the
return type of a static function?  Then you can record the position of the
gbooleans that you don't want to change, and only change the others.

An example is as follows:

@keep@
identifier f;
position p;
typedef	gpointer;
identifier i;
@@

static gboolean at p f(gpointer i)	{ ... }

@@
typedef bool;
position p != keep.p;
@@

- gboolean at p
+ bool

Note that you could never refer to

static gboolean func(gpointer)

because it is not a complete syntactic unit.  If you wanted to mention a
prototype, you would need a ; after it.  If you wanted to mention the
definition, then you need to put the complete definition, as shown above.

julia

[Cocci] gboolean -> bool conversion

[Daniel Wagner]

Hi Julia,

On 07/11/2013 03:07 PM, Julia Lawall wrote:
>> I am pondering if it would be possible to say
>>
>> - gboolean
>> 	when != 'static gboolean func(gpointer)'
>> + bool
>
> No, this is not possible.  Is the idea that you don't want to change the
> return type of a static function?  Then you can record the position of the
> gbooleans that you don't want to change, and only change the others.

It is so obvious when you explain it. Thanks a lot.

> An example is as follows:
>
> @keep@
> identifier f;
> position p;
> typedef	gpointer;
> identifier i;
> @@
>
> static gboolean at p f(gpointer i)	{ ... }
>
> @@
> typedef bool;
> position p != keep.p;
> @@
>
> - gboolean at p
> + bool
>

With some small modification this works fine now.

The only outstanding problem I'd like to address is patching the 
function calls.

Let's say I have this deceleration (which will be changed by the above
rules):

	int foo(gboolean b);

and some place I call then

	foo(FALSE);

This should be changed to

	foo(false);

> Note that you could never refer to
>
> static gboolean func(gpointer)
>
> because it is not a complete syntactic unit.  If you wanted to mention a
> prototype, you would need a ; after it.  If you wanted to mention the
> definition, then you need to put the complete definition, as shown above.

I see. Thanks again for explaining.

thanks
daniel

[Cocci] gboolean -> bool conversion

[Julia Lawall]

On Fri, 12 Jul 2013, Daniel Wagner wrote:

> Hi Julia,
> 
> On 07/11/2013 03:07 PM, Julia Lawall wrote:
> > > I am pondering if it would be possible to say
> > > 
> > > - gboolean
> > > 	when != 'static gboolean func(gpointer)'
> > > + bool
> > 
> > No, this is not possible.  Is the idea that you don't want to change the
> > return type of a static function?  Then you can record the position of the
> > gbooleans that you don't want to change, and only change the others.
> 
> It is so obvious when you explain it. Thanks a lot.
> 
> > An example is as follows:
> > 
> > @keep@
> > identifier f;
> > position p;
> > typedef	gpointer;
> > identifier i;
> > @@
> > 
> > static gboolean at p f(gpointer i)	{ ... }
> > 
> > @@
> > typedef bool;
> > position p != keep.p;
> > @@
> > 
> > - gboolean at p
> > + bool
> > 
> 
> With some small modification this works fine now.
> 
> The only outstanding problem I'd like to address is patching the function
> calls.
> 
> Let's say I have this deceleration (which will be changed by the above
> rules):
> 
> 	int foo(gboolean b);
> 
> and some place I call then
> 
> 	foo(FALSE);
> 
> This should be changed to
> 
> 	foo(false);
> 
> > Note that you could never refer to
> > 
> > static gboolean func(gpointer)
> > 
> > because it is not a complete syntactic unit.  If you wanted to mention a
> > prototype, you would need a ; after it.  If you wanted to mention the
> > definition, then you need to put the complete definition, as shown above.
> 
> I see. Thanks again for explaining.


@r@
identifier f;
parameter list[n] ps;
identifier i;
@@

f(ps, gboolean i, ...) { ... }

@@
identifier r.f;
expression list [r.n] es;
@@

f(es,
(
- FALSE
+ false
|
- TRUE
+ true
)
 ,...)


I have not tested this, but it should work.  Write back if you have 
problems.

julia

[Cocci] gboolean -> bool conversion

[Daniel Wagner]

On 12.07.2013 09:00, Julia Lawall wrote:
> I have not tested this, but it should work.  Write back if you have
> problems.

Sure, will do. Though today I don't find time to test.

thanks,
daniel

[Cocci] gboolean -> bool conversion

[Daniel Wagner]

Hi Julia,

On 07/12/2013 09:00 AM, Julia Lawall wrote:
> @r@
> identifier f;
> parameter list[n] ps;
> identifier i;
> @@
>
> f(ps, gboolean i, ...) { ... }
>
> @@
> identifier r.f;
> expression list [r.n] es;
> @@
>
> f(es,
> (
> - FALSE
> + false
> |
> - TRUE
> + true
> )
>   ,...)
>
>
> I have not tested this, but it should work.  Write back if you have
> problems.

This rule is a half success. The main problem stems from the fact, that 
a function is declared in a header, e.g.

	void connman_foo(gboolean foo);

and the caller just sees that and not the definition. If I understood
this correctly the 'f(ps, gboolean i, ...) { ... }' will not match.

Another boolean transformation I do is with connman_bool_t to bool. 
Coccinelle is kind of unhappy with this function:

https://git.kernel.org/cgit/network/connman/connman.git/tree/vpn/vpn-agent.c#n38

parse error
  = error in vpn/vpn-agent.c; set verbose_parsing for more info
badcount: 20
bad: #include "vpn.h"
bad:
bad: connman_bool_t vpn_agent_check_reply_has_dict(DBusMessage *reply)
bad: {
bad:    const char *signature = DBUS_TYPE_ARRAY_AS_STRING
BAD:!!!!!               DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
bad:            DBUS_TYPE_STRING_AS_STRING
bad:            DBUS_TYPE_VARIANT_AS_STRING
bad:            DBUS_DICT_ENTRY_END_CHAR_AS_STRING;
bad:
bad:    if (dbus_message_has_signature(reply, signature) == TRUE)
bad:            return TRUE;
bad:
bad:    connman_warn("Reply %s to %s from %s has wrong signature %s",
bad:                    signature,
bad:                    dbus_message_get_interface(reply),
bad:                    dbus_message_get_sender(reply),
bad:                    dbus_message_get_signature(reply));
bad:
bad:    return FALSE;
bad: }

Is there a way to make the parser happy?

cheers,
daniel

[Cocci] gboolean -> bool conversion

[Julia Lawall]

On Tue, 16 Jul 2013, Daniel Wagner wrote:

> Hi Julia,
>
> On 07/12/2013 09:00 AM, Julia Lawall wrote:
> > @r@
> > identifier f;
> > parameter list[n] ps;
> > identifier i;
> > @@
> >
> > f(ps, gboolean i, ...) { ... }
> >
> > @@
> > identifier r.f;
> > expression list [r.n] es;
> > @@
> >
> > f(es,
> > (
> > - FALSE
> > + false
> > |
> > - TRUE
> > + true
> > )
> >   ,...)
> >
> >
> > I have not tested this, but it should work.  Write back if you have
> > problems.
>
> This rule is a half success. The main problem stems from the fact, that a
> function is declared in a header, e.g.
>
> 	void connman_foo(gboolean foo);
>
> and the caller just sees that and not the definition. If I understood
> this correctly the 'f(ps, gboolean i, ...) { ... }' will not match.

OK, you can make a rule that matches a prototype as well.

> Another boolean transformation I do is with connman_bool_t to bool. Coccinelle
> is kind of unhappy with this function:
>
> https://git.kernel.org/cgit/network/connman/connman.git/tree/vpn/vpn-agent.c#n38
>
> parse error
>  = error in vpn/vpn-agent.c; set verbose_parsing for more info
> badcount: 20
> bad: #include "vpn.h"
> bad:
> bad: connman_bool_t vpn_agent_check_reply_has_dict(DBusMessage *reply)
> bad: {
> bad:    const char *signature = DBUS_TYPE_ARRAY_AS_STRING
> BAD:!!!!!               DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
> bad:            DBUS_TYPE_STRING_AS_STRING
> bad:            DBUS_TYPE_VARIANT_AS_STRING
> bad:            DBUS_DICT_ENTRY_END_CHAR_AS_STRING;
> bad:
> bad:    if (dbus_message_has_signature(reply, signature) == TRUE)
> bad:            return TRUE;
> bad:
> bad:    connman_warn("Reply %s to %s from %s has wrong signature %s",
> bad:                    signature,
> bad:                    dbus_message_get_interface(reply),
> bad:                    dbus_message_get_sender(reply),
> bad:                    dbus_message_get_signature(reply));
> bad:
> bad:    return FALSE;
> bad: }
>
> Is there a way to make the parser happy?

Yes, you need to give some macro definitions.  Make a copy of standard.h,
add what you need and give the name of the file with the option
--macro-file-builtins.

Concretely, you want to put the following definitions:

#define DBUS_TYPE_ARRAY_AS_STRING 0
#define DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
#define DBUS_TYPE_STRING_AS_STRING
#define DBUS_TYPE_VARIANT_AS_STRING
#define DBUS_DICT_ENTRY_END_CHAR_AS_STRING

julia

[Cocci] gboolean -> bool conversion

[Daniel Wagner]

Hi Julia,

On 07/16/2013 06:22 PM, Julia Lawall wrote:
> On Tue, 16 Jul 2013, Daniel Wagner wrote:
>
>> Hi Julia,
>>
>> On 07/12/2013 09:00 AM, Julia Lawall wrote:
>>> @r@
>>> identifier f;
>>> parameter list[n] ps;
>>> identifier i;
>>> @@
>>>
>>> f(ps, gboolean i, ...) { ... }
>>>
>>> @@
>>> identifier r.f;
>>> expression list [r.n] es;
>>> @@
>>>
>>> f(es,
>>> (
>>> - FALSE
>>> + false
>>> |
>>> - TRUE
>>> + true
>>> )
>>>    ,...)
>>>
>>>
>>> I have not tested this, but it should work.  Write back if you have
>>> problems.
>>
>> This rule is a half success. The main problem stems from the fact, that a
>> function is declared in a header, e.g.
>>
>> 	void connman_foo(gboolean foo);
>>
>> and the caller just sees that and not the definition. If I understood
>> this correctly the 'f(ps, gboolean i, ...) { ... }' will not match.
>
> OK, you can make a rule that matches a prototype as well.

I am still struggling with creating a rule for a prototype. Rereading 
examples and documentation.

So I am only allowed to write a prototype (funproto, right?) in the 
declaration section (common_decl)? How do I write the rule?

(I am really bad not getting this done by myself.)

>> Another boolean transformation I do is with connman_bool_t to bool. Coccinelle
>> is kind of unhappy with this function:
>>
>> https://git.kernel.org/cgit/network/connman/connman.git/tree/vpn/vpn-agent.c#n38
>>
>> parse error
>>   = error in vpn/vpn-agent.c; set verbose_parsing for more info
>> badcount: 20
>> bad: #include "vpn.h"
>> bad:
>> bad: connman_bool_t vpn_agent_check_reply_has_dict(DBusMessage *reply)
>> bad: {
>> bad:    const char *signature = DBUS_TYPE_ARRAY_AS_STRING
>> BAD:!!!!!               DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
>> bad:            DBUS_TYPE_STRING_AS_STRING
>> bad:            DBUS_TYPE_VARIANT_AS_STRING
>> bad:            DBUS_DICT_ENTRY_END_CHAR_AS_STRING;
>> bad:
>> bad:    if (dbus_message_has_signature(reply, signature) == TRUE)
>> bad:            return TRUE;
>> bad:
>> bad:    connman_warn("Reply %s to %s from %s has wrong signature %s",
>> bad:                    signature,
>> bad:                    dbus_message_get_interface(reply),
>> bad:                    dbus_message_get_sender(reply),
>> bad:                    dbus_message_get_signature(reply));
>> bad:
>> bad:    return FALSE;
>> bad: }
>>
>> Is there a way to make the parser happy?
>
> Yes, you need to give some macro definitions.  Make a copy of standard.h,
> add what you need and give the name of the file with the option
> --macro-file-builtins.
>
> Concretely, you want to put the following definitions:
>
> #define DBUS_TYPE_ARRAY_AS_STRING 0
> #define DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
> #define DBUS_TYPE_STRING_AS_STRING
> #define DBUS_TYPE_VARIANT_AS_STRING
> #define DBUS_DICT_ENTRY_END_CHAR_AS_STRING

That would work in the example but it would not do it for

https://git.kernel.org/cgit/network/connman/connman.git/tree/plugins/vpn.c#n666

	const char *signature = DBUS_TYPE_ARRAY_AS_STRING
		DBUS_STRUCT_BEGIN_CHAR_AS_STRING
		DBUS_TYPE_OBJECT_PATH_AS_STRING
		DBUS_TYPE_ARRAY_AS_STRING
		DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
		DBUS_TYPE_STRING_AS_STRING
		DBUS_TYPE_VARIANT_AS_STRING
		DBUS_DICT_ENTRY_END_CHAR_AS_STRING
		DBUS_STRUCT_END_CHAR_AS_STRING;

So my solution was to pull in the dbus-protocol.h header which defines 
them as strings and everyting works fine.

cheers,
daniel

[Cocci] gboolean -> bool conversion

[Julia Lawall]

> I am still struggling with creating a rule for a prototype. Rereading examples
> and documentation.
>
> So I am only allowed to write a prototype (funproto, right?) in the
> declaration section (common_decl)? How do I write the rule?
>
> (I am really bad not getting this done by myself.)

Could you send your attempt?  It should be possible to write a function
prototype pattern just like any other pattern, but it is true that I
haven't done that very much, so it might be deficient in some way.

julia

[Cocci] gboolean -> bool conversion

[Daniel Wagner]

On 07/17/2013 11:01 AM, Julia Lawall wrote:
>> I am still struggling with creating a rule for a prototype. Rereading examples
>> and documentation.
>>
>> So I am only allowed to write a prototype (funproto, right?) in the
>> declaration section (common_decl)? How do I write the rule?
>>
>> (I am really bad not getting this done by myself.)
>
> Could you send your attempt?  It should be possible to write a function
> prototype pattern just like any other pattern, but it is true that I
> haven't done that very much, so it might be deficient in some way.

So my approach was to try this here:


@r@
identifier f;
parameter list[n] ps;
identifier i;
@@

f(ps, gboolean i, ...);


And after going over the documentation I think I see my error though I 
have not figured yet how to do it right.

cheers,
daniel

[Cocci] gboolean -> bool conversion

[Julia Lawall]

On Wed, 17 Jul 2013, Daniel Wagner wrote:

> On 07/17/2013 11:01 AM, Julia Lawall wrote:
>>> I am still struggling with creating a rule for a prototype. Rereading 
>>> examples
>>> and documentation.
>>> 
>>> So I am only allowed to write a prototype (funproto, right?) in the
>>> declaration section (common_decl)? How do I write the rule?
>>> 
>>> (I am really bad not getting this done by myself.)
>> 
>> Could you send your attempt?  It should be possible to write a function
>> prototype pattern just like any other pattern, but it is true that I
>> haven't done that very much, so it might be deficient in some way.
>
> So my approach was to try this here:
>
>
> @r@
> identifier f;
> parameter list[n] ps;
> identifier i;
> @@
>
> f(ps, gboolean i, ...);

Just add the metavariable declaration

type T;

and T to the left of f in the prototype.  For function definitions, it is 
OK with the return type being absent, but apparently not for prototypes. 
Leaving it out probably causes conflicts for the parser, because a 
prototype looks like a function call.

julia

[Cocci] gboolean -> bool conversion

[Daniel Wagner]

On 07/17/2013 05:11 PM, Julia Lawall wrote:
> On Wed, 17 Jul 2013, Daniel Wagner wrote:
>
>> On 07/17/2013 11:01 AM, Julia Lawall wrote:
>>>> I am still struggling with creating a rule for a prototype.
>>>> Rereading examples
>>>> and documentation.
>>>>
>>>> So I am only allowed to write a prototype (funproto, right?) in the
>>>> declaration section (common_decl)? How do I write the rule?
>>>>
>>>> (I am really bad not getting this done by myself.)
>>>
>>> Could you send your attempt?  It should be possible to write a function
>>> prototype pattern just like any other pattern, but it is true that I
>>> haven't done that very much, so it might be deficient in some way.
>>
>> So my approach was to try this here:
>>
>>
>> @r@
>> identifier f;
>> parameter list[n] ps;
>> identifier i;
>> @@
>>
>> f(ps, gboolean i, ...);
>
> Just add the metavariable declaration
>
> type T;
>
> and T to the left of f in the prototype.  For function definitions, it
> is OK with the return type being absent, but apparently not for
> prototypes. Leaving it out probably causes conflicts for the parser,
> because a prototype looks like a function call.

Yes, that did the trick. Thanks again!

cheers,
daniel