[vlc-devel] [PATCH 1/2] plugin: add set_callback() macro

[Steve Lhomme]

On 2019-07-18 7:56, Thomas Guillem wrote:
> 
> 
> On Thu, Jul 18, 2019, at 07:48, Steve Lhomme wrote:
>> On 2019-07-17 22:26, Rémi Denis-Courmont wrote:
>>> This avoids registering NULL as a deactivate callback.
>>> ---
>>>    include/vlc_plugin.h | 8 ++++++--
>>>    src/modules/entry.c  | 2 +-
>>>    2 files changed, 7 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/vlc_plugin.h b/include/vlc_plugin.h
>>> index b16a313d1d..9f363eda57 100644
>>> --- a/include/vlc_plugin.h
>>> +++ b/include/vlc_plugin.h
>>> @@ -302,9 +302,13 @@ VLC_METADATA_EXPORTS
>>>         || vlc_module_set (VLC_MODULE_SCORE, (int)(score))) \
>>>            goto error;
>>>    
>>> +#define set_callback(activate) \
>>> +    if (vlc_module_set(VLC_MODULE_CB_OPEN, #activate, (void *)(activate))) \
>>> +        goto error;
>>
>> Maybe use a more meaningful name like set_open_callback() or
>> set_init_callback(). It's more readable.
> 
> If we rename it like you said, you'll have to rename set_display_callback to set_open_display_callback and so on.

Yes, that's fine. We can also remove the _callback part here, since we 
already know it's a callback.

> I'm OK with the current name
> 
> OK for the set.
> 
>>
>>> +
>>>    #define set_callbacks( activate, deactivate ) \
>>> -    if (vlc_module_set(VLC_MODULE_CB_OPEN, #activate, (void *)(activate)) \
>>> -     || vlc_module_set(VLC_MODULE_CB_CLOSE, #deactivate, \
>>> +    set_callback(activate) \
>>> +    if (vlc_module_set(VLC_MODULE_CB_CLOSE, #deactivate, \
>>>                           (void *)(deactivate))) \
>>>            goto error;
>>>    
>>> diff --git a/src/modules/entry.c b/src/modules/entry.c
>>> index bce94285c6..21eb09bf3c 100644
>>> --- a/src/modules/entry.c
>>> +++ b/src/modules/entry.c
>>> @@ -570,7 +570,7 @@ static int vlc_plugin_get_symbol(void *root, const char *name,
>>>                                     void **restrict addrp)
>>>    {
>>>        if (name == NULL)
>>> -    {   /* TODO: use this; do not define "NULL" as a name for NULL? */
>>> +    {
>>>            *addrp = NULL;
>>>            return 0;
>>>        }
>>> -- 
>>> 2.22.0
>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>