* [Cocci] gboolean -> bool conversion
@ 2013-07-11 8:17 Daniel Wagner
2013-07-11 8:29 ` Julia Lawall
0 siblings, 1 reply; 20+ messages in thread
From: Daniel Wagner @ 2013-07-11 8:17 UTC (permalink / raw)
To: cocci
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
^ permalink raw reply [flat|nested] 20+ messages in thread* [Cocci] gboolean -> bool conversion 2013-07-11 8:17 [Cocci] gboolean -> bool conversion Daniel Wagner @ 2013-07-11 8:29 ` Julia Lawall 2013-07-11 9:07 ` Daniel Wagner 0 siblings, 1 reply; 20+ messages in thread From: Julia Lawall @ 2013-07-11 8:29 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-11 8:29 ` Julia Lawall @ 2013-07-11 9:07 ` Daniel Wagner 2013-07-11 9:57 ` Julia Lawall 2013-07-11 9:58 ` Daniel Wagner 0 siblings, 2 replies; 20+ messages in thread From: Daniel Wagner @ 2013-07-11 9:07 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-11 9:07 ` Daniel Wagner @ 2013-07-11 9:57 ` Julia Lawall 2013-07-11 9:58 ` Daniel Wagner 1 sibling, 0 replies; 20+ messages in thread From: Julia Lawall @ 2013-07-11 9:57 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-11 9:07 ` Daniel Wagner 2013-07-11 9:57 ` Julia Lawall @ 2013-07-11 9:58 ` Daniel Wagner 2013-07-11 10:24 ` Julia Lawall 1 sibling, 1 reply; 20+ messages in thread From: Daniel Wagner @ 2013-07-11 9:58 UTC (permalink / raw) To: cocci 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; ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-11 9:58 ` Daniel Wagner @ 2013-07-11 10:24 ` Julia Lawall 2013-07-11 11:17 ` Daniel Wagner 0 siblings, 1 reply; 20+ messages in thread From: Julia Lawall @ 2013-07-11 10:24 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-11 10:24 ` Julia Lawall @ 2013-07-11 11:17 ` Daniel Wagner 2013-07-11 12:17 ` Julia Lawall 0 siblings, 1 reply; 20+ messages in thread From: Daniel Wagner @ 2013-07-11 11:17 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-11 11:17 ` Daniel Wagner @ 2013-07-11 12:17 ` Julia Lawall 2013-07-11 12:38 ` Daniel Wagner 0 siblings, 1 reply; 20+ messages in thread From: Julia Lawall @ 2013-07-11 12:17 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-11 12:17 ` Julia Lawall @ 2013-07-11 12:38 ` Daniel Wagner 2013-07-11 13:07 ` Julia Lawall 0 siblings, 1 reply; 20+ messages in thread From: Daniel Wagner @ 2013-07-11 12:38 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-11 12:38 ` Daniel Wagner @ 2013-07-11 13:07 ` Julia Lawall 2013-07-12 6:55 ` Daniel Wagner 0 siblings, 1 reply; 20+ messages in thread From: Julia Lawall @ 2013-07-11 13:07 UTC (permalink / raw) To: cocci > 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-11 13:07 ` Julia Lawall @ 2013-07-12 6:55 ` Daniel Wagner 2013-07-12 7:00 ` Julia Lawall 0 siblings, 1 reply; 20+ messages in thread From: Daniel Wagner @ 2013-07-12 6:55 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-12 6:55 ` Daniel Wagner @ 2013-07-12 7:00 ` Julia Lawall 2013-07-12 9:08 ` Daniel Wagner 2013-07-16 14:21 ` Daniel Wagner 0 siblings, 2 replies; 20+ messages in thread From: Julia Lawall @ 2013-07-12 7:00 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-12 7:00 ` Julia Lawall @ 2013-07-12 9:08 ` Daniel Wagner 2013-07-16 14:21 ` Daniel Wagner 1 sibling, 0 replies; 20+ messages in thread From: Daniel Wagner @ 2013-07-12 9:08 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-12 7:00 ` Julia Lawall 2013-07-12 9:08 ` Daniel Wagner @ 2013-07-16 14:21 ` Daniel Wagner 2013-07-16 16:22 ` Julia Lawall 1 sibling, 1 reply; 20+ messages in thread From: Daniel Wagner @ 2013-07-16 14:21 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-16 14:21 ` Daniel Wagner @ 2013-07-16 16:22 ` Julia Lawall 2013-07-17 8:47 ` Daniel Wagner 0 siblings, 1 reply; 20+ messages in thread From: Julia Lawall @ 2013-07-16 16:22 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-16 16:22 ` Julia Lawall @ 2013-07-17 8:47 ` Daniel Wagner 2013-07-17 9:01 ` Julia Lawall 0 siblings, 1 reply; 20+ messages in thread From: Daniel Wagner @ 2013-07-17 8:47 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-17 8:47 ` Daniel Wagner @ 2013-07-17 9:01 ` Julia Lawall 2013-07-17 14:58 ` Daniel Wagner 0 siblings, 1 reply; 20+ messages in thread From: Julia Lawall @ 2013-07-17 9:01 UTC (permalink / raw) To: cocci > 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-17 9:01 ` Julia Lawall @ 2013-07-17 14:58 ` Daniel Wagner 2013-07-17 15:11 ` Julia Lawall 0 siblings, 1 reply; 20+ messages in thread From: Daniel Wagner @ 2013-07-17 14:58 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-17 14:58 ` Daniel Wagner @ 2013-07-17 15:11 ` Julia Lawall 2013-07-17 15:40 ` Daniel Wagner 0 siblings, 1 reply; 20+ messages in thread From: Julia Lawall @ 2013-07-17 15:11 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Cocci] gboolean -> bool conversion 2013-07-17 15:11 ` Julia Lawall @ 2013-07-17 15:40 ` Daniel Wagner 0 siblings, 0 replies; 20+ messages in thread From: Daniel Wagner @ 2013-07-17 15:40 UTC (permalink / raw) To: cocci 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2013-07-17 15:40 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-11 8:17 [Cocci] gboolean -> bool conversion Daniel Wagner 2013-07-11 8:29 ` Julia Lawall 2013-07-11 9:07 ` Daniel Wagner 2013-07-11 9:57 ` Julia Lawall 2013-07-11 9:58 ` Daniel Wagner 2013-07-11 10:24 ` Julia Lawall 2013-07-11 11:17 ` Daniel Wagner 2013-07-11 12:17 ` Julia Lawall 2013-07-11 12:38 ` Daniel Wagner 2013-07-11 13:07 ` Julia Lawall 2013-07-12 6:55 ` Daniel Wagner 2013-07-12 7:00 ` Julia Lawall 2013-07-12 9:08 ` Daniel Wagner 2013-07-16 14:21 ` Daniel Wagner 2013-07-16 16:22 ` Julia Lawall 2013-07-17 8:47 ` Daniel Wagner 2013-07-17 9:01 ` Julia Lawall 2013-07-17 14:58 ` Daniel Wagner 2013-07-17 15:11 ` Julia Lawall 2013-07-17 15:40 ` Daniel Wagner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.