Hi,

There is a comment about shell signal handlers in gnulib-tool saying that
"The value of $? is 128 + signal number and is set before the
trap-registered command is run".  Unfortunately, this comment is wrong,
and it seems to be a widespread misunderstanding.

The GNU Autoconf manual says that "it is widely admitted that when
entering the trap `$?' should be set to the exit status of the last
command run before the trap."

In case of signal handler, the exit status of the last command run
before the trap might be 128 + signal number, this usually happens when
the last command before the trap was a process terminated by signal.  In
other cases, the value of $? may be arbitrary.  Sometimes it's quite
hard to guess this value due to race conditions.  Here is an example of
such race condition where the value of $? takes one of 3 different
values: 0, 1 and 143:

$ for i in `seq 0 9`; do sh -c 'trap "exit \$?" TERM; while /bin/true; do 
/bin/false; done' & pid=$! && sleep 0.01 && kill -TERM -$pid && wait $pid; done
[1] 5122
Terminated
[1]+  Exit 143                sh -c 'trap "exit \$?" TERM; while /bin/true; do 
/bin/false; done'
[1] 5142
Terminated
[1]+  Exit 143                sh -c 'trap "exit \$?" TERM; while /bin/true; do 
/bin/false; done'
[1] 5165
[1]+  Exit 1                  sh -c 'trap "exit \$?" TERM; while /bin/true; do 
/bin/false; done'
[1] 5201
[1]+  Done                    sh -c 'trap "exit \$?" TERM; while /bin/true; do 
/bin/false; done'
[1] 5238
Terminated
[1]+  Exit 143                sh -c 'trap "exit \$?" TERM; while /bin/true; do 
/bin/false; done'
[1] 5271
[1]+  Exit 1                  sh -c 'trap "exit \$?" TERM; while /bin/true; do 
/bin/false; done'
[1] 5306
Terminated
[1]+  Exit 143                sh -c 'trap "exit \$?" TERM; while /bin/true; do 
/bin/false; done'
[1] 5341
Terminated
[1]+  Exit 143                sh -c 'trap "exit \$?" TERM; while /bin/true; do 
/bin/false; done'
[1] 5376
[1]+  Done                    sh -c 'trap "exit \$?" TERM; while /bin/true; do 
/bin/false; done'
[1] 5411
[1]+  Done                    sh -c 'trap "exit \$?" TERM; while /bin/true; do 
/bin/false; done'

Proposed patch is attached.  It doesn't fix multiple trap bugs in the
test suite, though:
$ grep -r '\<trap[[:space:]].*15' tests/
tests/uniwidth/test-uc_width2.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-vc-list-files-git.sh:trap '(exit $?); exit $?' 1 2 13 15
tests/test-yesno.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-vprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-fpending.sh:trap 'rm -fr $tmpfile' 1 2 3 15
tests/test-tsearch.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-xstrtoimax.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-perror.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-fprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-select-out.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-binary-io.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-vc-list-files-cvs.sh:trap '(exit $?); exit $?' 1 2 13 15
tests/test-xstrtol.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-dprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-xstrtoumax.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-printf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-atexit.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-select-in.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-xprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-closein.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-sigpipe.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-update-copyright.sh:trap 'rm -f $TMP_BASE*' 0 1 2 3 15
tests/test-lseek.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-vdprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-vfprintf-posix.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-c-stack2.sh:trap 'rm -fr $tmpfiles' 1 2 3 15
tests/test-c-stack.sh:trap 'rm -fr $tmpfiles' 1 2 3 15

What's wrong with these "trap 'rm -fr $tmpfiles' 1 2 3 15" lines?
This signal handler does no process termination thus leaving things in
an inconsistent state.  Proper fix would be to use functions from
tests/init.sh


-- 
ldv
From ad41213f91021fcb15c91d236ec2395da76ae121 Mon Sep 17 00:00:00 2001
From: Dmitry V. Levin <l...@altlinux.org>
Date: Sat, 30 Jan 2010 15:07:14 +0000
Subject: [PATCH] Fix exit status of signal handlers in shell scripts

The value of `$?' on entrance to signal handlers in shell scripts
cannot be relied upon, so set the exit code explicitly to
128 + SIGTERM == 143.
* build-aux/bootstrap (cleanup_gnulib): Add exit code argument.
Explicitly pass exit code to cleanup_gnulib().
* MODULES.html.sh: Use `func_exit 143' in signal handler.
* gnulib-tool: Likewise.
* build-aux/gnu-web-doc-update: Use `exit 143' in signal handler.
* lib/t-idcache: Likewise.
* tests/init.sh (setup_): Use `Exit 143' in signal handler.
---
 MODULES.html.sh              |    2 +-
 build-aux/bootstrap          |    7 +++----
 build-aux/gnu-web-doc-update |    2 +-
 gnulib-tool                  |    5 ++---
 lib/t-idcache                |    2 +-
 tests/init.sh                |    2 +-
 6 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/MODULES.html.sh b/MODULES.html.sh
index 4c8cefa..6f8bfa4 100755
--- a/MODULES.html.sh
+++ b/MODULES.html.sh
@@ -3357,7 +3357,7 @@ func_end HTML
 rm -rf "$tmp"
 # Undo the effect of the previous 'trap' command.
 trap '' 0
-trap 'func_exit $?' 1 2 3 13 15
+trap 'func_exit 143' 1 2 3 13 15
 
 exit 0
 
diff --git a/build-aux/bootstrap b/build-aux/bootstrap
index 0fb0ac5..6f5332f 100755
--- a/build-aux/bootstrap
+++ b/build-aux/bootstrap
@@ -389,9 +389,8 @@ fi
 
 
 cleanup_gnulib() {
-  status=$?
   rm -fr gnulib
-  exit $status
+  exit $1
 }
 
 git_modules_config () {
@@ -410,11 +409,11 @@ case ${GNULIB_SRCDIR--} in
   elif [ ! -d gnulib ]; then
     echo "$0: getting gnulib files..."
 
-    trap cleanup_gnulib 1 2 13 15
+    trap 'cleanup_gnulib 143' 1 2 13 15
 
     git clone --help|grep depth > /dev/null && shallow='--depth 2' || shallow=
     git clone $shallow git://git.sv.gnu.org/gnulib ||
-      cleanup_gnulib
+      cleanup_gnulib $?
 
     trap - 1 2 13 15
   fi
diff --git a/build-aux/gnu-web-doc-update b/build-aux/gnu-web-doc-update
index 2c1a0cc..97dcd68 100755
--- a/build-aux/gnu-web-doc-update
+++ b/build-aux/gnu-web-doc-update
@@ -84,7 +84,7 @@ cleanup()
   exit $__st
 }
 trap cleanup 0
-trap 'exit $?' 1 2 13 15
+trap 'exit 143' 1 2 13 15
 
 # We must build using sources for which --version reports the
 # just-released version number, not some string like 7.6.18-20761.
diff --git a/gnulib-tool b/gnulib-tool
index 90da140..a9adb8c 100755
--- a/gnulib-tool
+++ b/gnulib-tool
@@ -4845,10 +4845,9 @@ rm -rf "$tmp"
 # Undo the effect of the previous 'trap' command. Some shellology:
 # We cannot use "trap - 0 1 2 3 13 15", because Solaris sh would attempt to
 # execute the command "-". "trap '' ..." is fine only for signal 0 (= normal
-# exit); for the others we need to call 'exit' explicitly. The value of $? is
-# 128 + signal number and is set before the trap-registered command is run.
+# exit); for the others we need to call 'exit' explicitly.
 trap '' 0
-trap 'func_exit $?' 1 2 3 13 15
+trap 'func_exit 143' 1 2 3 13 15
 
 exit 0
 
diff --git a/lib/t-idcache b/lib/t-idcache
index e4d71af..8cbfd5e 100755
--- a/lib/t-idcache
+++ b/lib/t-idcache
@@ -5,7 +5,7 @@
 pwd=`pwd`
 t0=`echo "$0"|sed 's,.*/,,'`.tmp; tmp=$t0/$$
 trap 'status=$?; cd "$pwd" && chmod -R u+rwx $t0 && rm -rf $t0 && exit 
$status' 0
-trap '(exit $?); exit $?' 1 2 13 15
+trap '(exit 143); exit 143' 1 2 13 15
 
 srcdir=../..
 framework_failure=0
diff --git a/tests/init.sh b/tests/init.sh
index 979eb3c..38d9146 100644
--- a/tests/init.sh
+++ b/tests/init.sh
@@ -128,7 +128,7 @@ setup_()
   # This pair of trap statements ensures that the temporary directory,
   # $test_dir_, is removed upon exit as well as upon catchable signal.
   trap remove_tmp_ 0
-  trap 'Exit $?' 1 2 13 15
+  trap 'Exit 143' 1 2 13 15
 }
 
 # Create a temporary directory, much like mktemp -d does.
-- 
1.6.5.8

Attachment: pgpuGumLKZ6lt.pgp
Description: PGP signature

Reply via email to