[PATCH] tests: avoid subtle shell semantics bug

Jim Meyering jim at meyering.net
Fri May 13 08:01:18 UTC 2011


I wrote:
> Subject: [PATCH] tests: avoid subtle shell semantics bug
>
> This bash command will always exit successfully,
> i.e., the "return 1" will never happen:
>   bash -c 'f(){ local f=$(false)||return 1; return 0; }; f'
> That's because the assignment always succeeds, and that success
> is what determines the return value, not the $(...) command.
> This is very counter-intuitive, sigh.
> The work-around is to separate the declaration and assignment, as in
> local f; f=$(...) || ...
> * t/replication: Do that.
> * t/exercise (create_many): Likewise.
> ---
>  t/exercise    |    2 +-
>  t/replication |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/exercise b/t/exercise
...
> -  local n_procs=$(nproc || echo 2)
> +  local n_procs; n_procs=$(nproc || echo 2)
...

> diff --git a/t/replication b/t/replication
...
>  wait_for_repl() {
> -	local n_req=$(curl -d op=rep_status $api) || return 1
> +	local n_req; n_req=$(curl -d op=rep_status $api) || return 1


I was glad I hadn't pushed the above, yet.
While the latter use is indeed buggy, the one above is
actually just fine, since the "||" is inside the $(...).
Here's the adjusted patch that I'm about to push, now:


>From 45054a61e402d954be2348017a3d97ef74e095c8 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Mon, 9 May 2011 21:55:53 +0200
Subject: [PATCH] tests: avoid subtle shell semantics bug

This bash command will always exit successfully,
i.e., the "return 1" will never happen:
  bash -c 'f(){ local f=$(false)||return 1; return 0; }; f'
That's because the assignment always succeeds, and that success
is what determines the return value, not the $(...) command.
This is very counter-intuitive, sigh.
The work-around is to separate the declaration and assignment, as in
local f; f=$(...) || ...
* t/replication: Do that.
---
 t/replication |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/replication b/t/replication
index 30d761c..2a7a1ef 100644
--- a/t/replication
+++ b/t/replication
@@ -54,7 +54,7 @@ cleanup_() { kill -9 $mongo_pid; kill $iwhd_pid $iwhd_d_pid; }

 api=http://localhost:$port
 wait_for_repl() {
-	local n_req=$(curl -d op=rep_status $api) || return 1
+	local n_req; n_req=$(curl -d op=rep_status $api) || return 1
 	case $n_req in
 	  '0 requests') return 0 ;;
 	  *) return 1 ;;
--
1.7.5.1.398.g86d1d


More information about the iwhd-devel mailing list