All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Radim Krčmář" <rkrcmar@redhat.com>
To: kvm@vger.kernel.org
Cc: Paolo Bonzini <pbonzini@redhat.com>, Andrew Jones <drjones@redhat.com>
Subject: [PATCH kvm-unit-tests] scripts/mkstandalone: fix races on forced exits
Date: Thu, 21 Jan 2016 16:10:16 +0100	[thread overview]
Message-ID: <1453389016-9141-1-git-send-email-rkrcmar@redhat.com> (raw)

There are two bugs that happen if we exit after the first line of
following examples,

1) var=`mktemp`
   # we don't remove the temp file
   trap 'rm -f $var' EXIT

2) trap 'rm -f $cfg' EXIT
   # if $cfg is defined (= exported), we delete an unrelated file
   cfg=`mktemp`

Unit tests don't use cleanup variable anymore, but redefine EXIT trap
every time a new file is added.  This makes the standalone test slower,
but both scripts are less tangled now, IMO.  (Passing temp_file_vars as
an argument is nicer in theory, but doesn't look very good.)

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 I wanted to redefine the trap from the beginning, but it didn't pass
 sanity test before ... and Paolo also noticed a severe bug in the
 original series, so I probably wasn't testing in the right directory
 on both occasions :/

 scripts/mkstandalone.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 58910059a355..7d3455e66c99 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -19,8 +19,11 @@ temp_file ()
 	local var="$1"
 	local file="$2"
 
+	temp_file_vars+=" \$$var"
+
+	echo "unset $var"
+	echo "trap 'rm -f $temp_file_vars' EXIT"
 	echo "$var=\`mktemp\`"
-	echo "cleanup=\"\$$var \$cleanup\""
 	echo "base64 -d << 'BIN_EOF' | zcat > \$$var || exit 1"
 
 	gzip - < $file | base64
@@ -32,6 +35,7 @@ temp_file ()
 generate_test ()
 {
 	local args=( $(escape "${@}") )
+	local temp_file_vars
 
 	echo "#!/bin/bash"
 	grep '^ARCH=' config.mak
@@ -42,8 +46,6 @@ generate_test ()
 		return 1
 	fi
 
-	echo "trap 'rm -f \$cleanup' EXIT"
-
 	temp_file bin "$kernel"
 	args[3]='$bin'
 
@@ -76,6 +78,7 @@ function mkstandalone()
 	return 0
 }
 
+unset cfg
 trap 'rm -f $cfg' EXIT
 cfg=$(mktemp)
 
-- 
2.7.0


                 reply	other threads:[~2016-01-21 15:11 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1453389016-9141-1-git-send-email-rkrcmar@redhat.com \
    --to=rkrcmar@redhat.com \
    --cc=drjones@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.