[Libguestfs] [PATCH nbdkit 1/3] include: Function indirection for PE DLL

Richard W.M. Jones rjones at redhat.com
Mon Mar 23 12:48:08 UTC 2020


On Mon, Mar 23, 2020 at 12:28:10PM +0000, Richard W.M. Jones wrote:
> From: Yifan Gu <gyf304 at gmail.com>
> 
> This patch adds in indirection for API functions, as PE DLL loader
> does not resolve undefined symbols.
> 
> This patch only includes header changes.
> ---
>  include/nbdkit-common.h | 133 +++++++++++++++++++++++++++++++++++++++-
>  include/nbdkit-compat.h |  97 +++++++++++++++++++++++++++++
>  include/nbdkit-filter.h |  39 ++++++++++++
>  include/nbdkit-plugin.h |  27 ++++++++
>  include/Makefile.am     |   1 +
>  5 files changed, 295 insertions(+), 2 deletions(-)
> 
> diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h
> index 9d1d89d0..57abc423 100644
> --- a/include/nbdkit-common.h
> +++ b/include/nbdkit-common.h
> @@ -40,10 +40,14 @@
>  #include <stdarg.h>
>  #include <stdint.h>
>  #include <errno.h>
> -#include <sys/socket.h>
>  
> +#include <nbdkit-compat.h>
>  #include <nbdkit-version.h>
>  
> +#if defined(NBDKIT_INTERNAL) || !defined(WINDOWS_COMPAT)
> +#include <sys/socket.h>
> +#endif

This should be #ifdef HAVE_SYS_SOCKET_H  You'll need to add
it to the list of headers in configure.ac AC_CHECK_HEADERS.

> +#else
> +static void nbdkit_error (const char *msg, ...)
> +  ATTRIBUTE_FORMAT_PRINTF (1, 2);
> +static void nbdkit_error (const char *msg, ...)
> +{
> +  va_list args;
> +  va_start(args, msg);
> +  _nbdkit_functions.verror(msg, args);

Note that it's not legal in C to prefix file-scope functions with ‘_’.
So this should be just nbdkit_functions I think.  But there's another
problem with this struct which I'll come to.

> diff --git a/include/nbdkit-compat.h b/include/nbdkit-compat.h
> new file mode 100644
> index 00000000..a0a4cb69
> --- /dev/null
> +++ b/include/nbdkit-compat.h
> @@ -0,0 +1,97 @@
> +/* nbdkit

Although it may not be visible in this email, in git this file
contains CR-LF line endings.  Needs to be LF only.

> +#ifdef WINDOWS_COMPAT
> +struct nbdkit_functions {
> +  void (*verror) (const char *msg, va_list args);
> +  void (*vdebug) (const char *msg, va_list args);
> +  char * (*absolute_path) (const char *path);
> +  int64_t (*parse_size) (const char *str);
> +  int (*parse_bool) (const char *str);
> +  int (*parse_int) (const char *what, const char *str, int *r);
> +  int (*parse_unsigned) (const char *what, const char *str, unsigned *r);
> +  int (*parse_int8_t) (const char *what, const char *str, int8_t *r);
> +  int (*parse_uint8_t) (const char *what, const char *str, uint8_t *r);
> +  int (*parse_int16_t) (const char *what, const char *str, int16_t *r);
> +  int (*parse_uint16_t) (const char *what, const char *str, uint16_t *r);
> +  int (*parse_int32_t) (const char *what, const char *str, int32_t *r);
> +  int (*parse_uint32_t) (const char *what, const char *str, uint32_t *r);
> +  int (*parse_int64_t) (const char *what, const char *str, int64_t *r);
> +  int (*parse_uint64_t) (const char *what, const char *str, uint64_t *r);
> +  int (*read_password) (const char *value, char **password);
> +  char * (*realpath) (const char *path);
> +  int (*nanosleep) (unsigned sec, unsigned nsec);
> +  const char * (*export_name) (void);
> +  int (*peer_name) (void *addr, void *addrlen);
> +  void (*shutdown) (void);
> +  
> +  struct nbdkit_extents *(*extents_new) (uint64_t start, uint64_t end);
> +  void (*extents_free) (struct nbdkit_extents *);
> +  size_t (*extents_count) (const struct nbdkit_extents *);
> +  struct nbdkit_extent (*get_extent) (const struct nbdkit_extents *, size_t);
> +  int (*add_extent) (struct nbdkit_extents *, uint64_t offset, uint64_t length,
> +                    uint32_t type);
> +
> +  void (*set_error) (int err);
> +};
> +
> +struct nbdkit_functions _nbdkit_functions;

If I understand what's happening, the plugin has this struct, but it's
essentially private to the plugin (why isn't it static?)

The plugin also declares a functions_init public function which the
server calls.  Why did you define a new entry point for this instead
of having a new function in the regular plugin/filter struct?
Something that is called before load()?

The bigger problem I foresee is what happens if you mix old/new plugin
and old/new server versions and the contents of this struct changes?
It's likely that the server should tell the plugin how large it thinks
the struct is, so the plugin knows how much to copy and what fields to
set to defaults.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top




More information about the Libguestfs mailing list