<!DOCTYPE html><html><head><title></title><style type="text/css">p.MsoNormal,p.MsoNoSpacing{margin:0}</style></head><body><div>There are maximum 400 modules, but generally only 30 max are used by VLC instance. <br></div><div>I don't think we should worry about 30 more strcmp, it's negligible.<br></div><div><br></div><div>On Fri, Jul 19, 2019, at 13:43, Rémi Denis-Courmont wrote:<br></div><blockquote type="cite" id="qt"><div>Hi,<br></div><div><br></div><div>This patch changes absolutely nothing to how runtime linking failures are handled. Such failure occurs before the version check, both with or without this patch.<br></div><div><br></div><div class="qt-gmail_quote"><div>Le 19 juillet 2019 13:19:26 GMT+03:00, "Marvin Scholz (ePirat)" <epirat07@gmail.com> a écrit :<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:0pt;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><pre class="qt-k9mail"><div><br></div><div><br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(114, 159, 207);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>Am 19.07.2019 um 12:08 schrieb Steve Lhomme <robux4@ycbcr.xyz>:<br></div><div><br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(173, 127, 168);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>On 2019-07-19 11:33, Rémi Denis-Courmont wrote:<br></div><div>And vlc_dlsym does a string match. Exact same logic for the version check.<br></div></blockquote><div>I don't deny that. But there is an extra string comparison in your code.<br></div><div><br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(173, 127, 168);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote">In practice, this fails miserably. Either the plugin will not load because of missing symbols (most common), or we'll crash or corrupt the process before we can dlsym(). No change there.<br></blockquote><div>Having a generic entry point has its benefits. But that also mean we have stronger backward/forward compatibility checks. For example I sometimes have old plugins lying around in my build folder. If the version doesn't match it won't be a problem in the current code. It will realize the new entry point is not there and won't go further. In the new system it will actually load it and call the version callback.<br></div></blockquote><div><br></div><div>Actually the current behavior is imo worse, it causes to popup error dialogs on Windows which is very confusing for the user. <br></div><div>Even though since the installer was fixed this is not a very common problem anymore, I am anyway in favor of the new approach as it seems to me in general like a better way to handle this.<br></div><div><br></div><div>> <br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(114, 159, 207);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(173, 127, 168);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>Le 19 juillet 2019 12:25:02 GMT+03:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(138, 226, 52);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>On 2019-07-19 11:15, Rémi Denis-Courmont wrote:<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(252, 175, 62);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>Hi,<br></div><div><br></div><div>It was a string with a string comparison before (inside vlc_dlsym).<br></div></blockquote><div>It still is a string comparison.<br></div><div><br></div><div>In the current code we do:<br></div><div>- vlc_dlopen<br></div><div>- vlc_dlsym to load the callback that has the version in its name<br></div><div><br></div><div>In the new code we do:<br></div><div>- vlc_dlopen<br></div><div>- vlc_dlsym to load the callback with a generic name<br></div><div>- strcmp to check the version of the module<br></div><div><br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(252, 175, 62);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote">Of course it could be changed to an integer, but then the meta export<br></blockquote><div>macro won't be usable, so it will need more custom code.<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(252, 175, 62);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote">Le 19 juillet 2019 10:04:48 GMT+03:00, Steve Lhomme<br></blockquote><div><robux4@ycbcr.xyz> a écrit :<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(252, 175, 62);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(233, 185, 110);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>On 2019-07-18 21:29, Rémi Denis-Courmont wrote:<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><hr> src/modules/bank.c | 59<br></blockquote><div>+++++++++++++++++++++++++++++++++++-----------<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div> 1 file changed, 45 insertions(+), 14 deletions(-)<br></div><div><br></div><div>diff --git a/src/modules/bank.c b/src/modules/bank.c<br></div><div>index 0b551a91bc..f6f5ae1a85 100644<br></div><div>--- a/src/modules/bank.c<br></div><div>+++ b/src/modules/bank.c<br></div><div>@@ -193,6 +193,49 @@ static void module_InitStaticModules(void) { }<br></div><div> #endif<br></div><div> #ifdef HAVE_DYNAMIC_PLUGINS<br></div><div>+static const char *module_GetVersion(void *handle)<br></div><div>+{<br></div><div>+ const char *(*get_api_version)(void);<br></div><div>+<br></div><div>+ get_api_version = vlc_dlsym(handle, "vlc_entry_api_version");<br></div><div>+ if (get_api_version == NULL)<br></div><div>+ return NULL;<br></div><div>+<br></div><div>+ return get_api_version();<br></div><div>+}<br></div><div>+<br></div><div>+static void *module_Open(struct vlc_logger *log,<br></div><div>+ const char *path, bool fast)<br></div><div>+{<br></div><div>+ void *handle = vlc_dlopen(path, fast);<br></div><div>+ if (handle == NULL)<br></div><div>+ {<br></div><div>+ char *errmsg = vlc_dlerror();<br></div><div>+<br></div><div>+ vlc_error(log, "cannot load plug-in %s: %s", path,<br></div><div>+ errmsg ? errmsg : "unknown error");<br></div><div>+ free(errmsg);<br></div><div>+ return NULL;<br></div><div>+ }<br></div><div>+<br></div><div>+ const char *str = module_GetVersion(handle);<br></div><div>+ if (str == NULL) {<br></div><div>+ vlc_error(log, "cannot load plug-in %s: %s", path,<br></div><div>+ "unknown version or not a plug-in");<br></div><div>+error:<br></div><div>+ vlc_dlclose(handle);<br></div><div>+ return NULL;<br></div><div>+ }<br></div><div>+<br></div><div>+ if (strcmp(str, VLC_API_VERSION_STRING)) {<br></div></blockquote><div>Couldn't we use an integer ? Speed-wise this is going backward from<br></div><div>have<br></div><div>no extra check needed because the name we loaded already had the<br></div></blockquote></blockquote><div>proper<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(252, 175, 62);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(233, 185, 110);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>version, which was a feature not a bug.<br></div><div><br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote">+ vlc_error(log, "cannot load plug-in %s: unsupported<br></blockquote></blockquote></blockquote><div>version<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(252, 175, 62);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(233, 185, 110);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>%s", path,<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>+ str);<br></div><div>+ goto error;<br></div><div>+ }<br></div><div>+<br></div><div>+ return handle;<br></div><div>+}<br></div><div>+<br></div><div> static const char vlc_entry_name[] = "vlc_entry" MODULE_SUFFIX;<br></div><div> /**<br></div><div>@@ -207,15 +250,9 @@ static const char vlc_entry_name[] =<br></div></blockquote></blockquote></blockquote><div>"vlc_entry"<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(252, 175, 62);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(233, 185, 110);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>MODULE_SUFFIX;<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote">static vlc_plugin_t *module_InitDynamic(vlc_object_t *obj, const<br></blockquote><div>char *path,<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div> bool fast)<br></div><div> {<br></div><div>- void *handle = vlc_dlopen(path, fast);<br></div><div>+ void *handle = module_Open(obj->logger, path, fast);<br></div><div> if (handle == NULL)<br></div><div>- {<br></div><div>- char *errmsg = vlc_dlerror();<br></div><div>- msg_Err(obj, "cannot load plug-in %s: %s", path,<br></div><div>- errmsg ? errmsg : "unknown error");<br></div><div>- free(errmsg);<br></div><div> return NULL;<br></div><div>- }<br></div><div> /* Try to resolve the symbol */<br></div><div> vlc_plugin_cb entry = vlc_dlsym(handle, vlc_entry_name);<br></div><div>@@ -507,15 +544,9 @@ int module_Map(struct vlc_logger *log,<br></div></blockquote><div>vlc_plugin_t *plugin)<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div> /* Try to load the plug-in (without locks, so read-only) */<br></div><div> assert(plugin->abspath != NULL);<br></div><div> - void *handle = vlc_dlopen(plugin->abspath, false);<br></div><div>+ void *handle = module_Open(log, plugin->abspath, false);<br></div><div> if (handle == NULL)<br></div><div>- {<br></div><div>- char *errmsg = vlc_dlerror();<br></div><div>- vlc_error(log, "cannot load plug-in %s: %s",<br></div><div>- plugin->abspath, errmsg ? errmsg : "unknown<br></div></blockquote><div>error");<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(204, 204, 204);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div>- free(errmsg);<br></div><div> return -1;<br></div><div>- }<br></div><div> vlc_plugin_cb entry = vlc_dlsym(handle, vlc_entry_name);<br></div><div> if (entry == NULL)<br></div><div>-- <br></div><div>2.22.0<hr>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></blockquote><div><hr>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></blockquote><div>-- <br></div><div>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez<br></div></blockquote><div>excuser ma brièveté.<br></div><blockquote style="margin-top:0pt;margin-right:0pt;margin-bottom:1ex;margin-left:0.8ex;border-left-color:rgb(252, 175, 62);border-left-style:solid;border-left-width:1px;padding-left:1ex;" class="qt-gmail_quote"><div><hr>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></blockquote><div><hr>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></blockquote><div>-- <br></div><div>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.<hr>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></blockquote><div><hr>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></blockquote><div><hr>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div></pre></blockquote></div><div><br></div><div>-- <br></div><div>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté. <br></div><div>_______________________________________________<br></div><div>vlc-devel mailing list<br></div><div>To unsubscribe or modify your subscription options:<br></div><div>https://mailman.videolan.org/listinfo/vlc-devel<br></div></blockquote><div><br></div></body></html>