Richard W.M. Jones
2018-Nov-14 09:53 UTC
[Libguestfs] [PATCH nbdkit v2 0/2] build: Replace ./nbdkit with a C program.
v1 was here: https://www.redhat.com/archives/libguestfs/2018-November/msg00147.html v2: - Use stdbool for booleans. - Use __attribute__((format(printf))). - Don't abort on invalid options, exit with failure instead. - Preserve long/short option choices in the output. - Add '=' in long option output, ie. always use --longopt=arg. - Add "--" parameter (unconditionally) before non-options. - Fix comments that referred to "alphanumeric" plugin and filter names. - Retested. Rich.
Richard W.M. Jones
2018-Nov-14 09:53 UTC
[Libguestfs] [PATCH nbdkit v2 1/2] 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 09:53 UTC
[Libguestfs] [PATCH nbdkit v2 2/2] 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.
---
Makefile.am | 11 ++-
README | 2 +-
configure.ac | 2 -
nbdkit.in | 160 -------------------------------
wrapper.c | 259 +++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 270 insertions(+), 164 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index d5ef59f..cd41132 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -46,7 +46,16 @@ 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)
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..f1be49f
--- /dev/null
+++ b/wrapper.c
@@ -0,0 +1,259 @@
+/* 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).
+ *------------------------------------------------------------
+ */
+
+#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[])
+{
+ int c;
+ int long_index;
+ bool verbose = false;
+ bool is_long_option;
+ size_t i;
+ 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 (;;) {
+ long_index = -1;
+ 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. However
+ * in nbdkit all short options have long equivalents so we can
+ * associate those with a long_index, but we must do it ourselves.
+ * https://stackoverflow.com/a/34070441
+ */
+ is_long_option = long_index >= 0;
+ if (long_index == -1) {
+ for (i = 0; long_options[i].name != NULL; ++i) {
+ if (long_options[i].val == c) {
+ long_index = i;
+ break;
+ }
+ }
+ if (long_index == -1)
+ abort ();
+ }
+
+ /* 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 (long_options[long_index].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 (long_options[long_index].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
Eric Blake
2018-Nov-14 13:58 UTC
Re: [Libguestfs] [PATCH nbdkit v2 2/2] build: Replace ./nbdkit with a C program.
On 11/14/18 3:53 AM, Richard W.M. Jones wrote:> 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. > --- > Makefile.am | 11 ++- > README | 2 +- > configure.ac | 2 - > nbdkit.in | 160 ------------------------------- > wrapper.c | 259 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 270 insertions(+), 164 deletions(-) >> + > + /* 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) {See my reply on the other thread about a potential followup for tree-wide consistency.> + /* Option parsing. We don't really parse options here. We are only > + * interested in which options have arguments and which need > + * rewriting. > + */ > + for (;;) { > + long_index = -1; > + 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. However > + * in nbdkit all short options have long equivalents so we can > + * associate those with a long_index, but we must do it ourselves. > + * https://stackoverflow.com/a/34070441 > + */ > + is_long_option = long_index >= 0; > + if (long_index == -1) { > + for (i = 0; long_options[i].name != NULL; ++i) { > + if (long_options[i].val == c) { > + long_index = i; > + break; > + } > + } > + if (long_index == -1) > + abort (); > + }I'm okay if you check the loop in as-is for readability sake, however, in my other thread I argued (and just now tested) that post-loop: assert(!long_options[long_index].has_arg == !optarg); will not kill the program, which means:> + /* Any short option. */ > + else { > + /* Short option which takes an argument. */ > + if (long_options[long_index].has_arg) {This can be written as 'if (optarg)', removing the dependence on .has_arg, and thus removing the need for the reverse-lookup loop.> + passthru_format ("-%c", c); > + passthru (optarg); > + } > + /* Short option which takes no argument. */ > + else > + passthru_format ("-%c", c); > + } > + } > +LGTM, whether or not you optimize out the loop. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Reasonably Related Threads
- [PATCH 0/2] build: Replace ./nbdkit with a C program.
- [PATCH nbdkit v3 0/4] 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.
- [libnbd PATCH 0/4] copy: wrap source code at 80 characters