<html><head></head><body>Hi,<br><br>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><br><div class="gmail_quote">Le 19 juillet 2019 13:19:26 GMT+03:00, "Marvin Scholz (ePirat)" <epirat07@gmail.com> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail"><br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">Am 19.07.2019 um 12:08 schrieb Steve Lhomme <robux4@ycbcr.xyz>:<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">On 2019-07-19 11:33, Rémi Denis-Courmont wrote:<br>And vlc_dlsym does a string match. Exact same logic for the version check.<br></blockquote>I don't deny that. But there is an extra string comparison in your code.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">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>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></blockquote><br>Actually the current behavior is imo worse, it causes to popup error dialogs on Windows which is very confusing for the user. <br>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><br>> <br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">Le 19 juillet 2019 12:25:02 GMT+03:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">On 2019-07-19 11:15, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;">Hi,<br><br>It was a string with a string comparison before (inside vlc_dlsym).<br></blockquote>It still is a string comparison.<br><br>In the current code we do:<br>- vlc_dlopen<br>- vlc_dlsym to load the callback that has the version in its name<br><br>In the new code we do:<br>- vlc_dlopen<br>- vlc_dlsym to load the callback with a generic name<br>- strcmp to check the version of the module<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;">Of course it could be changed to an integer, but then the meta export<br></blockquote>macro won't be usable, so it will need more custom code.<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;">Le 19 juillet 2019 10:04:48 GMT+03:00, Steve Lhomme<br></blockquote><robux4@ycbcr.xyz> a écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #e9b96e; padding-left: 1ex;">On 2019-07-18 21:29, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ccc; padding-left: 1ex;"><hr> src/modules/bank.c | 59<br></blockquote>+++++++++++++++++++++++++++++++++++-----------<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ccc; padding-left: 1ex;"> 1 file changed, 45 insertions(+), 14 deletions(-)<br><br>diff --git a/src/modules/bank.c b/src/modules/bank.c<br>index 0b551a91bc..f6f5ae1a85 100644<br>--- a/src/modules/bank.c<br>+++ b/src/modules/bank.c<br>@@ -193,6 +193,49 @@ static void module_InitStaticModules(void) { }<br> #endif<br> #ifdef HAVE_DYNAMIC_PLUGINS<br>+static const char *module_GetVersion(void *handle)<br>+{<br>+ const char *(*get_api_version)(void);<br>+<br>+ get_api_version = vlc_dlsym(handle, "vlc_entry_api_version");<br>+ if (get_api_version == NULL)<br>+ return NULL;<br>+<br>+ return get_api_version();<br>+}<br>+<br>+static void *module_Open(struct vlc_logger *log,<br>+ const char *path, bool fast)<br>+{<br>+ void *handle = vlc_dlopen(path, fast);<br>+ if (handle == NULL)<br>+ {<br>+ char *errmsg = vlc_dlerror();<br>+<br>+ vlc_error(log, "cannot load plug-in %s: %s", path,<br>+ errmsg ? errmsg : "unknown error");<br>+ free(errmsg);<br>+ return NULL;<br>+ }<br>+<br>+ const char *str = module_GetVersion(handle);<br>+ if (str == NULL) {<br>+ vlc_error(log, "cannot load plug-in %s: %s", path,<br>+ "unknown version or not a plug-in");<br>+error:<br>+ vlc_dlclose(handle);<br>+ return NULL;<br>+ }<br>+<br>+ if (strcmp(str, VLC_API_VERSION_STRING)) {<br></blockquote>Couldn't we use an integer ? Speed-wise this is going backward from<br>have<br>no extra check needed because the name we loaded already had the<br></blockquote></blockquote>proper<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #e9b96e; padding-left: 1ex;">version, which was a feature not a bug.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ccc; padding-left: 1ex;">+ vlc_error(log, "cannot load plug-in %s: unsupported<br></blockquote></blockquote></blockquote>version<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #e9b96e; padding-left: 1ex;">%s", path,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ccc; padding-left: 1ex;">+ str);<br>+ goto error;<br>+ }<br>+<br>+ return handle;<br>+}<br>+<br> static const char vlc_entry_name[] = "vlc_entry" MODULE_SUFFIX;<br> /**<br>@@ -207,15 +250,9 @@ static const char vlc_entry_name[] =<br></blockquote></blockquote></blockquote>"vlc_entry"<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #e9b96e; padding-left: 1ex;">MODULE_SUFFIX;<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ccc; padding-left: 1ex;">static vlc_plugin_t *module_InitDynamic(vlc_object_t *obj, const<br></blockquote>char *path,<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ccc; padding-left: 1ex;"> bool fast)<br> {<br>- void *handle = vlc_dlopen(path, fast);<br>+ void *handle = module_Open(obj->logger, path, fast);<br> if (handle == NULL)<br>- {<br>- char *errmsg = vlc_dlerror();<br>- msg_Err(obj, "cannot load plug-in %s: %s", path,<br>- errmsg ? errmsg : "unknown error");<br>- free(errmsg);<br> return NULL;<br>- }<br> /* Try to resolve the symbol */<br> vlc_plugin_cb entry = vlc_dlsym(handle, vlc_entry_name);<br>@@ -507,15 +544,9 @@ int module_Map(struct vlc_logger *log,<br></blockquote>vlc_plugin_t *plugin)<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ccc; padding-left: 1ex;"> /* Try to load the plug-in (without locks, so read-only) */<br> assert(plugin->abspath != NULL);<br> - void *handle = vlc_dlopen(plugin->abspath, false);<br>+ void *handle = module_Open(log, plugin->abspath, false);<br> if (handle == NULL)<br>- {<br>- char *errmsg = vlc_dlerror();<br>- vlc_error(log, "cannot load plug-in %s: %s",<br>- plugin->abspath, errmsg ? errmsg : "unknown<br></blockquote>error");<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ccc; padding-left: 1ex;">- free(errmsg);<br> return -1;<br>- }<br> vlc_plugin_cb entry = vlc_dlsym(handle, vlc_entry_name);<br> if (entry == NULL)<br>-- <br>2.22.0<hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez<br></blockquote>excuser ma brièveté.<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;"><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.<hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>