From fbe541727e551c7ed3601eaf661ee8724d3a1256 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 28 Sep 2026 11:51:02 +0200 Subject: [PATCH 1/7] path: drop useless `safe_create_leading_directories_1()` The function `safe_create_leading_directories_1()` is being called by both `safe_create_leading_directories()` and its `_no_share()` variant. It is ultimately the exact same as the former of these functions though and is thus quite useless. Drop the function and inline it into its callsites directly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- path.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/path.c b/path.c index c3a709a928..69b06c9464 100644 --- a/path.c +++ b/path.c @@ -829,8 +829,8 @@ int safe_create_dir_in_gitdir(struct repository *repo, const char *path) return adjust_shared_perm(repo, path); } -static enum scld_error safe_create_leading_directories_1(struct repository *repo, - char *path) +enum scld_error safe_create_leading_directories(struct repository *repo, + char *path) { char *next_component = path + offset_1st_component(path); enum scld_error ret = SCLD_OK; @@ -884,15 +884,9 @@ static enum scld_error safe_create_leading_directories_1(struct repository *repo return ret; } -enum scld_error safe_create_leading_directories(struct repository *repo, - char *path) -{ - return safe_create_leading_directories_1(repo, path); -} - enum scld_error safe_create_leading_directories_no_share(char *path) { - return safe_create_leading_directories_1(NULL, path); + return safe_create_leading_directories(NULL, path); } enum scld_error safe_create_leading_directories_const(struct repository *repo, From b428ea54ef89719b19b3ca877cc708e664a7c630 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 28 Sep 2026 11:51:03 +0200 Subject: [PATCH 2/7] path: introduce `safe_create_leading_directories_no_share_const()` The `safe_create_leading_directories()` family of functions modify the passed-in path so that we can obtain all the different segments of the path. This is done by overwriting path separators with a NUL byte for every component. While we ultimately restore the original string, the consequence is that the caller needs to pass a non-constant string. While it would be trivial to modify the function to not modify the path in-place anymore, the intent of this whole mechanism is to save an allocation. It's quite dubious whether this optimization really matters in the grand scheme of things, but here we are. In any case, we provide a `_const()` variant that handles the case where the caller only has a string constant. But we lack such a variant for the `safe_create_leading_directories_no_share()` function, and we're about to add a couple of callers that would need it. Add this helper function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- path.c | 5 +++++ path.h | 14 ++++++-------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/path.c b/path.c index 69b06c9464..f8f5a9dd28 100644 --- a/path.c +++ b/path.c @@ -889,6 +889,11 @@ enum scld_error safe_create_leading_directories_no_share(char *path) return safe_create_leading_directories(NULL, path); } +enum scld_error safe_create_leading_directories_no_share_const(const char *path) +{ + return safe_create_leading_directories_const(NULL, path); +} + enum scld_error safe_create_leading_directories_const(struct repository *repo, const char *path) { diff --git a/path.h b/path.h index 7e7408dd05..922bd6e377 100644 --- a/path.h +++ b/path.h @@ -234,14 +234,11 @@ int safe_create_dir_in_gitdir(struct repository *repo, const char *path); * race, callers might want to try invoking the function again when it * returns SCLD_VANISHED. * - * safe_create_leading_directories() temporarily changes path while it - * is working but restores it before returning. - * safe_create_leading_directories_const() doesn't modify path, even - * temporarily. Both these variants adjust the permissions of the - * created directories to honor core.sharedRepository, so they are best - * suited for files inside the git dir. For working tree files, use - * safe_create_leading_directories_no_share() instead, as it ignores - * the core.sharedRepository setting. + * The default variants honor "core.sharedRepository" and temporarily modify + * `path`. Note that this configuration should be honored for all files in the + * git directory. The `no_share()` variants ignore "core.sharedRepository", + * and should be used for working tree files. The `const()` variants do not + * modify `path`. */ enum scld_error { SCLD_OK = 0, @@ -254,6 +251,7 @@ enum scld_error safe_create_leading_directories(struct repository *repo, char *p enum scld_error safe_create_leading_directories_const(struct repository *repo, const char *path); enum scld_error safe_create_leading_directories_no_share(char *path); +enum scld_error safe_create_leading_directories_no_share_const(const char *path); /* * Create a file, potentially creating its leading directories in case they From a6d5ea281e756a174328c21da2bc68a1a7491c8c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 28 Sep 2026 11:51:04 +0200 Subject: [PATCH 3/7] builtin/init: refactor messy creation of leading directories When creating a new repository via git-init(1) we potentially have to create any leading directories via `safe_create_leading_directories()`. This function optionally knows to handle "core.sharedRepository" to adjust the permissions of the created directories. The value of that setting is taken from the passed-in repository. When creating a new repository we don't want to honor it though, so we painstakingly: 1. Save the current value of that setting. 2. Set it to 0. 3. Create the directory with `safe_create_leading_directories()`. This has the effect that `adjust_shared_perm()` will exit early and not adjust permissions. 4. Restore the old value. This is extremely awkward, but it achieves the desired effect that we ignore the configuration. There's a significantly easier way to achieve this though: we can just call the `_no_share()` variant, whose entire purpose it is to ignore "core.sharedRepository". Refactor the code to use that variant accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/init-db.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/builtin/init-db.c b/builtin/init-db.c index 5c22eae2f3..e45268f1ff 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -131,15 +131,7 @@ int cmd_init_db(int argc, retry: if (chdir(argv[0]) < 0) { if (!mkdir_tried) { - int saved; - /* - * At this point we haven't read any configuration, - * and we know shared_repository should always be 0; - * but just in case we play safe. - */ - saved = repo_settings_get_shared_repository(the_repository); - repo_settings_set_shared_repository(the_repository, 0); - switch (safe_create_leading_directories_const(the_repository, argv[0])) { + switch (safe_create_leading_directories_no_share_const(argv[0])) { case SCLD_OK: case SCLD_PERMS: break; @@ -150,7 +142,7 @@ int cmd_init_db(int argc, die_errno(_("cannot mkdir %s"), argv[0]); break; } - repo_settings_set_shared_repository(the_repository, saved); + if (mkdir(argv[0], 0777) < 0) die_errno(_("cannot mkdir %s"), argv[0]); mkdir_tried = 1; From 8f17ac5de4e139c9edb02a3f1473aa5f7e1757f7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 28 Sep 2026 11:51:05 +0200 Subject: [PATCH 4/7] builtin/init: move handling of "core.sharedRepository" into "setup.c" When initializing a new repository via git-init(1) we know to honor "core.sharedRepository" and adjust permissions of newly created files accordingly. The way we propagate that setting is quite awkward though, as we have to set it on the repository that we pass into `create_repository()` and pass it as a parameter. This is because there are two different scopes in play here: - We need to apply it to the repository so that creating the repository's directory uses the correct permissions. - We need to reapply it to the repository after we have created default files so that we know to override any configuration that we have read from the new repository's configuration. The effect of this though is that the repository works as an in-out parameter, which is quite awkward. Refactor the code so that the caller only needs to pass the value. Starting with this change, the passed-in repository can essentially be completely blank as it doesn't carry any state anymore that we'd care about in `create_repository()`. Note that this change in theory also impacts the other caller of `create_repository()` that exists in git-clone(1). But that caller already passes `-1` as a value for this parameter, and neither does that caller modify the repository it passes. So there shouldn't be any change in behaviour here. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/init-db.c | 3 --- setup.c | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/init-db.c b/builtin/init-db.c index e45268f1ff..34215bbf18 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -171,9 +171,6 @@ int cmd_init_db(int argc, die(_("unknown ref storage format '%s'"), ref_format); } - if (init_shared_repository != -1) - repo_settings_set_shared_repository(the_repository, init_shared_repository); - /* * GIT_WORK_TREE makes sense only in conjunction with GIT_DIR * without --bare. Catch the error early. diff --git a/setup.c b/setup.c index f335111d1e..0d0a4abbe6 100644 --- a/setup.c +++ b/setup.c @@ -2896,6 +2896,9 @@ void create_repository(struct repository *repo, */ repo_config(repo, git_default_core_config, NULL); + if (init_shared_repository != -1) + repo_settings_set_shared_repository(repo, init_shared_repository); + safe_create_dir(repo, git_dir, 0); if (!reinit_ok) From 8f74abc2de1c05e6a9d4eede0e95b41e57bbd120 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 28 Sep 2026 11:51:06 +0200 Subject: [PATCH 5/7] builtin/clone: don't apply "core.sharedRepository" to leading dirs When creating a repository via git-clone(1) we create leading directories with `safe_create_leading_directories()`. We have adapted git-init(1) in a preceding commit to instead use the variant of this function that doesn't honor "core.sharedRepository". In that subcommand it didn't have an effect though as we explicitly unset the value of that configuration anyway, so we never honored that config. In git-clone(1) it's a bit of a different thing though: while the repository isn't initialized at the point in time where we call the function, we didn't explicitly unset the value. Consequently we _do_ honor the configuration here, but when it's configured in global- or system-level scope. This divergence doesn't seem to be intentional -- I cannot think of any good reason why git-init(1) and git-clone(1) should have divergent behaviour here. Adapt git-clone(1) to work the same as git-init(1) by also using the `no_share()` variants to create leading directories. Add tests for both commands. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/clone.c | 4 ++-- t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index b14264c33a..e72f8aa325 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1133,7 +1133,7 @@ int cmd_clone(int argc, sigchain_push_common(remove_junk_on_signal); if (!option_bare) { - if (safe_create_leading_directories_const(the_repository, work_tree) < 0) + if (safe_create_leading_directories_no_share_const(work_tree) < 0) die_errno(_("could not create leading directories of '%s'"), work_tree); if (dest_exists) @@ -1153,7 +1153,7 @@ int cmd_clone(int argc, junk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL; junk_git_dir = git_dir; } - if (safe_create_leading_directories_const(the_repository, git_dir) < 0) + if (safe_create_leading_directories_no_share_const(git_dir) < 0) die(_("could not create leading directories of '%s'"), git_dir); if (0 <= option_verbosity) { diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh index 0e0d07a1a1..3bc4bdb038 100755 --- a/t/t1301-shared-repo.sh +++ b/t/t1301-shared-repo.sh @@ -210,4 +210,46 @@ test_expect_success POSIXPERM 'template can set core.sharedrepository' ' test_cmp expect actual ' +test_expect_success POSIXPERM 'init does not apply core.sharedRepository to leading directories' ' + test_config_global core.sharedRepository 0666 && + umask 0077 && + test_when_finished "rm -rf dst" && + git init --bare dst/with/leading/dirs && + cat >expect <<-\EOF && + drwx------ + drwx------ + drwx------ + drwxrwxrwx + EOF + { + test_modebits dst && + test_modebits dst/with && + test_modebits dst/with/leading && + test_modebits dst/with/leading/dirs + } >actual && + test_cmp expect actual +' + +test_expect_success POSIXPERM 'clone does not apply core.sharedRepository to leading directories' ' + test_config_global core.sharedRepository 0666 && + umask 0077 && + test_when_finished "rm -rf source dst" && + git init source && + test_commit -C source initial && + git clone --bare source dst/with/leading/dirs && + cat >expect <<-\EOF && + drwx------ + drwx------ + drwx------ + drwxrwxrwx + EOF + { + test_modebits dst && + test_modebits dst/with && + test_modebits dst/with/leading && + test_modebits dst/with/leading/dirs + } >actual && + test_cmp expect actual +' + test_done From b2adfd39a042d2b1e7f90b1b6072358c2e117190 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 28 Sep 2026 11:51:07 +0200 Subject: [PATCH 6/7] repository: adapt `repo_clear()` to fully reset the repository The function `repo_clear()` can be used to clear a repository's state. The way it's written though it's quite easy for it to accidentally leak some state because we don't make sure to clear the whole structure. Refactor the function to set the whole repository to all-zeroes to avoid any kind of leaking state. Replace calls of `FREE_AND_NULL()` to instead use free(3p) to avoid zeroing out the data twice. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- repository.c | 37 ++++++++++++++++++------------------- repository.h | 2 +- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/repository.c b/repository.c index b857e1c580..e67ff00550 100644 --- a/repository.c +++ b/repository.c @@ -374,60 +374,57 @@ void repo_clear(struct repository *repo) struct hashmap_iter iter; struct strmap_entry *e; - FREE_AND_NULL(repo->gitdir); - FREE_AND_NULL(repo->commondir); - FREE_AND_NULL(repo->prefix); - FREE_AND_NULL(repo->graft_file); - FREE_AND_NULL(repo->index_file); - FREE_AND_NULL(repo->worktree); - FREE_AND_NULL(repo->submodule_prefix); - FREE_AND_NULL(repo->ref_storage_payload); + free(repo->gitdir); + free(repo->commondir); + free(repo->prefix); + free(repo->graft_file); + free(repo->index_file); + free(repo->worktree); + free(repo->submodule_prefix); + free(repo->ref_storage_payload); odb_free(repo->objects); - repo->objects = NULL; if (repo->parsed_objects) parsed_object_pool_clear(repo->parsed_objects); - FREE_AND_NULL(repo->parsed_objects); + free(repo->parsed_objects); repo_settings_clear(repo); repo_config_values_clear(&repo->config_values_private_); if (repo->config) { git_configset_clear(repo->config); - FREE_AND_NULL(repo->config); + free(repo->config); } - if (repo->submodule_cache) { + if (repo->submodule_cache) submodule_cache_free(repo->submodule_cache); - repo->submodule_cache = NULL; - } if (repo->index) { discard_index(repo->index); - FREE_AND_NULL(repo->index); + free(repo->index); } if (repo->hook_config_cache) { hook_cache_clear(repo->hook_config_cache); - FREE_AND_NULL(repo->hook_config_cache); + free(repo->hook_config_cache); } strmap_clear(&repo->event_jobs, 0); /* values are uintptr_t, not heap ptrs */ string_list_clear(&repo->disabled_events, 0); if (repo->promisor_remote_config) { promisor_remote_clear(repo->promisor_remote_config); - FREE_AND_NULL(repo->promisor_remote_config); + free(repo->promisor_remote_config); } if (repo->remote_state) { remote_state_clear(repo->remote_state); - FREE_AND_NULL(repo->remote_state); + free(repo->remote_state); } if (repo->refs_private) { ref_store_release(repo->refs_private); - FREE_AND_NULL(repo->refs_private); + free(repo->refs_private); } strmap_for_each_entry(&repo->submodule_ref_stores, &iter, e) @@ -439,6 +436,8 @@ void repo_clear(struct repository *repo) strmap_clear(&repo->worktree_ref_stores, 1); repo_clear_path_cache(&repo->cached_paths); + + memset(repo, 0, sizeof(*repo)); } int repo_read_index(struct repository *repo) diff --git a/repository.h b/repository.h index 11f5c2ed10..2a348012e8 100644 --- a/repository.h +++ b/repository.h @@ -258,6 +258,7 @@ void repo_set_ref_storage_format(struct repository *repo, void initialize_repository(struct repository *repo); RESULT_MUST_BE_USED int repo_init(struct repository *r, const char *gitdir, const char *worktree); +void repo_clear(struct repository *repo); /* * Initialize the repository 'subrepo' as the submodule at the given path. If @@ -273,7 +274,6 @@ int repo_submodule_init(struct repository *subrepo, struct repository *superproject, const char *path, const struct object_id *treeish_name); -void repo_clear(struct repository *repo); /* * Populates the repository's index from its index_file, an index struct will From 0419d7d9c71a7b5fbe2e7594fb6cc26f43dc748b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 28 Sep 2026 11:51:08 +0200 Subject: [PATCH 7/7] setup: enforce that passed-in repo does not carry relevant state In the preceding patches we have refactored `create_repository()` so that the passed-in repository is not used anymore to propagate any kind of state. This was done so that the parameter doesn't act like an in-out parameter, but only as an out parameter that we initialize with the state of the newly created repository. We don't enforce though that the repository _cannot_ be used to propagate state anymore, which makes it quite easy for state to sneak in at a later point again. Ideally, we'd do that by having the function create a newly allocated repository instead of taking a repository as input. But unfortunately, that does not work because we end up calling `repo_config_values()` when we create the "files" ref database, and that function requires that the passed-in repository is `the_repository`. Instead, call `repo_clear()` at the beginning of the function, which gives us a clean slate. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- setup.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/setup.c b/setup.c index 0d0a4abbe6..fa39219d6a 100644 --- a/setup.c +++ b/setup.c @@ -2858,6 +2858,9 @@ void create_repository(struct repository *repo, struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT; struct strbuf err = STRBUF_INIT; + repo_clear(repo); + initialize_repository(repo); + if (real_git_dir) { struct stat st;