qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 05/10] qemu-iotests: define functions used in _c


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH 05/10] qemu-iotests: define functions used in _cleanup() before its use
Date: Fri, 1 Dec 2017 15:03:02 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0

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 <address@hidden>

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]