[Libguestfs] [nbdkit PATCH v2 1/5] maint: Improve ./nbdkit option parsing

[Eric Blake]

We had several poor option-parsing actions in our ./nbdkit wrapper:

Attempting './nbdkit --filter' went into an infloop with growing
memory, because bash treats 'shift 2' when $# as a soft error (which
we ignored) without even shifting 1, such that $# never decreases
but $args[] continues to grow (dash, on the other hand, follows the
POSIX recommendation of a hard error with immediate script termination
in a non-interactive shell).  Fix by checking that $# is large enough
before attempting to shift 2.

Commit 091ce402 tried to make './nbdkit --export=foo' work the
same as './nbdkit --export foo' - but has no testsuite coverage,
and silently eats the option instead.  Fix by incrementing i in
that case label.

Attempting './nbdkit --ipaddr=$addr plugin' doesn't special case
'plugin' properly, because we forgot to special-case '--ip*=*'
similarly to --export and --pid.  Fix by generalizing the early
glob to catch ALL longopts used with = rather than just a select
few, but only after we have handled the special-casing of
--filter[=] first (the biggest part of this patch is thus code
motion).

We forgot to add -D/--debug and --log from recent command line
additions.  Fix by adding more patterns; and split the long line
while at it.

Not fixed: the real nbdkit allows things like 'nbdkit -thr 1 ...'
while we don't; it's not worth teaching our debug wrapper about
unambiguous abbreviations.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 nbdkit.in | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/nbdkit.in b/nbdkit.in
index 0469f1a..a6318a4 100644
--- a/nbdkit.in
+++ b/nbdkit.in
@@ -64,19 +64,7 @@ verbose=

 while [ $# -gt 0 ]; do
     case "$1" in
-        # Flags that take an argument.  We must not rewrite the argument.
-        # But make sure globs don't mishandle longopts with =.
-        --export*=* | --pid*=*)
-            args[$i]="$1"
-            shift
-            ;;
-        -e | --export* | -g | --group | -i | --ip* | -P | --pid* | -p | --port | --run | --selinux-label | -t | --threads | --tls | --tls-certificates | --tls-psk | -U | --unix | -u | --user)
-            args[$i]="$1"
-            ((++i))
-            args[$i]="$2"
-            ((++i))
-            shift 2
-            ;;
+	# Options that we special-case.
         -v | --verbose)
             verbose=1
             args[$i]="$1"
@@ -93,6 +81,7 @@ while [ $# -gt 0 ]; do
         --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"
@@ -106,6 +95,24 @@ while [ $# -gt 0 ]; do
             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.
         *)
-- 
2.17.2