On 3/26/20 1:25 PM, Richard W.M. Jones wrote:
Note that this probably isn't testing anything because at least
on
Linux the -undefined/-no-undefined flags appear to have no effect on
linking.
[skipping review of 5/9 for now, due to its size. I don't know how easy
or hard it would be to split it into bisectable pieces: first
introducing the library with just the parsing symbols that can be done
locally, then adding more symbols, before finally getting to where
-no-undefined actually works for plugins. But as you said on IRC, it
gets tricky to have to get a half-finished library to still work
correctly, so it may be worth leaving that one squashed]
I'm using the -undefined flag here, but this is not actually a
flag
documented anywhere in the libtool documentation, but I assume it's
fine because libtool doesn't complain.
I'll have to research if it is just ignoring an unknown flag, or
actually honoring it (but even if it is ignoring the flag, libtool's
default behavior of allowing undefined symbols on Linux is what we are
indeed trying to test). This test will have to be conditional on
platform (it won't work on mingw); so you may have to revive the
configure.ac stuff you chopped out in patch 4.
---
tests/Makefile.am | 28 ++++++++++++++
tests/test-undefined.sh | 36 ++++++++++++++++++
tests/test-undefined-plugin.c | 70 +++++++++++++++++++++++++++++++++++
3 files changed, 134 insertions(+)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f9c6b8ad..901dca7e 100644
--- a/tests/Makefile.am
+# Build a plugin with libtool -undefined flag. This is how plugins
+# were built before libnbdkit.so existed.
+TESTS += \
+ test-undefined.sh \
+ $(NULL)
+# For use of the -rpath option, see:
+#
https://lists.gnu.org/archive/html/libtool/2007-07/msg00067.html
+test_undefined_plugin_la_LDFLAGS = \
+ -module -avoid-version -shared \
+ -undefined \
+ $(SHARED_LDFLAGS) \
+ -rpath /nowhere \
+ $(NULL)
The most important part here is that we did NOT pass in libnbdkit.so on
the link line, but it still builds, resulting in a plugin with undefined
symbols which can still be loaded by the just-built nbdkit.
+++ b/tests/test-undefined-plugin.c
@@ -0,0 +1,70 @@
+
+#include <config.h>
Does our plugin need <config.h>? Or can we omit that to be more like a
true out-of-tree plugin that did not opt-in to our library?
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <nbdkit-plugin.h>
+
+static void *
+undefined_open (int readonly)
+{
+ return NBDKIT_HANDLE_NOT_NEEDED;
Hmm. I don't see any _use_ of an nbdkit_* function here. It's not
enough that this library links without libnbdkit.so, but we need to be
testing that when it uses an undefined symbol, such a symbol actually
gets provided via the nbdkit binary dlopen()ing this plugin.
+static struct nbdkit_plugin plugin = {
+ .name = "testundefined",
+ .version = PACKAGE_VERSION,
+ .open = undefined_open,
+ .get_size = undefined_get_size,
+ .pread = undefined_pread,
+};
Easiest might be adding a .config callback to test an nbdkit_parse_*
function.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization:
qemu.org |
libvirt.org