[PATCH 1/9] worktree: don't die() in library function find_worktree()

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 28, 2018, 21:20 UTC
Message-ID: <20180828212026.21989-2-sunshine@sunshineco.com>
In-Reply-To: <20180828212026.21989-1-sunshine@sunshineco.com>

Callers don't expect library function find_worktree() to die(); they expect it to return the named worktree if found, or NULL if not. Although find_worktree() itself never invokes die(), it calls real_pathdup() with 'die_on_error' incorrectly set to 'true', thus will die() indirectly if the user-provided path is not to real_pathdup()'s liking. This can be observed, for instance, with any git-worktree command which searches for an existing worktree:

    $ git worktree unlock foo
    fatal: 'foo' is not a working tree
    $ git worktree unlock foo/bar
    fatal: Invalid path '.../foo': No such file or directory

The first error message is the expected one from "git worktree unlock" not finding the specified worktree; the second is from find_worktree() invoking real_pathdup() incorrectly and die()ing prematurely.

Aside from the inconsistent error message between the two cases, this bug hasn't otherwise been a serious problem since existing callers all die() anyhow when the worktree can't be found. However, that may not be true of callers added in the future, so fix find_worktree() to avoid die()ing.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/t2028-worktree-move.sh | 8 ++++++++
 worktree.c               | 6 +++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 5f7d45b7b7..60aba7c41a 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -141,4 +141,12 @@ test_expect_success 'NOT remove missing-but-locked worktree' '
 	test_path_is_dir .git/worktrees/gone-but-locked
 '
 
+test_expect_success 'proper error when worktree not found' '
+	for i in noodle noodle/bork
+	do
+		test_must_fail git worktree lock $i 2>err &&
+		test_i18ngrep "not a working tree" err || return 1
+	done
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index 97cda5f97b..b0d0b5426d 100644
--- a/worktree.c
+++ b/worktree.c
@@ -217,7 +217,11 @@ struct worktree *find_worktree(struct worktree **list,
 
 	if (prefix)
 		arg = to_free = prefix_filename(prefix, arg);
-	path = real_pathdup(arg, 1);
+	path = real_pathdup(arg, 0);
+	if (!path) {
+		free(to_free);
+		return NULL;
+	}
 	for (; *list; list++)
 		if (!fspathcmp(path, real_path((*list)->path)))
 			break;

-- 
2.19.0.rc1.350.ge57e33dbd1