[vlc-devel] [PATCH 2/6] modules: check plugin version

[Steve Lhomme]

On 2019-07-19 11:15, Rémi Denis-Courmont wrote:
> Hi,
> 
> It was a string with a string comparison before (inside vlc_dlsym). It still is a string comparison.

In the current code we do:
- vlc_dlopen
- vlc_dlsym to load the callback that has the version in its name

In the new code we do:
- vlc_dlopen
- vlc_dlsym  to load the callback with a generic name
- strcmp to check the version of the module

> Of course it could be changed to an integer, but then the meta export macro won't be usable, so it will need more custom code.
> 
> Le 19 juillet 2019 10:04:48 GMT+03:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>> On 2019-07-18 21:29, Rémi Denis-Courmont wrote:
>>> ---
>>>    src/modules/bank.c | 59
>> +++++++++++++++++++++++++++++++++++-----------
>>>    1 file changed, 45 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/src/modules/bank.c b/src/modules/bank.c
>>> index 0b551a91bc..f6f5ae1a85 100644
>>> --- a/src/modules/bank.c
>>> +++ b/src/modules/bank.c
>>> @@ -193,6 +193,49 @@ static void module_InitStaticModules(void) { }
>>>    #endif
>>>    
>>>    #ifdef HAVE_DYNAMIC_PLUGINS
>>> +static const char *module_GetVersion(void *handle)
>>> +{
>>> +    const char *(*get_api_version)(void);
>>> +
>>> +    get_api_version = vlc_dlsym(handle, "vlc_entry_api_version");
>>> +    if (get_api_version == NULL)
>>> +        return NULL;
>>> +
>>> +    return get_api_version();
>>> +}
>>> +
>>> +static void *module_Open(struct vlc_logger *log,
>>> +                         const char *path, bool fast)
>>> +{
>>> +    void *handle = vlc_dlopen(path, fast);
>>> +    if (handle == NULL)
>>> +    {
>>> +        char *errmsg = vlc_dlerror();
>>> +
>>> +        vlc_error(log, "cannot load plug-in %s: %s", path,
>>> +                  errmsg ? errmsg : "unknown error");
>>> +        free(errmsg);
>>> +        return NULL;
>>> +    }
>>> +
>>> +    const char *str = module_GetVersion(handle);
>>> +    if (str == NULL) {
>>> +        vlc_error(log, "cannot load plug-in %s: %s", path,
>>> +                  "unknown version or not a plug-in");
>>> +error:
>>> +        vlc_dlclose(handle);
>>> +        return NULL;
>>> +    }
>>> +
>>> +    if (strcmp(str, VLC_API_VERSION_STRING)) {
>>
>> Couldn't we use an integer ? Speed-wise this is going backward from
>> have
>> no extra check needed because the name we loaded already had the proper
>>
>> version, which was a feature not a bug.
>>
>>> +        vlc_error(log, "cannot load plug-in %s: unsupported version
>> %s", path,
>>> +                  str);
>>> +        goto error;
>>> +    }
>>> +
>>> +    return handle;
>>> +}
>>> +
>>>    static const char vlc_entry_name[] = "vlc_entry" MODULE_SUFFIX;
>>>    
>>>    /**
>>> @@ -207,15 +250,9 @@ static const char vlc_entry_name[] = "vlc_entry"
>> MODULE_SUFFIX;
>>>    static vlc_plugin_t *module_InitDynamic(vlc_object_t *obj, const
>> char *path,
>>>                                            bool fast)
>>>    {
>>> -    void *handle = vlc_dlopen(path, fast);
>>> +    void *handle = module_Open(obj->logger, path, fast);
>>>        if (handle == NULL)
>>> -    {
>>> -        char *errmsg = vlc_dlerror();
>>> -        msg_Err(obj, "cannot load plug-in %s: %s", path,
>>> -                errmsg ? errmsg : "unknown error");
>>> -        free(errmsg);
>>>            return NULL;
>>> -    }
>>>    
>>>        /* Try to resolve the symbol */
>>>        vlc_plugin_cb entry = vlc_dlsym(handle, vlc_entry_name);
>>> @@ -507,15 +544,9 @@ int module_Map(struct vlc_logger *log,
>> vlc_plugin_t *plugin)
>>>        /* Try to load the plug-in (without locks, so read-only) */
>>>        assert(plugin->abspath != NULL);
>>>    
>>> -    void *handle = vlc_dlopen(plugin->abspath, false);
>>> +    void *handle = module_Open(log, plugin->abspath, false);
>>>        if (handle == NULL)
>>> -    {
>>> -        char *errmsg = vlc_dlerror();
>>> -        vlc_error(log, "cannot load plug-in %s: %s",
>>> -                  plugin->abspath, errmsg ? errmsg : "unknown
>> error");
>>> -        free(errmsg);
>>>            return -1;
>>> -    }
>>>    
>>>        vlc_plugin_cb entry = vlc_dlsym(handle, vlc_entry_name);
>>>        if (entry == NULL)
>>> -- 
>>> 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
> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>