[Libguestfs] [PATCH nbdkit] Set visibility to default for exported symbols
Richard W.M. Jones
rjones at redhat.com
Wed Jun 2 21:06:21 UTC 2021
On Wed, Jun 02, 2021 at 09:42:52PM +0100, Daniel P. Berrangé wrote:
> On Wed, Jun 02, 2021 at 09:05:26PM +0100, Richard W.M. Jones wrote:
> > In theory this should also be required for all the nbdkit_*
> > functions exported by the server, but I found that it is not needed by
> > either GCC or Clang.
I think this part is probably wrong. In this patch I'm redefining
NBDKIT_EXTERN_DECL to include default visibility, and this is included
in the files which define the nbdkit_* functions, so I think they get
default visibility as a side-effect of that. (More below ...)
> This is kinda wierd, as it feels like we're duplicating information
> that is already available in the linker script, but I guess the clue
> is in the name of the latter. The visibility option is processed by
> the compiler, while the linker script is processed by the linker
> and thus too late for the compiler to use.
>
> It is annoying to have to maintain both a .syms script and the source
> level annotations. Perhaps there is value in a script that looks for
> the NBDKIT_DLL_PUBLIC annotations and uses them to auto-generated a
> .syms file for the linker ?
I'm not too committed to this patch. It arises because of some
experiments I'm trying with Clang + CFI.
It might be possible to derive the .syms files from the sources,
although it sounds a bit hairy.
> > ---
> > include/nbdkit-common.h | 6 +++++-
> > include/nbdkit-filter.h | 1 +
> > include/nbdkit-plugin.h | 1 +
> > tests/Makefile.am | 1 +
> > plugins/ocaml/bindings.c | 26 +++++++++++++-------------
> > plugins/ocaml/plugin.c | 14 +++++++-------
> > tests/dummy-vddk.c | 30 ++++++++++++++++--------------
> > 7 files changed, 44 insertions(+), 35 deletions(-)
> >
> > diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h
> > index 51a6264c..f43ebe59 100644
> > --- a/include/nbdkit-common.h
> > +++ b/include/nbdkit-common.h
> > @@ -83,10 +83,14 @@ extern "C" {
> > #define NBDKIT_EXTENT_ZERO (1<<1) /* Same as NBD_STATE_ZERO */
> >
> > #ifndef WIN32
> > -#define NBDKIT_EXTERN_DECL(ret, fn, args) extern ret fn args
> > +#define NBDKIT_EXTERN_DECL(ret, fn, args) \
> > + __attribute__((__visibility__("default"))) \
> > + extern ret fn args
(Contd from above) Therefore this part of the change is wrong. It
should use NBDKIT_DLL_PUBLIC on the function definitions instead.
Rich.
> > +#define NBDKIT_DLL_PUBLIC __attribute__((__visibility__("default")))
> > #else
> > #define NBDKIT_EXTERN_DECL(ret, fn, args) \
> > extern __declspec(dllexport) ret fn args
> > +#define NBDKIT_DLL_PUBLIC __declspec(dllimport)
> > #endif
> >
> > NBDKIT_EXTERN_DECL (void, nbdkit_error,
> > diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h
> > index 633d34e7..8b587d70 100644
> > --- a/include/nbdkit-filter.h
> > +++ b/include/nbdkit-filter.h
> > @@ -263,6 +263,7 @@ struct nbdkit_filter {
> >
> > #define NBDKIT_REGISTER_FILTER(filter) \
> > NBDKIT_CXX_LANG_C \
> > + NBDKIT_DLL_PUBLIC \
> > struct nbdkit_filter * \
> > filter_init (void) \
> > { \
> > diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h
> > index 611987c7..59ec11b6 100644
> > --- a/include/nbdkit-plugin.h
> > +++ b/include/nbdkit-plugin.h
> > @@ -154,6 +154,7 @@ NBDKIT_EXTERN_DECL (int, nbdkit_is_tls, (void));
> >
> > #define NBDKIT_REGISTER_PLUGIN(plugin) \
> > NBDKIT_CXX_LANG_C \
> > + NBDKIT_DLL_PUBLIC \
> > struct nbdkit_plugin * \
> > plugin_init (void) \
> > { \
> > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > index ae801290..acb74fa8 100644
> > --- a/tests/Makefile.am
> > +++ b/tests/Makefile.am
> > @@ -1027,6 +1027,7 @@ libvixDiskLib_la_SOURCES = \
> > $(NULL)
> > libvixDiskLib_la_CPPFLAGS = \
> > -I$(top_srcdir)/plugins/vddk \
> > + -I$(top_srcdir)/include \
> > $(NULL)
> > libvixDiskLib_la_CXXFLAGS = $(WARNINGS_CFLAGS)
> > # For use of the -rpath option, see:
> > diff --git a/plugins/ocaml/bindings.c b/plugins/ocaml/bindings.c
> > index 493b147b..a6d57084 100644
> > --- a/plugins/ocaml/bindings.c
> > +++ b/plugins/ocaml/bindings.c
> > @@ -51,7 +51,7 @@
> > /* Bindings for miscellaneous nbdkit_* utility functions. */
> >
> > /* NB: noalloc function. */
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_set_error (value nv)
> > {
> > int err;
> > @@ -77,7 +77,7 @@ ocaml_nbdkit_set_error (value nv)
> > return Val_unit;
> > }
> >
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_parse_size (value strv)
> > {
> > CAMLparam1 (strv);
> > @@ -92,7 +92,7 @@ ocaml_nbdkit_parse_size (value strv)
> > CAMLreturn (rv);
> > }
> >
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_parse_bool (value strv)
> > {
> > CAMLparam1 (strv);
> > @@ -107,7 +107,7 @@ ocaml_nbdkit_parse_bool (value strv)
> > CAMLreturn (rv);
> > }
> >
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_read_password (value strv)
> > {
> > CAMLparam1 (strv);
> > @@ -124,7 +124,7 @@ ocaml_nbdkit_read_password (value strv)
> > CAMLreturn (rv);
> > }
> >
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_realpath (value strv)
> > {
> > CAMLparam1 (strv);
> > @@ -140,7 +140,7 @@ ocaml_nbdkit_realpath (value strv)
> > CAMLreturn (rv);
> > }
> >
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_nanosleep (value secv, value nsecv)
> > {
> > CAMLparam2 (secv, nsecv);
> > @@ -155,7 +155,7 @@ ocaml_nbdkit_nanosleep (value secv, value nsecv)
> > CAMLreturn (Val_unit);
> > }
> >
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_export_name (value unitv)
> > {
> > CAMLparam1 (unitv);
> > @@ -174,7 +174,7 @@ ocaml_nbdkit_export_name (value unitv)
> > }
> >
> > /* NB: noalloc function. */
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_shutdown (value unitv)
> > {
> > CAMLparam1 (unitv);
> > @@ -184,7 +184,7 @@ ocaml_nbdkit_shutdown (value unitv)
> > }
> >
> > /* NB: noalloc function. */
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_debug (value strv)
> > {
> > nbdkit_debug ("%s", String_val (strv));
> > @@ -192,7 +192,7 @@ ocaml_nbdkit_debug (value strv)
> > return Val_unit;
> > }
> >
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_version (value unitv)
> > {
> > CAMLparam1 (unitv);
> > @@ -202,7 +202,7 @@ ocaml_nbdkit_version (value unitv)
> > CAMLreturn (rv);
> > }
> >
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_peer_pid (value unitv)
> > {
> > CAMLparam1 (unitv);
> > @@ -213,7 +213,7 @@ ocaml_nbdkit_peer_pid (value unitv)
> > CAMLreturn (rv);
> > }
> >
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_peer_uid (value unitv)
> > {
> > CAMLparam1 (unitv);
> > @@ -224,7 +224,7 @@ ocaml_nbdkit_peer_uid (value unitv)
> > CAMLreturn (rv);
> > }
> >
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_peer_gid (value unitv)
> > {
> > CAMLparam1 (unitv);
> > diff --git a/plugins/ocaml/plugin.c b/plugins/ocaml/plugin.c
> > index dc2559b5..00959cb6 100644
> > --- a/plugins/ocaml/plugin.c
> > +++ b/plugins/ocaml/plugin.c
> > @@ -85,7 +85,7 @@ static struct nbdkit_plugin plugin = {
> > .unload = unload_wrapper,
> > };
> >
> > -struct nbdkit_plugin *
> > +NBDKIT_DLL_PUBLIC struct nbdkit_plugin *
> > plugin_init (void)
> > {
> > if (plugin.name == NULL) {
> > @@ -743,7 +743,7 @@ cache_wrapper (void *h, uint32_t count, uint64_t offset, uint32_t flags)
> > */
> >
> > /* NB: noalloc function */
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_set_name (value namev)
> > {
> > plugin.name = strdup (String_val (namev));
> > @@ -751,7 +751,7 @@ ocaml_nbdkit_set_name (value namev)
> > }
> >
> > /* NB: noalloc function */
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_set_longname (value longnamev)
> > {
> > plugin.longname = strdup (String_val (longnamev));
> > @@ -759,7 +759,7 @@ ocaml_nbdkit_set_longname (value longnamev)
> > }
> >
> > /* NB: noalloc function */
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_set_version (value versionv)
> > {
> > plugin.version = strdup (String_val (versionv));
> > @@ -767,7 +767,7 @@ ocaml_nbdkit_set_version (value versionv)
> > }
> >
> > /* NB: noalloc function */
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_set_description (value descriptionv)
> > {
> > plugin.description = strdup (String_val (descriptionv));
> > @@ -775,7 +775,7 @@ ocaml_nbdkit_set_description (value descriptionv)
> > }
> >
> > /* NB: noalloc function */
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_set_config_help (value helpv)
> > {
> > plugin.config_help = strdup (String_val (helpv));
> > @@ -783,7 +783,7 @@ ocaml_nbdkit_set_config_help (value helpv)
> > }
> >
> > /* NB: noalloc function */
> > -value
> > +NBDKIT_DLL_PUBLIC value
> > ocaml_nbdkit_set_field (value fieldv, value fv)
> > {
> > const char *field = String_val (fieldv);
> > diff --git a/tests/dummy-vddk.c b/tests/dummy-vddk.c
> > index c100fc8f..9b5ae0a2 100644
> > --- a/tests/dummy-vddk.c
> > +++ b/tests/dummy-vddk.c
> > @@ -42,6 +42,8 @@
> >
> > #include <pthread.h>
> >
> > +#include "nbdkit-plugin.h" /* only for NBDKIT_DLL_PUBLIC */
> > +
> > #include "vddk-structs.h"
> >
> > #define STUB(fn,ret,args) extern ret fn args;
> > @@ -64,7 +66,7 @@ bg_thread (void *datav)
> > return NULL;
> > }
> >
> > -VixError
> > +NBDKIT_DLL_PUBLIC VixError
> > VixDiskLib_InitEx (uint32_t major, uint32_t minor,
> > VixDiskLibGenericLogFunc *log_function,
> > VixDiskLibGenericLogFunc *warn_function,
> > @@ -88,32 +90,32 @@ VixDiskLib_InitEx (uint32_t major, uint32_t minor,
> > return VIX_OK;
> > }
> >
> > -void
> > +NBDKIT_DLL_PUBLIC void
> > VixDiskLib_Exit (void)
> > {
> > /* Do nothing. */
> > }
> >
> > -char *
> > +NBDKIT_DLL_PUBLIC char *
> > VixDiskLib_GetErrorText (VixError err, const char *unused)
> > {
> > return strdup ("dummy-vddk: error message");
> > }
> >
> > -void
> > +NBDKIT_DLL_PUBLIC void
> > VixDiskLib_FreeErrorText (char *text)
> > {
> > free (text);
> > }
> >
> > -void
> > +NBDKIT_DLL_PUBLIC void
> > VixDiskLib_FreeConnectParams (VixDiskLibConnectParams *params)
> > {
> > /* never called since we don't define optional AllocateConnectParams */
> > abort ();
> > }
> >
> > -VixError
> > +NBDKIT_DLL_PUBLIC VixError
> > VixDiskLib_ConnectEx (const VixDiskLibConnectParams *params,
> > char read_only,
> > const char *snapshot_ref,
> > @@ -129,7 +131,7 @@ VixDiskLib_ConnectEx (const VixDiskLibConnectParams *params,
> > return VIX_OK;
> > }
> >
> > -VixError
> > +NBDKIT_DLL_PUBLIC VixError
> > VixDiskLib_Open (const VixDiskLibConnection connection,
> > const char *path,
> > uint32_t flags,
> > @@ -142,25 +144,25 @@ VixDiskLib_Open (const VixDiskLibConnection connection,
> > return VIX_OK;
> > }
> >
> > -const char *
> > +NBDKIT_DLL_PUBLIC const char *
> > VixDiskLib_GetTransportMode (VixDiskLibHandle handle)
> > {
> > return "file";
> > }
> >
> > -VixError
> > +NBDKIT_DLL_PUBLIC VixError
> > VixDiskLib_Close (VixDiskLibHandle handle)
> > {
> > return VIX_OK;
> > }
> >
> > -VixError
> > +NBDKIT_DLL_PUBLIC VixError
> > VixDiskLib_Disconnect (VixDiskLibConnection connection)
> > {
> > return VIX_OK;
> > }
> >
> > -VixError
> > +NBDKIT_DLL_PUBLIC VixError
> > VixDiskLib_GetInfo (VixDiskLibHandle handle,
> > VixDiskLibInfo **info)
> > {
> > @@ -169,13 +171,13 @@ VixDiskLib_GetInfo (VixDiskLibHandle handle,
> > return VIX_OK;
> > }
> >
> > -void
> > +NBDKIT_DLL_PUBLIC void
> > VixDiskLib_FreeInfo (VixDiskLibInfo *info)
> > {
> > free (info);
> > }
> >
> > -VixError
> > +NBDKIT_DLL_PUBLIC VixError
> > VixDiskLib_Read (VixDiskLibHandle handle,
> > uint64_t start_sector, uint64_t nr_sectors,
> > unsigned char *buf)
> > @@ -186,7 +188,7 @@ VixDiskLib_Read (VixDiskLibHandle handle,
> > return VIX_OK;
> > }
> >
> > -VixError
> > +NBDKIT_DLL_PUBLIC VixError
> > VixDiskLib_Write (VixDiskLibHandle handle,
> > uint64_t start_sector, uint64_t nr_sectors,
> > const unsigned char *buf)
> > --
> > 2.31.1
> >
> > _______________________________________________
> > Libguestfs mailing list
> > Libguestfs at redhat.com
> > https://listman.redhat.com/mailman/listinfo/libguestfs
> >
>
> Regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
More information about the Libguestfs
mailing list