thr3ads.net - Libguestfs - Re: [Libguestfs] [PATCH nbdkit v3 4/4] tests: Valgrind is only enabled when NBDKIT_VALGRIND = 1, not any other values. [Nov 2018]

If this information is useful, please help other people find it:
Share via:

Richard W.M. Jones

2018-Nov-14 14:27 UTC

[Libguestfs] [PATCH nbdkit v3 0/4] build: Replace ./nbdkit with a C program.

v1 was here:
https://www.redhat.com/archives/libguestfs/2018-November/msg00147.html

v2 was here:
https://www.redhat.com/archives/libguestfs/2018-November/msg00152.html

v3:

 - Use optarg != NULL as a sentinel for has_arg.

 - Moved some variable decls into the inner loop.

 - Make nbdkit wrapper depend on config.status, so if srcdir or
   builddir changes then we rebuild the wrapper.  It probably does so
   anyway because <config.h> will change, but let's be safe.

 - Document NBDKIT_VALGRIND=1 and NBDKIT_GDB=1.

 - Add two extra patches which fix existing uses of NBDKIT_VALGRIND.

 - Retested (make check && make check-valgrind).

Rich.

Richard W.M. Jones

2018-Nov-14 14:27 UTC

head link

[Libguestfs] [PATCH nbdkit v3 1/4] src: Move long_options short_options, is_short_name to a header file.

This refactoring is simply so the exact same options can be reused in
another program.
---
 src/Makefile.am |   1 +
 src/main.c      |  70 +------------------------------
 src/options.h   | 109 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+), 69 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index c094ed4..3490c0f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -46,6 +46,7 @@ nbdkit_SOURCES = \
 	log-stderr.c \
 	log-syslog.c \
 	main.c \
+	options.h \
 	plugins.c \
 	protocol.h \
 	sockets.c \
diff --git a/src/main.c b/src/main.c
index 224955b..0c7dbce 100644
--- a/src/main.c
+++ b/src/main.c
@@ -55,11 +55,11 @@
 #include <dlfcn.h>
 
 #include "internal.h"
+#include "options.h"
 #include "exit-with-parent.h"
 
 #define FIRST_SOCKET_ACTIVATION_FD 3 /* defined by systemd ABI */
 
-static int is_short_name (const char *);
 static char *make_random_fifo (void);
 static struct backend *open_plugin_so (size_t i, const char *filename, int
short_name);
 static struct backend *open_filter_so (struct backend *next, size_t i, const
char *filename, int short_name);
@@ -114,67 +114,6 @@ struct backend *backend;
 static char *random_fifo_dir = NULL;
 static char *random_fifo = NULL;
 
-enum {
-  HELP_OPTION = CHAR_MAX + 1,
-  DUMP_CONFIG_OPTION,
-  DUMP_PLUGIN_OPTION,
-  EXIT_WITH_PARENT_OPTION,
-  FILTER_OPTION,
-  LOG_OPTION,
-  LONG_OPTIONS_OPTION,
-  RUN_OPTION,
-  SELINUX_LABEL_OPTION,
-  SHORT_OPTIONS_OPTION,
-  TLS_OPTION,
-  TLS_CERTIFICATES_OPTION,
-  TLS_PSK_OPTION,
-  TLS_VERIFY_PEER_OPTION,
-};
-
-static const char *short_options = "D:e:fg:i:nop:P:rst:u:U:vV";
-static const struct option long_options[] = {
-  { "debug",            required_argument, NULL, 'D' },
-  { "dump-config",      no_argument,       NULL, DUMP_CONFIG_OPTION
},
-  { "dump-plugin",      no_argument,       NULL, DUMP_PLUGIN_OPTION
},
-  { "exit-with-parent", no_argument,       NULL,
EXIT_WITH_PARENT_OPTION },
-  { "export",           required_argument, NULL, 'e' },
-  { "export-name",      required_argument, NULL, 'e' },
-  { "exportname",       required_argument, NULL, 'e' },
-  { "filter",           required_argument, NULL, FILTER_OPTION },
-  { "foreground",       no_argument,       NULL, 'f' },
-  { "no-fork",          no_argument,       NULL, 'f' },
-  { "group",            required_argument, NULL, 'g' },
-  { "help",             no_argument,       NULL, HELP_OPTION },
-  { "ip-addr",          required_argument, NULL, 'i' },
-  { "ipaddr",           required_argument, NULL, 'i' },
-  { "log",              required_argument, NULL, LOG_OPTION },
-  { "long-options",     no_argument,       NULL, LONG_OPTIONS_OPTION
},
-  { "new-style",        no_argument,       NULL, 'n' },
-  { "newstyle",         no_argument,       NULL, 'n' },
-  { "old-style",        no_argument,       NULL, 'o' },
-  { "oldstyle",         no_argument,       NULL, 'o' },
-  { "pid-file",         required_argument, NULL, 'P' },
-  { "pidfile",          required_argument, NULL, 'P' },
-  { "port",             required_argument, NULL, 'p' },
-  { "read-only",        no_argument,       NULL, 'r' },
-  { "readonly",         no_argument,       NULL, 'r' },
-  { "run",              required_argument, NULL, RUN_OPTION },
-  { "selinux-label",    required_argument, NULL, SELINUX_LABEL_OPTION
},
-  { "short-options",    no_argument,       NULL, SHORT_OPTIONS_OPTION
},
-  { "single",           no_argument,       NULL, 's' },
-  { "stdin",            no_argument,       NULL, 's' },
-  { "threads",          required_argument, NULL, 't' },
-  { "tls",              required_argument, NULL, TLS_OPTION },
-  { "tls-certificates", required_argument, NULL,
TLS_CERTIFICATES_OPTION },
-  { "tls-psk",          required_argument, NULL, TLS_PSK_OPTION },
-  { "tls-verify-peer",  no_argument,       NULL,
TLS_VERIFY_PEER_OPTION },
-  { "unix",             required_argument, NULL, 'U' },
-  { "user",             required_argument, NULL, 'u' },
-  { "verbose",          no_argument,       NULL, 'v' },
-  { "version",          no_argument,       NULL, 'V' },
-  { NULL },
-};
-
 static void
 usage (void)
 {
@@ -748,13 +687,6 @@ main (int argc, char *argv[])
   exit (EXIT_SUCCESS);
 }
 
