Re: [Libguestfs] [nbdkit PATCH] tests: Better quoting for cleanup_fn

Testing with: === #!/bin/bash . tests/functions.sh cleanup_fn echo 'a b' cleanup_fn echo 'c d' === gives output: === a b c d === That is, our commands were munged by IFS splitting, because we stored commands in a flat variable. Fix it by instead using an array variable per cleanup_fn invocation. This does not fix the issue that commands are run in FIFO order; the comments in the recent test-nbd-client.sh mention that reverse order might be nicer, however, our existing kill_nbdkit() function assumes that it can call cleanup_fn during an existing cleanup function and that such cleanups will still be reached. Running cleanups in reverse order from the top level while still allowing multiple rounds of cleanup once cleanup is started is harder to achieve. --- If we like this, I'll apply the same patch to libnbd. Running the cleanups in reverse order is a tougher nut to crack; the nbdkit testsuite passed 'make check' when I did that (basically, swap the iteration of the for loop over _i in _run_cleanup_hooks), but that only tests the success paths, and it is the failure path of kill_nbdkit() where sane semantics are harder to guarantee with reverse ordering. tests/functions.sh.in | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/functions.sh.in b/tests/functions.sh.in index 6c5c481a..82f284a7 100644 --- a/tests/functions.sh.in +++ b/tests/functions.sh.in @@ -65,15 +65,18 @@ largest_qemu_disk=9223372035781033984 # # Run the command ‘cmd [args]’ when the test script exits. This is # run in all cases when the script exits, so is a reliable way to -# clean up test files, external processes etc. +# clean up test files, external processes etc. Cleanup hooks are run +# in the order of registration. # # Examples: # cleanup_fn rm -f test.out # cleanup_fn kill $pid -declare -a _cleanup_hook +_cleanup_hook_count=0 cleanup_fn () { - _cleanup_hook[${#_cleanup_hook[@]}]="$@" + local _hook=_cleanup_hook$((_cleanup_hook_count++)) + declare -ag $_hook + eval "$_hook=(\"\$@\")" } _run_cleanup_hooks () @@ -84,8 +87,9 @@ _run_cleanup_hooks () trap '' INT QUIT TERM EXIT ERR echo $0: run cleanup hooks: exit code $_status - for (( _i = 0; _i < ${#_cleanup_hook[@]}; ++_i )); do - ${_cleanup_hook[_i]} + for (( _i = 0; _i < $_cleanup_hook_count; ++_i )); do + local -n _hook=_cleanup_hook$_i + "${_hook[@]}" done exit $_status _______________________________________________ Libguestfs mailing list Libguestfs(a)redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs