Hi Julia,
thanks for the long explanation. With the great help of Viresh, it is
almost done. However I miss a rule to add a comment description in the
comment cartridge of the changed function.
/**
* cpuhp_setup_state - Setup hotplug state
*
* @state: The state for which the calls
* @name: Name of the callback
* @startup: startup callback function
* @teardown: teardown callback function
+ * @data: startup/teardown's cookie
*
* Installs the callback functions and invokes the startup
* callback on the present cpus which have already reached
* the @state.
*/
Apparently, coccinelle does not like the '@' symbol.
Is it possible to add such comment ?
Thanks !
-- Daniel
On 03/11/2016 07:17, Julia Lawall wrote:
>
>
> 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
>>
>>
>
--
<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