-/* Is it a plugin or filter name relative to the plugindir/filterdir? */
-static int
-is_short_name (const char *filename)
-{
-  return strchr (filename, '.') == NULL && strchr (filename,
'/') == NULL;
-}
-
 /* Implementation of '-U -' */
 static char *
 make_random_fifo (void)
diff --git a/src/options.h b/src/options.h
new file mode 100644
index 0000000..2bbc96e
--- /dev/null
+++ b/src/options.h
@@ -0,0 +1,109 @@
+/* nbdkit
+ * Copyright (C) 2013-2018 Red Hat Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Red Hat nor the names of its contributors may be
+ * used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#ifndef NBDKIT_OPTIONS_H
+#define NBDKIT_OPTIONS_H
+
+#include <getopt.h>
+#include <limits.h>
+#include <string.h>
+
+enum {
+  HELP_OPTION = CHAR_MAX + 1,
+  DUMP_CONFIG_OPTION,
+  DUMP_PLUGIN_OPTION,
+  EXIT_WITH_PARENT_OPTION,
+  FILTER_OPTION,
+  LOG_OPTION,
+  LONG_OPTIONS_OPTION,
+  RUN_OPTION,
+  SELINUX_LABEL_OPTION,
+  SHORT_OPTIONS_OPTION,
+  TLS_OPTION,
+  TLS_CERTIFICATES_OPTION,
+  TLS_PSK_OPTION,
+  TLS_VERIFY_PEER_OPTION,
+};
+
+static const char *short_options = "D:e:fg:i:nop:P:rst:u:U:vV";
+static const struct option long_options[] = {
+  { "debug",            required_argument, NULL, 'D' },
+  { "dump-config",      no_argument,       NULL, DUMP_CONFIG_OPTION
},
+  { "dump-plugin",      no_argument,       NULL, DUMP_PLUGIN_OPTION
},
+  { "exit-with-parent", no_argument,       NULL,
EXIT_WITH_PARENT_OPTION },
+  { "export",           required_argument, NULL, 'e' },
+  { "export-name",      required_argument, NULL, 'e' },
+  { "exportname",       required_argument, NULL, 'e' },
+  { "filter",           required_argument, NULL, FILTER_OPTION },
+  { "foreground",       no_argument,       NULL, 'f' },
+  { "no-fork",          no_argument,       NULL, 'f' },
+  { "group",            required_argument, NULL, 'g' },
+  { "help",             no_argument,       NULL, HELP_OPTION },
+  { "ip-addr",          required_argument, NULL, 'i' },
+  { "ipaddr",           required_argument, NULL, 'i' },
+  { "log",              required_argument, NULL, LOG_OPTION },
+  { "long-options",     no_argument,       NULL, LONG_OPTIONS_OPTION
},
+  { "new-style",        no_argument,       NULL, 'n' },
+  { "newstyle",         no_argument,       NULL, 'n' },
+  { "old-style",        no_argument,       NULL, 'o' },
+  { "oldstyle",         no_argument,       NULL, 'o' },
+  { "pid-file",         required_argument, NULL, 'P' },
+  { "pidfile",          required_argument, NULL, 'P' },
+  { "port",             required_argument, NULL, 'p' },
+  { "read-only",        no_argument,       NULL, 'r' },
+  { "readonly",         no_argument,       NULL, 'r' },
+  { "run",              required_argument, NULL, RUN_OPTION },
+  { "selinux-label",    required_argument, NULL, SELINUX_LABEL_OPTION
},
+  { "short-options",    no_argument,       NULL, SHORT_OPTIONS_OPTION
},
+  { "single",           no_argument,       NULL, 's' },
+  { "stdin",            no_argument,       NULL, 's' },
+  { "threads",          required_argument, NULL, 't' },
+  { "tls",              required_argument, NULL, TLS_OPTION },
+  { "tls-certificates", required_argument, NULL,
TLS_CERTIFICATES_OPTION },
+  { "tls-psk",          required_argument, NULL, TLS_PSK_OPTION },
+  { "tls-verify-peer",  no_argument,       NULL,
TLS_VERIFY_PEER_OPTION },
+  { "unix",             required_argument, NULL, 'U' },
+  { "user",             required_argument, NULL, 'u' },
+  { "verbose",          no_argument,       NULL, 'v' },
+  { "version",          no_argument,       NULL, 'V' },
+  { NULL },
+};
+
+/* Is it a plugin or filter name relative to the plugindir/filterdir? */
+static inline int
+is_short_name (const char *filename)
+{
+  return strchr (filename, '.') == NULL && strchr (filename,
'/') == NULL;
+}
+
+#endif /* NBDKIT_OPTIONS_H */
-- 
2.19.0.rc0

