Skip to content

Commit 268ef4d

Browse files
peffgitster
authored andcommitted
always quote shell arguments to test -z/-n
In shell code like: test -z $foo test -n $foo that does not quote its arguments, it's easy to think that it is actually looking at the contents of $foo in each case. But if $foo is empty, then "test" does not see any argument at all! The results are quite subtle. POSIX specifies that test's behavior depends on the number of arguments it sees, and if $foo is empty, it sees only one. The behavior in this case is: 1 argument: Exit true (0) if $1 is not null; otherwise, exit false. So in the "-z $foo" case, if $foo is empty, then we check that "-z" is non-null, and it returns success. Which happens to match what we expected. But for "-n $foo", if $foo is empty, we'll see that "-n" is non-null and still return success. That's the opposite of what we intended! Furthermore, if $foo contains whitespace, we'll end up with more than 2 arguments. The results in this case are generally unspecified (unless the first part of $foo happens to be a valid binary operator, in which case the results are specified but certainly not what we intended). And on top of this, even though "test -z $foo" _should_ work for the empty case, some older shells (reportedly ksh88) complain about the missing argument. So let's make sure we consistently quote our variable arguments to "test". After this patch, the results of: git grep 'test -[zn] [^"]' are empty. Reported-by: Armin Kunaschik <[email protected]> Helped-by: Junio C Hamano <[email protected]> Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 2a86cb6 commit 268ef4d

File tree

3 files changed

+5
-5
lines changed

3 files changed

+5
-5
lines changed

git-rebase--interactive.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -866,12 +866,12 @@ add_exec_commands () {
866866
# $3: the input filename
867867
check_commit_sha () {
868868
badsha=0
869-
if test -z $1
869+
if test -z "$1"
870870
then
871871
badsha=1
872872
else
873873
sha1_verif="$(git rev-parse --verify --quiet $1^{commit})"
874-
if test -z $sha1_verif
874+
if test -z "$sha1_verif"
875875
then
876876
badsha=1
877877
fi

git-stash.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ store_stash () {
185185

186186
git update-ref --create-reflog -m "$stash_msg" $ref_stash $w_commit
187187
ret=$?
188-
test $ret != 0 && test -z $quiet &&
188+
test $ret != 0 && test -z "$quiet" &&
189189
die "$(eval_gettext "Cannot update \$ref_stash with \$w_commit")"
190190
return $ret
191191
}
@@ -277,7 +277,7 @@ save_stash () {
277277
git clean --force --quiet -d $CLEAN_X_OPTION
278278
fi
279279

280-
if test "$keep_index" = "t" && test -n $i_tree
280+
if test "$keep_index" = "t" && test -n "$i_tree"
281281
then
282282
git read-tree --reset -u $i_tree
283283
fi

t/t4151-am-abort.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ test_expect_success 'am -3 --abort removes otherfile-4' '
8282
test 4 = "$(cat otherfile-4)" &&
8383
git am --abort &&
8484
test_cmp_rev initial HEAD &&
85-
test -z $(git ls-files -u) &&
85+
test -z "$(git ls-files -u)" &&
8686
test_path_is_missing otherfile-4
8787
'
8888

0 commit comments

Comments
 (0)