On Thu, 3 Nov 2016, Daniel Lezcano wrote:
> On 02/11/2016 22:40, Julia Lawall wrote:
> >
> >
> > On Wed, 2 Nov 2016, Daniel Lezcano wrote:
> >
> >> Hi all,
> >>
> >> I'm very new to coccinelle, so sorry if the question is irrelevant or was
> >> already asked.
> >>
> >> I'm trying to add a new argument to a function which has a couple of
> >> callbacks.
> >>
> >> The original function is:
> >>
> >> static inline int cpuhp_setup_state(enum cpuhp_state state,
> >> int (*startup)(unsigned int cpu),
> >> int (*teardown)(unsigned int cpu));
> >>
> >> I would like to pass a private pointer when setting the state and then pass
> >> this pointer to the 'startup' / 'teardown' callbacks where there signature
> >> will be
> >> different and all call sites must change their callbacks signature also.
> >>
> >> I tried to first add the new argument to the function, in order to have:
> >>
> >> static inline int cpuhp_setup_state(enum cpuhp_state state,
> >> int (*startup)(unsigned int cpu),
> >> int (*teardown)(unsigned int cpu),
> >> void *data);
> >>
> >> ... with the rule:
> >>
> >> @rule@
> >> identifier cpuhp_setup_state;
> >> identifier state, name, startup, teardown, data;
> >> @@
> >>
> >> static inline int cpuhp_setup_state(enum cpuhp_state state,
Here you make the name of the function a metavariable. This will cause it
to make the change to any function with any name that has a signature that
looks like this. If you want it to be only this function, you should not
declare its name as a metavariable, but just put it directly. For the
parameter names, it will give the warning you have below about cpu if you
don't either make them metavariables with identifier, or declare them with
symbol, indicating that you know that you really want that name. But you
can also just ignore the warning.
The second problem is that this function is defined in a header file.
Coccinelle does not automatically include everything that is indicated
with #include. Indeed, it doesn't run the C preprocessor at all. If you
give the option --no-includes then you will get no includes at all. The
default with no options is to include the local files and those with the
same name as the .c file, ie foo.h for foo.c. --all-includes gives all
the include files mentioned in the .c file. --recursive-includes gives
all the files they include and so on. The more include files you have,
the more time everthing will take. In your case, --recursive-includes
would be needed to see the definition at the same time as the use, because
the function is defined in include/linux/cpuhotplug.h, which is not
included explicitly. But that would probably take a huge amount of time.
Actually, all you want to do is process the relevant definition in the
relevant include file once, not once for every file that it is included
in. So you want the options --include-headers --no-includes. Without
--include-headers, Coccinelle will only look at the .c files.
Since you are working on a specific function with a known name, you can
benefit from indexing. You may want to first rule
coccinelle/scripts/idutile_index.sh on your kernel source tree. Then put
--use-idutils at the end of your command line. You can also run mkid
directly in your kernel source tree. That will give the index a name that
is different than what Coccinelle expects, and you will have to give the
name of the index as an argument to --use-idutils. You can also run
Coccinelle in parallel with the -j option.
julia
> >> const char *name,
> >> int (*startup)(unsigned int cpu),
> >> int (*teardown)(unsigned int cpu),
> >> void *data)
> >> {
> >> ...
> >> }
> >>
> >> @@
> >> identifier rule.cpuhp_setup_state;
> >> expression E1, E2, E3, E4;
> >> @@
> >>
> >> cpuhp_setup_state(E1, E2, E3, E4
> >> + , NULL
> >> )
> >>
> >> But the parsing gives:
> >>
> >> ...
> >>
> >> cpuhp_setup_state(E1, E2, E3, E4
> >> >>> , NULL
> >
> > This is normal. The >>> has to do with whether the NULL is to be put
> > after what comes before or before what comes after. <<< would mean the
> > opposite. Unfortunately, I don't remember which is which, but it doesn't
> > really matter in this case either.
> >
> >> )
> >>
> >>
> >> grep tokens
> >> cpuhp_state || cpu
> >> No query
> >> warning: line 8: should cpu be a metavariable?
> >> warning: line 9: should cpu be a metavariable?
> >
> > I guess you would want an identifier cpu metavriable declaration in the
> > first rule. Otherwise, I don't really see any problems. Do you have a .c
> > file on which you expect to get a result?
>
> Yes, that is all around the linux kernel code (eg.
> ./drivers/clocksource/qcom-timer.c)
>
> If I change the cocci file to:
>
> @@
> expression E1, E2, E3, E4;
> @@
>
> -cpuhp_setup_state(E1, E2, E3, E4)
> +cpuhp_setup_state(E1, E2, E3, E4, NULL)
>
> It seems to work, except the function signature itself is not changed. I
> can do add a manual change in the patch but if coccinelle can handle
> both that would be nice.
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
>_______________________________________________
Cocci mailing list
[email protected]
https://systeme.lip6.fr/mailman/listinfo/cocci
