[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Fix buglet in code searching for a good shell (tests/init.sh
From: |
Jim Meyering |
Subject: |
Re: [PATCH] Fix buglet in code searching for a good shell (tests/init.sh). |
Date: |
Fri, 11 Jun 2010 13:49:35 +0200 |
Stefano Lattarini wrote:
> At Friday 11 June 2010, Jim Meyering wrote:
>> > diff --git a/tests/init.sh b/tests/init.sh
>> > index ef0957c..4eb4a95 100644
>> > --- a/tests/init.sh
>> > +++ b/tests/init.sh
>> > @@ -97,12 +97,15 @@ else
>> > for re_shell_ in "${CONFIG_SHELL:-no_shell}" /bin/sh bash
>> > dash zsh pdksh fail do
>> > test "$re_shell_" = no_shell && continue
>> > - test "$re_shell_" = fail && skip_ failed to find an
>> > adequate shell + if test "$re_shell_" = fail; then
>> > + echo "$0: skipped test: failed to find an adequate
>> > shell" 1>&2 + exit 77
>>
>> Did you deliberately choose not to use the "(exit 77); exit 77"
>> idiom from Exit()? If so, why?
> I thought the '(exit ...); exit ...' idiom to be necessary only when
> an exit trap is in place, to ensure that the code in the trap is
> passed the correct exit status in $?. Did I miss something?
While not the recommended use case, some script may well
set up a trap prior to sourcing init.sh.
>> Did you consider simply hoisting the definitions
>> of Exit and/or ME_ to precede their first use?
> No, I just tried to fix the problem with the minimal amount of changes.
> Anyway, the absence of a decent shell and the failure of the exec call
> are very very unlikely situations, so IMHO it's not a big deal if the
> program name (`$0' instead of `$ME_') is suboptimal in the error
> messages associated with those situations.
Sure, but I prefer a more invasive change that keeps the code
(already rather obfuscated) a little cleaner.
Here's a patch that does that at the expense of using
shell functions *before* guaranteeing we have a decent shell.
I think that is ok, these days.
>From 0ae8d6338cc686b1fca0da26aefadebc0875cdf9 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 11 Jun 2010 13:41:31 +0200
Subject: [PATCH] init.sh: don't use $ME_ or skip_ before they are defined
* tests/init.sh: Hoist definitions of $ME_ and skip_ to precede
their first uses. Also hoist their companions: warn_, fail_,
framework_failure_, $stderr_fileno. Prompted by a patch from
Stefano Lattarini.
---
ChangeLog | 6 ++++++
tests/init.sh | 43 ++++++++++++++++++++++---------------------
2 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 726e66a..5f93588 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2010-06-11 Jim Meyering <address@hidden>
+ init.sh: don't use $ME_ or skip_ before they are defined
+ * tests/init.sh: Hoist definitions of $ME_ and skip_ to precede
+ their first uses. Also hoist their companions: warn_, fail_,
+ framework_failure_, $stderr_fileno. Prompted by a patch from
+ Stefano Lattarini.
+
test-sys_socket: avoid set-but-not-used warnings from gcc
* tests/test-sys_socket.c (main): Use "i" and "x", in order to
avoid warning about set-but-not-used variables.
diff --git a/tests/init.sh b/tests/init.sh
index ef0957c..286bbf1 100644
--- a/tests/init.sh
+++ b/tests/init.sh
@@ -57,6 +57,28 @@
# 4. Finally
# $ exit
+ME_=`expr "./$0" : '.*/\(.*\)$'`
+
+# We use a trap below for cleanup. This requires us to go through
+# hoops to get the right exit status transported through the handler.
+# So use `Exit STATUS' instead of `exit STATUS' inside of the tests.
+# Turn off errexit here so that we don't trip the bug with OSF1/Tru64
+# sh inside this function.
+Exit () { set +e; (exit $1); exit $1; }
+
+# Print warnings (e.g., about skipped and failed tests) to this file number.
+# Override by defining to say, 9, in init.cfg, and putting say,
+# "export ...ENVVAR_SETTINGS...; exec 9>&2; $(SHELL)" in the definition
+# of TESTS_ENVIRONMENT in your tests/Makefile.am file.
+# This is useful when using automake's parallel tests mode, to print
+# the reason for skip/failure to console, rather than to the .log files.
+: ${stderr_fileno_=2}
+
+warn_() { echo "$@" 1>&$stderr_fileno_; }
+fail_() { warn_ "$ME_: failed test: $@"; Exit 1; }
+skip_() { warn_ "$ME_: skipped test: $@"; Exit 77; }
+framework_failure_() { warn_ "$ME_: set-up failure: $@"; Exit 1; }
+
# We require $(...) support unconditionally.
# We require a few additional shell features only when $EXEEXT is nonempty,
# in order to support automatic $EXEEXT emulation:
@@ -118,26 +140,6 @@ test -n "$EXEEXT" && shopt -s expand_aliases
: ${MALLOC_PERTURB_=87}
export MALLOC_PERTURB_
-# We use a trap below for cleanup. This requires us to go through
-# hoops to get the right exit status transported through the handler.
-# So use `Exit STATUS' instead of `exit STATUS' inside of the tests.
-# Turn off errexit here so that we don't trip the bug with OSF1/Tru64
-# sh inside this function.
-Exit () { set +e; (exit $1); exit $1; }
-
-# Print warnings (e.g., about skipped and failed tests) to this file number.
-# Override by defining to say, 9, in init.cfg, and putting say,
-# "export ...ENVVAR_SETTINGS...; exec 9>&2; $(SHELL)" in the definition
-# of TESTS_ENVIRONMENT in your tests/Makefile.am file.
-# This is useful when using automake's parallel tests mode, to print
-# the reason for skip/failure to console, rather than to the .log files.
-: ${stderr_fileno_=2}
-
-warn_() { echo "$@" 1>&$stderr_fileno_; }
-fail_() { warn_ "$ME_: failed test: $@"; Exit 1; }
-skip_() { warn_ "$ME_: skipped test: $@"; Exit 77; }
-framework_failure_() { warn_ "$ME_: set-up failure: $@"; Exit 1; }
-
# This is a stub function that is run upon trap (upon regular exit and
# interrupt). Override it with a per-test function, e.g., to unmount
# a partition, or to undo any other global state changes.
@@ -247,7 +249,6 @@ setup_()
test "$VERBOSE" = yes && set -x
initial_cwd_=$PWD
- ME_=`expr "./$0" : '.*/\(.*\)$'`
pfx_=`testdir_prefix_`
test_dir_=`mktempd_ "$initial_cwd_" "$pfx_-$ME_.XXXX"` \
--
1.7.1.501.g23b46