[CRIU] [PATCH 2/3] plugin: Rework plugins API, v2

[Pavel Emelyanov]

On 03/14/2014 04:59 PM, Cyrill Gorcunov wrote:
> On Fri, Mar 14, 2014 at 04:50:21PM +0400, Pavel Emelyanov wrote:
>> On 03/14/2014 04:38 PM, Cyrill Gorcunov wrote:
>>> On Fri, Mar 14, 2014 at 04:27:59PM +0400, Pavel Emelyanov wrote:
>>>> On 03/13/2014 06:55 PM, Cyrill Gorcunov wrote:
>>>>> On Thu, Mar 13, 2014 at 06:41:17PM +0400, Cyrill Gorcunov wrote:
>>>>>>
>>>>>> Here we define new api to be used in plugins.
>>>>
>>>>> The idea behind is to not limit plugins authors with names
>>>>> of functions they might need to use for particular hook.
>>>>
>>>> Is that the only thing we're trying to address?
>>>> Why do we consider this as problem?
>>>
>>> Because it's more convenient to work with. Look, with new plugin API if
>>> you introduce new hook entry you need
>>>
>>> 1) Add new enum CR_PLUGIN_HOOK__
>>> 2) Specify arguments for hook as
>>>
>>> 	DECLARE_PLUGIN_HOOK_ARGS(CR_PLUGIN_HOOK__DUMP_UNIX_SK, int fd, int id);
>>
>> So before the patches the plugin writer had to write
>>
>>    static int dump_ext_unix_sk(int fd, int id)
>>
>> where the "dump_ext_unix_sk" is constant name. After the patch, the writer has to write
>>
>>    DECLARE_PLUGIN_HOOK_ARGS(CR_PLUGIN_HOOK__DUMP_UNIX_SK, int fd, int id)
>>
>> were the whole "DECLARE_PLUGIN_HOOK_ARGS(CR_PLUGIN_HOOK__DUMP_UNIX_SK" string
>> is constant (and in capitals).
>>
>> Why is the 2nd approach better?
> 
> No, with second approach plugin writer can use any name he likes for the hook, but define the hook as
> 
>      | static int dump_ext_file(int fd, int id)
>      | {
>      |	pr_info("dump_ext_file: fd %d id %d\n", fd, id);
>      |	return 0;
>      | }
>      |
>      | CR_PLUGIN_REGISTER_HOOK(CR_PLUGIN_HOOK__DUMP_EXT_FILE, dump_ext_file)

OK, why not do it simpler, by introducing a helper

#define CR_PLUGIN_REGISTER_HOOK(type, fn, args)		\
static int type(args)					\
{							\
	return fn(args);				\
}

and use it

static int my_dump_fn()
{
}

CR_PLUGIN_REGISTER_HOOK(dump_ext_unix_sk, my_dump_fn, (int fd, int id))

?

> 
> have you read changelog? Same is done by linux modules for init and exit functions,

In the kernel this is done to properly resolve init/exit locations of such
and initcalls, not just for neat names.

> for module parameters. This allows a plugin driver to grep over all source code
> to find which hooks has been defined by "grep CR_PLUGIN_REGISTER_HOOK". As to
> me this is more convenient.
> .
>