real_pathdup(): fix callsites that wanted it to die on error

In 4ac9006f832 (real_path: have callers use real_pathdup and
strbuf_realpath, 2016-12-12), we changed the xstrdup(real_path())
pattern to use real_pathdup() directly.

The problem with this change is that real_path() calls
strbuf_realpath() with die_on_error = 1 while real_pathdup() calls
it with die_on_error = 0. Meaning that in cases where real_path()
causes Git to die() with an error message, real_pathdup() is silent
and returns NULL instead.

The callers, however, are ill-prepared for that change, as they expect
the return value to be non-NULL (and otherwise the function died
with an appropriate error message).

Fix this by extending real_pathdup()'s signature to accept the
die_on_error flag and simply pass it through to strbuf_realpath(),
and then adjust all callers after a careful audit whether they would
handle NULLs well.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/abspath.c b/abspath.c
index 2f0c26e0e2cbee88aa671f3960d21720e4307aba..b02e068aa347db683b00246061b48bfafabbc1c0 100644 (file)
--- a/abspath.c
+++ b/abspath.c
@@ -214,12 +214,12 @@ const char *real_path_if_valid(const char *path)
        return strbuf_realpath(&realpath, path, 0);
 }
 
-char *real_pathdup(const char *path)
+char *real_pathdup(const char *path, int die_on_error)
 {
        struct strbuf realpath = STRBUF_INIT;
        char *retval = NULL;
 
-       if (strbuf_realpath(&realpath, path, 0))
+       if (strbuf_realpath(&realpath, path, die_on_error))
                retval = strbuf_detach(&realpath, NULL);
 
        strbuf_release(&realpath);

diff --git a/builtin/init-db.c b/builtin/init-db.c
index 1d4d6a0078919636dfc0ffefa2aec1699a9827f1..8a6acb0ec69330f7aae20d34fb5c5733f0058c67 100644 (file)
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -338,7 +338,7 @@ int init_db(const char *git_dir, const char *real_git_dir,
 {
        int reinit;
        int exist_ok = flags & INIT_DB_EXIST_OK;
-       char *original_git_dir = real_pathdup(git_dir);
+       char *original_git_dir = real_pathdup(git_dir, 1);
 
        if (real_git_dir) {
                struct stat st;
@@ -489,7 +489,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
        argc = parse_options(argc, argv, prefix, init_db_options, init_db_usage, 0);
 
        if (real_git_dir && !is_absolute_path(real_git_dir))
-               real_git_dir = real_pathdup(real_git_dir);
+               real_git_dir = real_pathdup(real_git_dir, 1);
 
        if (argc == 1) {
                int mkdir_tried = 0;
@@ -560,7 +560,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
                const char *git_dir_parent = strrchr(git_dir, '/');
                if (git_dir_parent) {
                        char *rel = xstrndup(git_dir, git_dir_parent - git_dir);
-                       git_work_tree_cfg = real_pathdup(rel);
+                       git_work_tree_cfg = real_pathdup(rel, 1);
                        free(rel);
                }
                if (!git_work_tree_cfg)

diff --git a/cache.h b/cache.h
index 61fc86e6d7199518555632a0b0b584471b9083a8..c26c45f899d2be6e1618d31881c6be9a212a9077 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -1109,7 +1109,7 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,
                      int die_on_error);
 const char *real_path(const char *path);
 const char *real_path_if_valid(const char *path);
-char *real_pathdup(const char *path);
+char *real_pathdup(const char *path, int die_on_error);
 const char *absolute_path(const char *path);
 char *absolute_pathdup(const char *path);
 const char *remove_leading_path(const char *in, const char *prefix);

diff --git a/dir.c b/dir.c
index 4541f9e1460cde7945e2443f2fee5c7df18073f5..aeeb5ce10490ef1e0adb907d1e21bcbbbbb401ac 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -2730,8 +2730,8 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)
 {
        struct strbuf file_name = STRBUF_INIT;
        struct strbuf rel_path = STRBUF_INIT;
-       char *git_dir = real_pathdup(git_dir_);
-       char *work_tree = real_pathdup(work_tree_);
+       char *git_dir = real_pathdup(git_dir_, 1);
+       char *work_tree = real_pathdup(work_tree_, 1);
 
        /* Update gitfile */
        strbuf_addf(&file_name, "%s/.git", work_tree);

diff --git a/environment.c b/environment.c
index c07fb17fb70bdb3cdf3ae9ab23196af9a0b7ad1b..42dc3106d2fae1976dcfdfbf264a1a9ed96a71b6 100644 (file)
--- a/environment.c
+++ b/environment.c
@@ -259,7 +259,7 @@ void set_git_work_tree(const char *new_work_tree)
                return;
        }
        git_work_tree_initialized = 1;
-       work_tree = real_pathdup(new_work_tree);
+       work_tree = real_pathdup(new_work_tree, 1);
 }
 
 const char *get_git_work_tree(void)

diff --git a/setup.c b/setup.c
index 967f289f1ef07d78f4b680e1d880e2fa86215371..6b48cb91ff2bdde1d02b2696b5b38a64cc663161 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -698,7 +698,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,
        /* --work-tree is set without --git-dir; use discovered one */
        if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {
                if (offset != cwd->len && !is_absolute_path(gitdir))
-                       gitdir = real_pathdup(gitdir);
+                       gitdir = real_pathdup(gitdir, 1);
                if (chdir(cwd->buf))
                        die_errno("Could not come back to cwd");
                return setup_explicit_git_dir(gitdir, cwd, nongit_ok);
@@ -806,7 +806,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
                /* Keep entry but do not canonicalize it */
                return 1;
        } else {
-               char *real_path = real_pathdup(ceil);
+               char *real_path = real_pathdup(ceil, 0);
                if (!real_path) {
                        return 0;
                }

diff --git a/submodule.c b/submodule.c
index 3b98766a6bcfe983e5f94e84edd6fcec7ce74f8d..0a2831d846d27e694ada4bc557a7d54a8bf2d8df 100644 (file)
--- a/submodule.c
+++ b/submodule.c
@@ -1403,7 +1403,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,
                /* If it is an actual gitfile, it doesn't need migration. */
                return;
 
-       real_old_git_dir = real_pathdup(old_git_dir);
+       real_old_git_dir = real_pathdup(old_git_dir, 1);
 
        sub = submodule_from_path(null_sha1, path);
        if (!sub)
@@ -1412,7 +1412,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,
        new_git_dir = git_path("modules/%s", sub->name);
        if (safe_create_leading_directories_const(new_git_dir) < 0)
                die(_("could not create directory '%s'"), new_git_dir);
-       real_new_git_dir = real_pathdup(new_git_dir);
+       real_new_git_dir = real_pathdup(new_git_dir, 1);
 
        if (!prefix)
                prefix = get_super_prefix();
@@ -1472,14 +1472,14 @@ void absorb_git_dir_into_superproject(const char *prefix,
                new_git_dir = git_path("modules/%s", sub->name);
                if (safe_create_leading_directories_const(new_git_dir) < 0)
                        die(_("could not create directory '%s'"), new_git_dir);
-               real_new_git_dir = real_pathdup(new_git_dir);
+               real_new_git_dir = real_pathdup(new_git_dir, 1);
                connect_work_tree_and_git_dir(path, real_new_git_dir);
 
                free(real_new_git_dir);
        } else {
                /* Is it already absorbed into the superprojects git dir? */
-               char *real_sub_git_dir = real_pathdup(sub_git_dir);
-               char *real_common_git_dir = real_pathdup(get_git_common_dir());
+               char *real_sub_git_dir = real_pathdup(sub_git_dir, 1);
+               char *real_common_git_dir = real_pathdup(get_git_common_dir(), 1);
 
                if (!starts_with(real_sub_git_dir, real_common_git_dir))
                        relocate_single_git_dir_into_superproject(prefix, path);

diff --git a/t/t1501-work-tree.sh b/t/t1501-work-tree.sh
index 046d9b7909f468b3e9c604641d3088e687da6db0..b06210ec5e8ffa30b59314b1c79ebc9bdb8f6476 100755 (executable)
--- a/t/t1501-work-tree.sh
+++ b/t/t1501-work-tree.sh
@@ -423,7 +423,7 @@ test_expect_success '$GIT_WORK_TREE overrides $GIT_DIR/common' '
        )
 '
 
-test_expect_failure 'error out gracefully on invalid $GIT_WORK_TREE' '
+test_expect_success 'error out gracefully on invalid $GIT_WORK_TREE' '
        (
                GIT_WORK_TREE=/.invalid/work/tree &&
                export GIT_WORK_TREE &&

diff --git a/worktree.c b/worktree.c
index d633761575bff4811fa4f3f3822807b1f77f5ecb..0486e31ad4a5f5ee02813d06bf9a48e48e3667cb 100644 (file)
--- a/worktree.c
+++ b/worktree.c
@@ -255,7 +255,7 @@ struct worktree *find_worktree(struct worktree **list,
                return wt;
 
        arg = prefix_filename(prefix, strlen(prefix), arg);
-       path = real_pathdup(arg);
+       path = real_pathdup(arg, 1);
        for (; *list; list++)
                if (!fspathcmp(path, real_path((*list)->path)))
                        break;