Richard W.M. Jones

2018-Nov-14 14:27 UTC

head link

[Libguestfs] [PATCH nbdkit v3 2/4] build: Replace ./nbdkit with a C program.

There are advantages to having the same code parse the options in the
./nbdkit wrapper as in the real nbdkit:

- We can parse options in exactly the same way as the real program.

- Use the more accurate ‘is_short_name’ test for unadorned
  plugin/filter names on the command line.

- Fixes the FreeBSD problem with shebangs caused because FreeBSD
  refuses to use a shell script as a shebang path.

Apart from the above, this is a straightforward translation of the
original shell script into C and preserves all the existing features
such as valgrind and gdb support.

Thanks: Eric Blake.
---
 Makefile.am  |  12 ++-
 README       |   2 +-
 configure.ac |   2 -
 nbdkit.in    | 160 --------------------------------
 wrapper.c    | 252 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 264 insertions(+), 164 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index d5ef59f..5abe18e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -46,7 +46,17 @@ EXTRA_DIST = \
 
 CLEANFILES += html/*.html
 
-noinst_SCRIPTS = nbdkit
+# NB: This is not the real nbdkit binary.  It's a wrapper that allows
+# you to run nbdkit from the build directory before it is installed.
+noinst_PROGRAMS = nbdkit
+nbdkit_SOURCES = wrapper.c src/options.h
+nbdkit_CPPFLAGS = \
+	-I$(top_srcdir)/src \
+	-Dbuilddir=\"$(abs_top_builddir)\" \
+	-Dsrcdir=\"$(abs_top_srcdir)\" \
+	-DVALGRIND=\"$(VALGRIND)\"
+nbdkit_CFLAGS = $(WARNINGS_CFLAGS)
+nbdkit_DEPENDENCIES = config.status
 
 SUBDIRS = \
 	bash \
diff --git a/README b/README
index 0b266f2..57583a3 100644
--- a/README
+++ b/README
@@ -154,7 +154,7 @@ Building
     make check                     make check
 
 To run nbdkit from the source directory, use the top level ./nbdkit
-script.  It will run nbdkit and plugins from the locally compiled
+wrapper.  It will run nbdkit and plugins from the locally compiled
 directory:
 
     $ ./nbdkit example1 -f -v
diff --git a/configure.ac b/configure.ac
index ed07177..5c2e951 100644
--- a/configure.ac
+++ b/configure.ac
@@ -736,8 +736,6 @@ AC_SUBST([filters])
 
 dnl Produce output files.
 AC_CONFIG_HEADERS([config.h])
-AC_CONFIG_FILES([nbdkit],
-                [chmod +x,-w nbdkit])
 AC_CONFIG_FILES([podwrapper.pl],
                 [chmod +x,-w podwrapper.pl])
 AC_CONFIG_FILES([Makefile
diff --git a/nbdkit.in b/nbdkit.in
deleted file mode 100644
index 5efbf75..0000000
--- a/nbdkit.in
+++ /dev/null
@@ -1,160 +0,0 @@
-#!/usr/bin/env bash
-# @configure_input@
-
-# Copyright (C) 2017-2018 Red Hat Inc.
-# All rights reserved.
-#
-# Redistribution and use in source and binary forms, with or without
-# modification, are permitted provided that the following conditions are
-# met:
-#
-# * Redistributions of source code must retain the above copyright
-# notice, this list of conditions and the following disclaimer.
-#
-# * Redistributions in binary form must reproduce the above copyright
-# notice, this list of conditions and the following disclaimer in the
-# documentation and/or other materials provided with the distribution.
-#
-# * Neither the name of Red Hat nor the names of its contributors may be
-# used to endorse or promote products derived from this software without
-# specific prior written permission.
-#
-# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
-# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
-# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
-# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
-# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
-# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
-# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
-# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
-# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
-# SUCH DAMAGE.
-
-#------------------------------------------------------------
-# This script lets you run nbdkit from the source directory.
-#
-# You can use either:
-#   ./nbdkit file [arg=value] [arg=value] ...
-# or:
-#   /path/to/nbdkit file [arg=value] [arg=value] ...
-#
-# Or you can set $PATH to include the nbdkit source directory and run
-# the bare "nbdkit" command without supplying the full path.
-#
-# The script modifies the bare plugin name (eg. "file") to be the
full
-# path to the locally compiled plugin.  If you don't use this script
-# and run src/nbdkit directly then it will pick up the installed
-# plugins which is not usually what you want.
-#
-# This script is also used to run the tests (make check).
-#------------------------------------------------------------
-
-# The location of the source and build directories.  Absolute paths
-# are used so this script can be called from any location.
-s="$(cd @abs_srcdir@ && pwd)"
-b="$(cd @abs_builddir@ && pwd)"
-
-# Rewrite the bare module name on the command line.
-declare -a args
-i=0
-done-verbose-
-while [ $# -gt 0 ]; do
-    case "$1" in
-        # Options that we special-case.  Unlike getopt_long with short option
-        # squashing, we only recognize -v in isolation. Oh well.
-        -v | --verbose)
-            verbose=1
-            args[$i]="$1"
-            ((++i))
-            shift
-            ;;
-
-        # Filters can be rewritten if purely alphanumeric.
-        --filter=*)
-            tmp=${1#*=}
-            shift
-            set - --filter "$tmp" "$@"
-            ;& # fallthru
-        --filter)
-            args[$i]="--filter"
-            ((++i))
-            [ $# -gt 1 ] || break
-            if [[ "$2" =~ ^[a-zA-Z0-9]+$ ]]; then
-                if [ -x "$b/filters/$2/.libs/nbdkit-$2-filter.so" ];
then
-                   
args[$i]="$b/filters/$2/.libs/nbdkit-$2-filter.so"
-                else
-                    args[$i]="$2"
-                fi
-            else
-                args[$i]="$2"
-            fi
-            ((++i))
-            shift 2
-            ;;
-
-        # Remaining options that take an argument, which we pass through as is.
-        # Although getopt_long handles abbreviations, we don't. Oh well.
-        --*=*)
-            args[$i]="$1"
-            ((++i))
-            shift
-            ;;
-        -D | --debug | -e | --export* | -g | --group | -i | --ip* | --log | \
-        -P | --pid* | -p | --port | --run | --selinux-label | -t | --threads |
\
-        --tls | --tls-certificates | --tls-psk | -U | --unix | -u | --user)
-            args[$i]="$1"
-            ((++i))
-            [ $# -gt 1 ] || break
-            args[$i]="$2"
-            ((++i))
-            shift 2
-            ;;
-
-        # Anything else can be rewritten if it's purely alphanumeric,
-        # but there is only one module name so only rewrite once.
-        *)
-            if [ ! $done ] && [[ "$1" =~ ^[a-zA-Z0-9]+$ ]];
then
-                # Usual plugins written in C.
-                if [ -x "$b/plugins/$1/.libs/nbdkit-$1-plugin.so" ];
then
-                   
args[$i]="$b/plugins/$1/.libs/nbdkit-$1-plugin.so"
-                    done=1
-                # Special plugins written in Perl.
-                elif [ "$1" = "example4" ] || [
"$1" = "tar" ]; then
-                   
args[$i]="$b/plugins/perl/.libs/nbdkit-perl-plugin.so"
-                    ((++i))
-                    args[$i]="$b/plugins/$1/nbdkit-$1-plugin"
-                    done=1
-                else
-                    args[$i]="$1"
-                fi
-            else
-                args[$i]="$1"
-            fi
-            ((++i))
-            shift
-            ;;
-    esac
-done
-
-# If -v flag was given on the command line, print the command.
-if [ $verbose ]; then
-    echo $b/src/nbdkit "${args[@]}"
-fi
-
-prefix-# If NBDKIT_VALGRIND=1 is set in the environment, then we run
-# the program under valgrind.  This is used by the tests.
-# Similarly, if NBDKIT_GDB=1 is set in the environment, we run
-# the program under gdb, useful during development.
-if [ "$NBDKIT_VALGRIND" ]; then
-    prefix="@VALGRIND@ --vgdb=no --leak-check=full --error-exitcode=119
--suppressions=$s/valgrind-suppressions --trace-children=no
--child-silent-after-fork=yes --run-libc-freeres=no --num-callers=20"
-elif [ "$NBDKIT_GDB" ]; then
-    prefix="gdb --args"
-fi
-
-# Run the final command.
-exec $prefix $b/src/nbdkit "${args[@]}"
diff --git a/wrapper.c b/wrapper.c
new file mode 100644
index 0000000..c253e9f
--- /dev/null
+++ b/wrapper.c
@@ -0,0 +1,252 @@
+/* nbdkit
+ * Copyright (C) 2017-2018 Red Hat Inc.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Red Hat nor the names of its contributors may be
+ * used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+/*------------------------------------------------------------
+ * This wrapper lets you run nbdkit from the source directory.
+ *
+ * You can use either:
+ * ./nbdkit file [arg=value] [arg=value] ...
+ * or:
+ *   /path/to/nbdkit file [arg=value] [arg=value] ...
+ *
+ * Or you can set $PATH to include the nbdkit source directory and run
+ * the bare "nbdkit" command without supplying the full path.
+ *
+ * The wrapper modifies the bare plugin name (eg. "file") to be the
+ * full path to the locally compiled plugin.  If you don't use this
+ * program and run src/nbdkit directly then it will pick up the
+ * installed plugins which is not usually what you want.
+ *
+ * This program is also used to run the tests (make check).
+ *
+ * You can enable valgrind by setting NBDKIT_VALGRIND=1 (this
+ * is mainly used by the internal tests).
+ *
+ * You can enable debugging by setting NBDKIT_GDB=1
+ *------------------------------------------------------------
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdarg.h>
+#include <string.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <limits.h>
+
+#include "options.h"
+
+/* Construct an array of parameters passed through to real nbdkit. */
+static const char **cmd;
+static size_t len;
+
+static void
+passthru (const char *s)
+{
+  cmd = realloc (cmd, (len+1) * sizeof (const char *));
+  if (cmd == NULL)
+    abort ();
+  cmd[len] = s;
+  ++len;
+}
+
+static void __attribute__((format (printf, 1, 2)))
+passthru_format (const char *fs, ...)
+{
+  va_list args;
+  char *str;
+
+  va_start (args, fs);
+  if (vasprintf (&str, fs, args) == -1)
+    abort ();
+  va_end (args);
+  passthru (str);
+}
+
+static void
+end_passthru (void)
+{
+  passthru (NULL);
+}
+
+static void
+print_command (void)
+{
+  size_t i;
+
+  if (len > 0)
+    fprintf (stderr, "%s", cmd[0]);
+  for (i = 1; i < len && cmd[i] != NULL; ++i)
+    fprintf (stderr, " %s", cmd[i]);
+  fprintf (stderr, "\n");
+}
+
+int
+main (int argc, char *argv[])
+{
+  bool verbose = false;
+  char *s;
+
+  /* If NBDKIT_VALGRIND=1 is set in the environment, then we run the
+   * program under valgrind.  This is used by the tests.  Similarly if
+   * NBDKIT_GDB=1 is set, we run the program under GDB, useful during
+   * development.
+   */
+  s = getenv ("NBDKIT_VALGRIND");
+  if (s && strcmp (s, "1") == 0) {
+    passthru (VALGRIND);
+    passthru ("--vgdb=no");
+    passthru ("--leak-check=full");
+    passthru ("--error-exitcode=119");
+    passthru_format ("--suppressions=%s/valgrind-suppressions",
srcdir);
+    passthru ("--trace-children=no");
+    passthru ("--child-silent-after-fork=yes");
+    passthru ("--run-libc-freeres=no");
+    passthru ("--num-callers=20");
+  }
+  else {
+    s = getenv ("NBDKIT_GDB");
+    if (s && strcmp (s, "1") == 0) {
+      passthru ("gdb");
+      passthru ("--args");
+    }
+  }
+
+  /* Absolute path of the real nbdkit command. */
+  passthru_format ("%s/src/nbdkit", builddir);
+
+  /* Option parsing.  We don't really parse options here.  We are only
+   * interested in which options have arguments and which need
+   * rewriting.
+   */
+  for (;;) {
+    int c;
+    int long_index = -1;
+    bool is_long_option, has_arg;
+
+    c = getopt_long (argc, argv, short_options, long_options, &long_index);
+    if (c == -1)
+      break;
+
+    if (c == '?')               /* getopt prints an error */
+      exit (EXIT_FAILURE);
+
+    /* long_index is only set if it's an actual long option. */
+    is_long_option = long_index >= 0;
+
+    /* If optarg != NULL then the option has an argument. */
+    has_arg = optarg != NULL;
+
+    /* Verbose is special because we will print the final command. */
+    if (c == 'v') {
+      verbose = true;
+      if (is_long_option)
+        passthru ("--verbose");
+      else
+        passthru ("-v");
+    }
+    /* Filters can be rewritten if they are a short name. */
+    else if (c == FILTER_OPTION) {
+      if (is_short_name (optarg))
+        passthru_format
("--filter=%s/filters/%s/.libs/nbdkit-%s-filter.so",
+                         builddir, optarg, optarg);
+      else
+        passthru_format ("--filter=%s", optarg);
+    }
+    /* Any long option. */
+    else if (is_long_option) {
+      /* Long option which takes an argument. */
+      if (has_arg)
+        passthru_format ("--%s=%s", long_options[long_index].name,
optarg);
+      /* Long option which takes no argument. */
+      else
+        passthru_format ("--%s", long_options[long_index].name);
+    }
+    /* Any short option. */
+    else {
+      /* Short option which takes an argument. */
+      if (has_arg) {
+        passthru_format ("-%c", c);
+        passthru (optarg);
+      }
+      /* Short option which takes no argument. */
+      else
+        passthru_format ("-%c", c);
+    }
+  }
+
+  /* Are there any non-option arguments? */
+  if (optind < argc) {
+    /* Ensure any further parameters can never be parsed as options by
+     * real nbdkit.
+     */
+    passthru ("--");
+
+    /* The first non-option argument is the plugin name.  If it is
+     * a short then rewrite it.
+     */
+    if (is_short_name (argv[optind])) {
+      /* Special plugins written in Perl. */
+      if (strcmp (argv[optind], "example4") == 0 ||
+          strcmp (argv[optind], "tar") == 0) {
+        passthru_format
("%s/plugins/perl/.libs/nbdkit-perl-plugin.so",
+                         builddir);
+        passthru_format ("%s/plugins/%s/nbdkit-%s-plugin",
+                         builddir, argv[optind], argv[optind]);
+      }
+      else {
+        passthru_format ("%s/plugins/%s/.libs/nbdkit-%s-plugin.so",
+                         builddir, argv[optind], argv[optind]);
+      }
+      ++optind;
+    }
+
+    /* Everything else is passed through without rewriting. */
+    while (optind < argc) {
+      passthru (argv[optind]);
+      ++optind;
+    }
+  }
+
+  end_passthru ();
+  if (verbose)
+    print_command ();
+
+  /* Run the final command. */
+  execvp (cmd[0], (char **) cmd);
+  perror (cmd[0]);
+  exit (EXIT_FAILURE);
+}
-- 
2.19.0.rc0

