Re: [Qemu-devel] [PATCH 05/10] qemu-iotests: define functions used in _cleanup() before its use

[Eric Blake <eblake@redhat.com>]

On 11/16/2017 11:38 AM, Cleber Rosa wrote:
> The functions used in _cleanup() come from common.rc, which currently
> gets sourced after _cleanup() is defined and registered as a signal
> handler.  When _cleanup() is executed, it has no valid references to
> those functions, as BASH won't resolve the reference at that time.
> 

Rather, there is a (small) window of time where if the test gets killed 
after the trap handler is registered but before the function definition 
is sourced, then the trap handler will fail to execute correctly 
(although if you kill the process that early, there was probably nothing 
that the handler would have cleaned up, so you are just littering extra 
messages to stderr).

Bash does NOT require a function to exist before a trap handler 
references it - only that the function exists at the time the trap 
handler body is invoked later.

> +++ b/tests/qemu-iotests/001
> @@ -27,15 +27,15 @@ echo "QA output created by $seq"
>   here=`pwd`
>   status=1	# failure is the default!
>   
> +# get standard environment, filters and checks
> +. ./common.rc
> +
>   _cleanup()
>   {
>   	_cleanup_test_img
>   }
>   trap "_cleanup; exit \$status" 0 1 2 3 15
>   
> -# get standard environment, filters and checks
> -. ./common.rc
> -

At any rate, I do think it reads better in this order, and consistency 
is nice.

With an improved commit message,
Reviewed-by: Eric Blake <eblake@redhat.com>

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