[Cocci] modifying array initializers?

[Cocci] modifying array initializers?

[Johannes Berg]

Hi,

I'm exploring backporting the netlink error reporting stuff from Linux
in backports. I have this:

static __genl_const struct genl_ops nl80211_ops[] = {
????????{
????????????????.cmd = NL80211_CMD_GET_WIPHY,
????????????????.doit = nl80211_get_wiphy,
????????????????.dumpit = nl80211_dump_wiphy,
????????????????.done = nl80211_dump_wiphy_done,
????????????????.policy = nl80211_policy,
????????????????/* can be retrieved by unprivileged users */
????????????????.internal_flags = NL80211_FLAG_NEED_WIPHY |
??????????????????????????????????NL80211_FLAG_NEED_RTNL,
????????},
????????{
????????????????.cmd = NL80211_CMD_SET_WIPHY,
????????????????.doit = nl80211_set_wiphy,
????????????????.policy = nl80211_policy,
????????????????.flags = GENL_UNS_ADMIN_PERM,
????????????????.internal_flags = NL80211_FLAG_NEED_RTNL,
????????},
	[...]
};

and would like to wrap all the .doit calls.

This works for the first instance, but I can't seem to figure out if
it's possible to apply to each one:

@@
identifier fn;
fresh identifier wrap_fn = "_wrap_" ## fn;
@@
+static int wrap_fn(struct sk_buff *skb, struct genl_info *info)
+{
+???????return fn(skb, info);
+}
static struct genl_ops nl80211_ops[] = {
????????{
-???????????????.doit = fn,
+???????????????.doit = wrap_fn,
????????????????...
????????},
????????...
};

Is there a way to loop over all the initializers?

johannes

[Cocci] modifying array initializers?

[Julia Lawall]

On Fri, 21 Apr 2017, Johannes Berg wrote:

> Hi,
>
> I'm exploring backporting the netlink error reporting stuff from Linux
> in backports. I have this:
>
> static __genl_const struct genl_ops nl80211_ops[] = {
> ????????{
> ????????????????.cmd = NL80211_CMD_GET_WIPHY,
> ????????????????.doit = nl80211_get_wiphy,
> ????????????????.dumpit = nl80211_dump_wiphy,
> ????????????????.done = nl80211_dump_wiphy_done,
> ????????????????.policy = nl80211_policy,
> ????????????????/* can be retrieved by unprivileged users */
> ????????????????.internal_flags = NL80211_FLAG_NEED_WIPHY |
> ??????????????????????????????????NL80211_FLAG_NEED_RTNL,
> ????????},
> ????????{
> ????????????????.cmd = NL80211_CMD_SET_WIPHY,
> ????????????????.doit = nl80211_set_wiphy,
> ????????????????.policy = nl80211_policy,
> ????????????????.flags = GENL_UNS_ADMIN_PERM,
> ????????????????.internal_flags = NL80211_FLAG_NEED_RTNL,
> ????????},
> 	[...]
> };
>
> and would like to wrap all the .doit calls.
>
> This works for the first instance, but I can't seem to figure out if
> it's possible to apply to each one:
>
> @@
> identifier fn;
> fresh identifier wrap_fn = "_wrap_" ## fn;
> @@
> +static int wrap_fn(struct sk_buff *skb, struct genl_info *info)
> +{
> +???????return fn(skb, info);
> +}
> static struct genl_ops nl80211_ops[] = {

Just add ..., here (note the comma at the end).

julia

> ????????{
> -???????????????.doit = fn,
> +???????????????.doit = wrap_fn,
> ????????????????...
> ????????},
> ????????...
> };
>
> Is there a way to loop over all the initializers?
>
> johannes
> _______________________________________________
> Cocci mailing list
> Cocci at systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[Cocci] modifying array initializers?

[Johannes Berg]

On Fri, 2017-04-21 at 16:07 +0200, Julia Lawall wrote:
> 
> Just add ..., here (note the comma at the end).

I thought I tried that, but maybe not. It doesn't work - at least in my
version - though:

HANDLING: net/wireless/nl80211.c
?????
previous modification:

? <<< static int wrap_fn(struct sk_buff *skb, struct genl_info *info)
? <<< {
? <<< return fn(skb, info);
? <<< }
? <<<?
CONTEXT
According to environment 2:
???rule starting on line 1.wrap_fn -> id _wrap_nl80211_wiphy_netns
???rule starting on line 1.fn -> id nl80211_wiphy_netns

current modification:

? <<< static int wrap_fn(struct sk_buff *skb, struct genl_info *info)
? <<< {
? <<< return fn(skb, info);
? <<< }
? <<<?
CONTEXT
According to environment 2:
???rule starting on line 1.wrap_fn -> id _wrap_nl80211_vendor_cmd
???rule starting on line 1.fn -> id nl80211_vendor_cmd

Fatal error: exception Failure("rule starting on line 1: already tagged
fake token\n")


Sounds like it doesn't like to add multiple things here, let me think
about how to work around that.

johannes

[Cocci] modifying array initializers?

[Julia Lawall]

On Fri, 21 Apr 2017, Johannes Berg wrote:

> On Fri, 2017-04-21 at 16:07 +0200, Julia Lawall wrote:
> >
> > Just add ..., here (note the comma at the end).
>
> I thought I tried that, but maybe not. It doesn't work - at least in my
> version - though:
>
> HANDLING: net/wireless/nl80211.c
> ?????
> previous modification:
>
> ? <<< static int wrap_fn(struct sk_buff *skb, struct genl_info *info)
> ? <<< {
> ? <<< return fn(skb, info);
> ? <<< }
> ? <<<?
> CONTEXT
> According to environment 2:
> ???rule starting on line 1.wrap_fn -> id _wrap_nl80211_wiphy_netns
> ???rule starting on line 1.fn -> id nl80211_wiphy_netns
>
> current modification:
>
> ? <<< static int wrap_fn(struct sk_buff *skb, struct genl_info *info)
> ? <<< {
> ? <<< return fn(skb, info);
> ? <<< }
> ? <<<?
> CONTEXT
> According to environment 2:
> ???rule starting on line 1.wrap_fn -> id _wrap_nl80211_vendor_cmd
> ???rule starting on line 1.fn -> id nl80211_vendor_cmd
>
> Fatal error: exception Failure("rule starting on line 1: already tagged
> fake token\n")
>
>
> Sounds like it doesn't like to add multiple things here, let me think
> about how to work around that.

If you don't find a solution, send back the rule and the code that causes
trouble, and I will take a look.

julia

[Cocci] modifying array initializers?

[Johannes Berg]

> > Sounds like it doesn't like to add multiple things here, let me
> > think about how to work around that.
> 
> If you don't find a solution, send back the rule and the code that
> causes trouble, and I will take a look.

I tried for a while now, but didn't find one. I think the problem is
that it's evaluating all of these things at once, and then it doesn't
know how to place two of the new wrap_fn functions when they both
should go before nl80211_ops[].

The code is net/wireless/n80211.c, and my current version of the
spatch, as you suggested, is this:


@@
identifier fn;
fresh identifier wrap_fn = "_wrap_" ## fn;
@@
+static int wrap_fn(struct sk_buff *skb, struct genl_info *info)
+{
+       return fn(skb, info);
+}
+
static struct genl_ops nl80211_ops[] = {
        ...,
        {
-               .doit = fn,
+               .doit = wrap_fn,
                ...
        },
        ...
};


It *does* work to match multiple .doit instances now, but it seems to
get confused when trying to place more than one wrap_fn right before
nl80211_ops[].

I tried pulling it out into a separate rule, attaching it to some other
token, e.g. after nl80211_post_doit(), but I can't seem to get that to
work.

johannes

[Cocci] modifying array initializers?

[Julia Lawall]

On Fri, 21 Apr 2017, Johannes Berg wrote:

>
> > > Sounds like it doesn't like to add multiple things here, let me
> > > think about how to work around that.
> >
> > If you don't find a solution, send back the rule and the code that
> > causes trouble, and I will take a look.
>
> I tried for a while now, but didn't find one. I think the problem is
> that it's evaluating all of these things at once, and then it doesn't
> know how to place two of the new wrap_fn functions when they both
> should go before nl80211_ops[].
>
> The code is net/wireless/n80211.c, and my current version of the
> spatch, as you suggested, is this:
>
>
> @@
> identifier fn;
> fresh identifier wrap_fn = "_wrap_" ## fn;
> @@
> +static int wrap_fn(struct sk_buff *skb, struct genl_info *info)
> +{
> +       return fn(skb, info);
> +}
> +

Use ++ instead.  You may need to put the new function all on one line.

julia

> static struct genl_ops nl80211_ops[] = {
>         ...,
>         {
> -               .doit = fn,
> +               .doit = wrap_fn,
>                 ...
>         },
>         ...
> };
>
>
> It *does* work to match multiple .doit instances now, but it seems to
> get confused when trying to place more than one wrap_fn right before
> nl80211_ops[].
>
> I tried pulling it out into a separate rule, attaching it to some other
> token, e.g. after nl80211_post_doit(), but I can't seem to get that to
> work.
>
> johannes
>

[Cocci] modifying array initializers?

[Johannes Berg]

On Fri, 2017-04-21 at 23:22 +0200, Julia Lawall wrote:

> > @@
> > identifier fn;
> > fresh identifier wrap_fn = "_wrap_" ## fn;
> > @@
> > +static int wrap_fn(struct sk_buff *skb, struct genl_info *info)
> > +{
> > +???????return fn(skb, info);
> > +}
> > +
> 
> Use ++ instead.??You may need to put the new function all on one
> line.

Indeed, that works (and yes, I had to put it on a single line)

Thanks!

johannes