Richard W.M. Jones

2018-Nov-14 14:27 UTC

head link

[Libguestfs] [PATCH nbdkit v3 3/4] tests/test-lang-plugins.c: Free data and reuse variable.

---
 tests/test-lang-plugins.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/test-lang-plugins.c b/tests/test-lang-plugins.c
index 69471c4..46b0563 100644
--- a/tests/test-lang-plugins.c
+++ b/tests/test-lang-plugins.c
@@ -112,6 +112,7 @@ main (int argc, char *argv[])
              program_name, filename, data, content);
     exit (EXIT_FAILURE);
   }
+  free (data);
 
   /* Run sync to test flush path. */
   if (guestfs_sync (g) == -1)
@@ -130,8 +131,8 @@ main (int argc, char *argv[])
   if (guestfs_umount (g, "/") == -1)
     exit (EXIT_FAILURE);
   const char *cmd[] = { "fallocate", "-nzl",
"64k", "/dev/sda", NULL };
-  char *s = guestfs_debug (g, "sh", (char **) cmd);
-  free (s);
+  data = guestfs_debug (g, "sh", (char **) cmd);
+  free (data);
 
   if (guestfs_shutdown (g) == -1)
     exit (EXIT_FAILURE);
-- 
2.19.0.rc0

Richard W.M. Jones

