From b0f1a413cd902b6c8ad9ed935ac58e67f5206c76 Mon Sep 17 00:00:00 2001 From: Andrey Zabavnikov Date: Fri, 28 Oct 2022 17:12:06 +0300 Subject: [PATCH] Fix status for old-style submodules with commondir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some setups, old-style submodules (i.e. the ones with .git directory within theirs worktrees) with commondir can be of tremendous help. For example, commondir link can be used to avoid duplication of objects and also to keep branches in sync with multiple copies of the repo's worktree, while keeping the .git directory inside the worktree can be (ab?-)used to exploit the sharing of the same submodule worktree across different projects (this at least works on Windows with submodule directory being a directory junction, but having a junction is not relevant for reproducing the bug described below). Unfortunately, at the moment, when `git status` is run in the root repo of such a setup, it gives an output akin to this: ```sh fatal: unable to access '�??\1?/config': Invalid argument fatal: 'git status --porcelain=2' failed in submodule commonlibs ``` where `�??\1?` part of '�??\1?/config' varies from run to run, and `commonlibs` is the name of submodule's directory. Currently, when Git discovers old-style submodule , it spawns subprocess to get its status, like this one: ```sh cd commonlibs; unset GIT_PREFIX; GIT_DIR=.git git status --porcelain=2 ``` Unsurprisingly, the following output is also quite unexpected: ``` fatal: unable to access '`??L&?/config': Invalid argument ``` The core reason for these is that global repository field for commondir is not being cleared to `NULL` after being `free()`'d in `repo_set_commondir()`, which is precisely what this commit fixes. Regarding the further details of the case of investigation, this value of struct pointed by the global `the_repository` pointer is checked for being not-NULL down in the callstack in compatibility layer for MinGW in a function that is called by `repo_set_commondir()` before the `free()`'d value gets assigned in its body (i.e. the body of `repo_set_commondir()`). Backtrace from the check is: #0 mingw_open (filename=0x ".git/commondir", oflags=0) at compat/mingw.c:784 #1 0x in strbuf_read_file (sb=0x, path=0x ".git/commondir", hint=0) at strbuf.c:758 #2 0x in get_common_dir_noenv (sb=0x, gitdir=0x ".git") at setup.c:313 #3 0x in repo_set_commondir (repo=0x , commondir=0x0) at repository.c:57 #4 0x in repo_set_gitdir (repo=0x , root=0x ".git", o=0x) at repository.c:76 #5 0x in setup_git_env (git_dir=0x ".git") at environment.c:179 #6 0x in set_git_dir_1 (path=0x ".git") at environment.c:334 #7 0x in update_relative_gitdir (name=0x0, old_cwd=0x "C:/Users/%username%//commonlibs", new_cwd=0x "C:/Users/%username%//commonlibs", data=0x0) at environment.c:348 #8 0x in chdir_notify ( new_cwd=0x "C:/Users/%username%//commonlibs") at chdir-notify.c:72 #9 0x in setup_work_tree () at setup.c:428 #10 0x in run_builtin (p=0x , argc=2, argv=0x) at git.c:458 #11 0x in handle_builtin (argc=2, argv=0x) at git.c:721 #12 0x in run_argv (argcp=0x, argv=0x) at git.c:788 #13 0x in cmd_main (argc=2, argv=0x) at git.c:921 #14 0x in main (argc=6, argv=0x) at common-main.c:56 Backtrace from the death is: #0 die_errno (fmt=0x "unable to access '%s'") at usage.c:210 #1 0x in access_or_die ( path=0x "`\001\r��\004/config", mode=4, flag=0) at wrapper.c:667 #2 0x in do_git_config_sequence (opts=0x, fn=0x , data=0x) at config.c:2142 #3 0x in config_with_options ( fn=0x , data=0x, config_source=0x0, opts=0x) at config.c:2198 #4 0x in repo_read_config (repo=0x ) at config.c:2524 #5 0x in git_config_check_init ( repo=0x ) at config.c:2543 #6 0x in repo_config_get_bool ( repo=0x , key=0x "windows.appendatomically", dest=0x ) at config.c:2612 #7 0x in git_config_get_bool ( key=0x "windows.appendatomically", dest=0x ) at config.c:2714 #8 0x in mingw_open ( filename=0x ".git/commondir", oflags=0) at compat/mingw.c:785 #9 0x in strbuf_read_file (sb=0x, path=0x ".git/commondir", hint=0) at strbuf.c:758 #10 0x in get_common_dir_noenv (sb=0x, gitdir=0x ".git") at setup.c:313 #11 0x in repo_set_commondir (repo=0x , commondir=0x0) at repository.c:57 #12 0x in repo_set_gitdir (repo=0x , root=0x ".git", o=0x) at repository.c:76 #13 0x in setup_git_env (git_dir=0x ".git") at environment.c:179 #14 0x in set_git_dir_1 (path=0x ".git") at environment.c:334 #15 0x in update_relative_gitdir (name=0x0, old_cwd=0x "C:/Users/%username%//commonlibs", new_cwd=0x "C:/Users/%username%//commonlibs", data=0x0) at environment.c:348 #16 0x in chdir_notify ( new_cwd=0x "C:/Users/%username%//commonlibs") at chdir-notify.c:72 #17 0x in setup_work_tree () at setup.c:428 #18 0x in run_builtin (p=0x , argc=2, argv=0x) at git.c:458 #19 0x in handle_builtin (argc=2, argv=0x) at git.c:721 #20 0x in run_argv (argcp=0x, argv=0x) at git.c:788 #21 0x in cmd_main (argc=2, argv=0x) at git.c:921 #22 0x in main (argc=6, argv=0x) at common-main.c:56 Signed-off-by: Andrey Zabavnikov --- repository.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repository.c b/repository.c index 5d166b692c8aa8..a2e1b89e14d979 100644 --- a/repository.c +++ b/repository.c @@ -46,7 +46,7 @@ static void repo_set_commondir(struct repository *repo, { struct strbuf sb = STRBUF_INIT; - free(repo->commondir); + FREE_AND_NULL(repo->commondir); if (commondir) { repo->different_commondir = 1;