| Message ID | 1326492321-13540-3-git-send-email-ryanh@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 01/13/2012 03:05 PM, Ryan Harper wrote: > Create a cleanup function and call it from all exits so we don't leave > temp files and directories around since we change the name on each invocation. > > Also, no need to delete the files in the tmpdir, so just remove the tmpdir > if it exists. > > Signed-off-by: Ryan Harper <ryanh@us.ibm.com> > --- > qemu-test | 11 +++++++++-- > 1 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/qemu-test b/qemu-test > index cd102a7..71c1ba1 100755 > --- a/qemu-test > +++ b/qemu-test > @@ -1,7 +1,14 @@ > #!/bin/sh > > +cleanup() { > + if test -n "$tmpdir"; then > + rm -rf $tmpdir; > + fi > +} > + > if test -z "$1" -o -z "$2"; then > echo "Usage: $0 QEMU TEST1 [TEST2 ...]" > + cleanup > exit 1 Is it worth using 'trap cleanup 0' to install the cleanup handler up front, instead of modifying all exit call sites?
* Eric Blake <eblake@redhat.com> [2012-01-13 17:18]: > On 01/13/2012 03:05 PM, Ryan Harper wrote: > > Create a cleanup function and call it from all exits so we don't leave > > temp files and directories around since we change the name on each invocation. > > > > Also, no need to delete the files in the tmpdir, so just remove the tmpdir > > if it exists. > > > > Signed-off-by: Ryan Harper <ryanh@us.ibm.com> > > --- > > qemu-test | 11 +++++++++-- > > 1 files changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/qemu-test b/qemu-test > > index cd102a7..71c1ba1 100755 > > --- a/qemu-test > > +++ b/qemu-test > > @@ -1,7 +1,14 @@ > > #!/bin/sh > > > > +cleanup() { > > + if test -n "$tmpdir"; then > > + rm -rf $tmpdir; > > + fi > > +} > > + > > if test -z "$1" -o -z "$2"; then > > echo "Usage: $0 QEMU TEST1 [TEST2 ...]" > > + cleanup > > exit 1 > > Is it worth using 'trap cleanup 0' to install the cleanup handler up > front, instead of modifying all exit call sites? I thought about that, but it seemed to require switching to /bin/bash and I know Anthony had written the scripts carefully to be /bin/sh. > > -- > Eric Blake eblake@redhat.com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
On 01/16/2012 10:16 AM, Ryan Harper wrote: >>> if test -z "$1" -o -z "$2"; then >>> echo "Usage: $0 QEMU TEST1 [TEST2 ...]" >>> + cleanup >>> exit 1 >> >> Is it worth using 'trap cleanup 0' to install the cleanup handler up >> front, instead of modifying all exit call sites? > > I thought about that, but it seemed to require switching to /bin/bash Not really. > > and I know Anthony had written the scripts carefully to be /bin/sh. POSIX requires /bin/sh to support 'trap cleanup 0', and I don't know of any counter-example shells that fail to do this. There are non-POSIX shells where installing a trap 0 handler from inside a function body invokes the handler upon exiting the function, instead of exiting the overall script, but even Solaris /bin/sh knows how to correctly handle a trap 0 handler installed outside of any function calls. https://www.gnu.org/software/autoconf/manual/autoconf.html#trap
* Eric Blake <eblake@redhat.com> [2012-01-17 16:03]: > On 01/16/2012 10:16 AM, Ryan Harper wrote: > >>> if test -z "$1" -o -z "$2"; then > >>> echo "Usage: $0 QEMU TEST1 [TEST2 ...]" > >>> + cleanup > >>> exit 1 > >> > >> Is it worth using 'trap cleanup 0' to install the cleanup handler up > >> front, instead of modifying all exit call sites? > > > > I thought about that, but it seemed to require switching to /bin/bash > > Not really. > > > > > and I know Anthony had written the scripts carefully to be /bin/sh. > > POSIX requires /bin/sh to support 'trap cleanup 0', and I don't know of I was using trap cleanup SIGINT; which /bin/sh didn't like: (finalgravity) qemu-test % ./qemu-test ~/work/git/qemu/x86_64-softmmu/qemu-system-x86_64 tests/virtio-serial.sh trap: SIGINT: bad trap but with 0 instead, that seems to work. > any counter-example shells that fail to do this. There are non-POSIX > shells where installing a trap 0 handler from inside a function body > invokes the handler upon exiting the function, instead of exiting the > overall script, but even Solaris /bin/sh knows how to correctly handle a > trap 0 handler installed outside of any function calls. > > https://www.gnu.org/software/autoconf/manual/autoconf.html#trap > > -- > Eric Blake eblake@redhat.com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
diff --git a/qemu-test b/qemu-test index cd102a7..71c1ba1 100755 --- a/qemu-test +++ b/qemu-test @@ -1,7 +1,14 @@ #!/bin/sh +cleanup() { + if test -n "$tmpdir"; then + rm -rf $tmpdir; + fi +} + if test -z "$1" -o -z "$2"; then echo "Usage: $0 QEMU TEST1 [TEST2 ...]" + cleanup exit 1 fi @@ -23,6 +30,7 @@ if ! which qmp >/dev/null 2>/dev/null; then if ! test -x "${qmp}"; then echo "Please set QEMU_SRC to set to a recent qemu.git tree" + cleanup exit 1 fi else @@ -182,7 +190,6 @@ QEMU_TEST=1 . "$1" rc=$? -rm -f $tmplog $tmppid $tmpqmp $tmpinitrd -rm -rf $tmpdir $tmprc +cleanup exit $rc
Create a cleanup function and call it from all exits so we don't leave temp files and directories around since we change the name on each invocation. Also, no need to delete the files in the tmpdir, so just remove the tmpdir if it exists. Signed-off-by: Ryan Harper <ryanh@us.ibm.com> --- qemu-test | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)