2018-Nov-14 14:27 UTC

head link

[Libguestfs] [PATCH nbdkit v3 4/4] tests: Valgrind is only enabled when NBDKIT_VALGRIND = 1, not any other values.

Fix the tests so they only enable valgrind workarounds or skips when
NBDKIT_VALGRIND is exactly set to "1" and not any other values
(including empty string, or not set).

Thanks: Eric Blake.
---
 tests/test-dump-plugin.sh      | 3 ++-
 tests/test-ext2.c              | 4 +++-
 tests/test-help.sh             | 3 ++-
 tests/test-lang-plugins.c      | 4 +++-
 tests/test-ocaml.c             | 4 +++-
 tests/test-python-exception.sh | 2 +-
 tests/test-version.sh          | 3 ++-
 7 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/tests/test-dump-plugin.sh b/tests/test-dump-plugin.sh
index dc52742..6e0e3d7 100755
--- a/tests/test-dump-plugin.sh
+++ b/tests/test-dump-plugin.sh
@@ -55,7 +55,8 @@ fi
 # However some of these tests are expected to fail.
 test ()
 {
-    case "$1${NBDKIT_VALGRIND:+-valgrind}" in
+    [ "$NBDKIT_VALGRIND" = "1" ] &&
vg="-valgrind"
+    case "$1$vg" in
         vddk | vddk-valgrind)
             # VDDK won't run without special environment variables
             # being set, so ignore it.
