set_git_dir: handle feeding gitdir to itself

Ideally we'd free the existing gitdir field before assigning the new one, to avoid a memory leak. But we can't do so safely because some callers do the equivalent of: set_git_dir(get_git_dir()); We can detect that case as a noop, but there are even more complicated cases like: set_git_dir(remove_leading_path(worktree, get_git_dir()); where we really do need to do some work, but the original string must remain valid. Rather than put the burden on callers to make a copy of the string (only to free it later, since we'll make a copy of it ourselves), let's solve the problem inside set_git_dir(). We can make a copy of the pointer for the old gitdir, and then avoid freeing it until after we've made our new copy. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 5, 2017 at 09:05 UTC 1fb2b636c672fea06fdc5f50d5c0ed44117ae45a
2 files changed +3 -12
repository.c
+3 -7
@@ -56,16 +56,12 @@ static void repo_setup_env(struct repository *repo)
56 void repo_set_gitdir(struct repository *repo, const char *path)
57 {
58 const char *gitfile = read_gitfile(path);
59 + char *old_gitdir = repo->gitdir;
60
60 - /*
61 - * NEEDSWORK: Eventually we want to be able to free gitdir and the rest
62 - * of the environment before reinitializing it again, but we have some
63 - * crazy code paths where we try to set gitdir with the current gitdir
64 - * and we don't want to free gitdir before copying the passed in value.
65 - */
61 repo->gitdir = xstrdup(gitfile ? gitfile : path);
67 -
62 repo_setup_env(repo);
63 +
64 + free(old_gitdir);
65 }
66
67 /*
setup.c
-5
@@ -399,11 +399,6 @@ void setup_work_tree(void)
399 if (getenv(GIT_WORK_TREE_ENVIRONMENT))
400 setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
401
402 - /*
403 - * NEEDSWORK: this call can essentially be set_git_dir(get_git_dir())
404 - * which can cause some problems when trying to free the old value of
405 - * gitdir.
406 - */
402 set_git_dir(remove_leading_path(git_dir, work_tree));
403 initialized = 1;
404 }