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
[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
[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
[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
[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
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" inCould 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
- [PATCH nbdkit v2 0/2] build: Replace ./nbdkit with a C program.
- [PATCH 0/2] build: Replace ./nbdkit with a C program.
- Re: [PATCH 2/2] build: Replace ./nbdkit with a C program.
- Re: [PATCH 2/2] build: Replace ./nbdkit with a C program.
- [PATCH nbdkit 0/4] Multiple valgrind improvements and possible security fix.