diff --git a/tests/test-ext2.c b/tests/test-ext2.c
index 6878028..59e9281 100644
--- a/tests/test-ext2.c
+++ b/tests/test-ext2.c
@@ -49,12 +49,14 @@ main (int argc, char *argv[])
 {
   guestfs_h *g;
   int r;
+  const char *s;
   char *data;
 
   /* The ext2 test fails valgrind.  It seems as if the ext2fs error
    * table cannot be freed.
    */
-  if (getenv ("NBDKIT_VALGRIND") != NULL) {
+  s = getenv ("NBDKIT_VALGRIND");
+  if (s && strcmp (s, "1") == 0) {
     fprintf (stderr, "ext2 test skipped under valgrind.\n");
     exit (77);                  /* Tells automake to skip the test. */
   }
diff --git a/tests/test-help.sh b/tests/test-help.sh
index 68e0bfc..8dc0675 100755
--- a/tests/test-help.sh
+++ b/tests/test-help.sh
@@ -45,7 +45,8 @@ fi
 # However some of these tests are expected to fail.
 test ()
 {
-    case "$1${NBDKIT_VALGRIND:+-valgrind}" in
+    [ "$NBDKIT_VALGRIND" = "1" ] &&
vg="-valgrind"
+    case "$1$vg" in
         vddk | vddk-valgrind)
             # VDDK won't run without special environment variables
             # being set, so ignore it.
diff --git a/tests/test-lang-plugins.c b/tests/test-lang-plugins.c
index 46b0563..f569540 100644
--- a/tests/test-lang-plugins.c
+++ b/tests/test-lang-plugins.c
@@ -49,6 +49,7 @@ main (int argc, char *argv[])
 {
   guestfs_h *g;
   int r;
+  const char *s;
   char *data;
 
   /* These languages currently fail completely when run under
@@ -56,7 +57,8 @@ main (int argc, char *argv[])
    */
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-result"
-  if (getenv ("NBDKIT_VALGRIND") != NULL &&
+  s = getenv ("NBDKIT_VALGRIND");
+  if (s && strcmp (s, "1") == 0 &&
       (strcmp (LANG, "python") == 0 ||
        strcmp (LANG, "ruby") == 0 ||
        strcmp (LANG, "tcl") == 0)) {
diff --git a/tests/test-ocaml.c b/tests/test-ocaml.c
index 4cae8a6..fa61966 100644
--- a/tests/test-ocaml.c
+++ b/tests/test-ocaml.c
@@ -49,13 +49,15 @@ main (int argc, char *argv[])
 {
   guestfs_h *g;
   int r;
+  const char *s;
   char *data;
   size_t i, size;
 
   /* The OCaml tests fail valgrind, so skip them.  We should be able
    * to get this working with a bit of effort.  XXX
    */
-  if (getenv ("NBDKIT_VALGRIND") != NULL) {
+  s = getenv ("NBDKIT_VALGRIND");
+  if (s && strcmp (s, "1") == 0) {
     fprintf (stderr, "ocaml test skipped under valgrind.\n");
     exit (77);                  /* Tells automake to skip the test. */
   }
diff --git a/tests/test-python-exception.sh b/tests/test-python-exception.sh
index 567853a..7813918 100755
--- a/tests/test-python-exception.sh
+++ b/tests/test-python-exception.sh
@@ -36,7 +36,7 @@ set -x
 
 # Python language leaks like a sieve as well as a lot of worrying
 # "Conditional jump or move depends on uninitialised value(s)".
-if test -n "$NBDKIT_VALGRIND"; then
+if [ "$NBDKIT_VALGRIND" = "1" ]; then
     echo "$0: skipping Python test under valgrind."
     exit 77
 fi
diff --git a/tests/test-version.sh b/tests/test-version.sh
index 869d88c..a3b8a18 100755
--- a/tests/test-version.sh
+++ b/tests/test-version.sh
@@ -45,7 +45,8 @@ fi
 # However some of these tests are expected to fail.
 test ()
 {
-    case "$1${NBDKIT_VALGRIND:+-valgrind}" in
+    [ "$NBDKIT_VALGRIND" = "1" ] &&
vg="-valgrind"
+    case "$1$vg" in
         vddk | vddk-valgrind)
             # VDDK won't run without special environment variables
             # being set, so ignore it.
-- 
2.19.0.rc0

Eric Blake

2018-Nov-14 14:43 UTC

head link

Re: [Libguestfs] [PATCH nbdkit v3 4/4] tests: Valgrind is only enabled when NBDKIT_VALGRIND = 1, not any other values.

On 11/14/18 8:27 AM, Richard W.M. Jones wrote:> Fix the tests so they only enable valgrind workarounds or skips when
> NBDKIT_VALGRIND is exactly set to "1" and not any other values
> (including empty string, or not set).
> 
> Thanks: Eric Blake.
> ---
> +++ b/tests/test-dump-plugin.sh
> @@ -55,7 +55,8 @@ fi
>   # However some of these tests are expected to fail.
>   test ()
>   {
> -    case "$1${NBDKIT_VALGRIND:+-valgrind}" in
> +    [ "$NBDKIT_VALGRIND" = "1" ] &&
vg="-valgrind"
> +    case "$1$vg" in
Could misbehave if I have 'export vg=-valgrind' in my environment but 
not NBDKIT_VALGRIND (cure by prepping with 'vg=' before the [...]).  Not
sure if that is worth worrying about (someone playing that hard with 
their environment to try and break the testsuite deserves their problems).

Bash has sane rules even if I have NBDKIT_VALGRIND='!' or 
NBDKIT_VALGRIND='(', but not all other implementations do; the most 
common ways to ensure that user-supplied input can't cause an unexpected 
misparse of [ is to use either '[ "x$NBDKIT_VALGRIND" =
"x1" ]' or '[ 1
= "$NBDKIT_VALGRIND" ]'.  Not sure if that is worth worrying
about,
since the tests require bash.
> +++ b/tests/test-lang-plugins.c
> @@ -49,6 +49,7 @@ main (int argc, char *argv[])
>   {
>     guestfs_h *g;
>     int r;
> +  const char *s;
>     char *data;
>   
>     /* These languages currently fail completely when run under
> @@ -56,7 +57,8 @@ main (int argc, char *argv[])
>      */
>   #pragma GCC diagnostic push
>   #pragma GCC diagnostic ignored "-Wunused-result"
> -  if (getenv ("NBDKIT_VALGRIND") != NULL &&
> +  s = getenv ("NBDKIT_VALGRIND");
> +  if (s && strcmp (s, "1") == 0 &&
>         (strcmp (LANG, "python") == 0 ||
>          strcmp (LANG, "ruby") == 0 ||
>          strcmp (LANG, "tcl") == 0)) {
Do we still need the #pragma here?

Series is looking good now.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Possibly Parallel Threads

Search for more maybe matching threads

Libguestfs - Nov 2018 - Re: [PATCH nbdkit v3 4/4] tests: Valgrind is only enabled when NBDKIT_VALGRIND = 1, not any other values.

[Libguestfs] [PATCH nbdkit v3 0/4] build: Replace ./nbdkit with a C program.

[Libguestfs] [PATCH nbdkit v3 1/4] src: Move long_options short_options, is_short_name to a header file.

[Libguestfs] [PATCH nbdkit v3 2/4] build: Replace ./nbdkit with a C program.

[Libguestfs] [PATCH nbdkit v3 3/4] tests/test-lang-plugins.c: Free data and reuse variable.

[Libguestfs] [PATCH nbdkit v3 4/4] tests: Valgrind is only enabled when NBDKIT_VALGRIND = 1, not any other values.

Re: [Libguestfs] [PATCH nbdkit v3 4/4] tests: Valgrind is only enabled when NBDKIT_VALGRIND = 1, not any other values.

Possibly Parallel Threads