[PATCH 0/7] setup: enforce repo passed to `create_repository()` has no state

[PATCH 0/7] setup: enforce repo passed to `create_repository()` has no state

[Patrick Steinhardt]

Hi,

when creating a new repository via `create_repository()` we pass in a
repository. This repository is acting as an in/out parameter: the caller
expects that it will be fully configured after the call, but the
function itself also uses some information from the passed-in repository
to figure out how exactly we want to create it.

This interface is quite confusing, as it's not obvious at all what
configuration of the repository is relevant. We have thus over a couple
of patch series reduced the use of the parameter as in/out parameter. So
now, the only piece of info that is still being propagated via the repo
is "core.sharedRepository".

This patch series cleans up that last remaining part so that the repo
becomes purely an out-parameter. To ensure that this is the case we also
start to `repo_clear()` it as a first step.

Besides simplifying the interface, the intent is also to go further into
the direction of unifying repository initialization in a follow-up patch
series.

The series is built on top of 0f8e75abeb (Revert "Merge branch
'en/no-amend-during-conflicts'", 2026-09-23) with
ps/odb-alternates-at-creation at d1019ac894 (odb/source: remove the
ability to write alternates, 2026-09-10) merged into it.

Thanks!

Patrick

---
Patrick Steinhardt (7):
      path: drop useless `safe_create_leading_directories_1()`
      path: introduce `safe_create_leading_directories_no_share_const()`
      builtin/init: refactor messy creation of leading directories
      builtin/init: move handling of "core.sharedRepository" into "setup.c"
      builtin/clone: don't apply "core.sharedRepository" to leading dirs
      repository: adapt `repo_clear()` to fully reset the repository
      setup: enforce that passed-in repo does not carry relevant state

 builtin/clone.c        |  4 ++--
 builtin/init-db.c      | 15 ++-------------
 path.c                 | 13 ++++++-------
 path.h                 |  1 +
 repository.c           | 37 ++++++++++++++++++-------------------
 repository.h           |  2 +-
 setup.c                |  6 ++++++
 t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 78 insertions(+), 42 deletions(-)


---
base-commit: 6b6fe25b12e5324f2fdaf8c73816b9d2207e9404
change-id: 20260916-pks-create-repository-stateless-f0ca03cca689

[PATCH 1/7] path: drop useless `safe_create_leading_directories_1()`

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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,

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH 2/7] path: introduce `safe_create_leading_directories_no_share_const()`

[Patrick Steinhardt]

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 <ps@pks.im>
---
 path.c | 5 +++++
 path.h | 1 +
 2 files changed, 6 insertions(+)

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..e2d62c4978 100644
--- a/path.h
+++ b/path.h
@@ -254,6 +254,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

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH 3/7] builtin/init: refactor messy creation of leading directories

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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;

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH 4/7] builtin/init: move handling of "core.sharedRepository" into "setup.c"

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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)

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH 5/7] builtin/clone: don't apply "core.sharedRepository" to leading dirs

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH 6/7] repository: adapt `repo_clear()` to fully reset the repository

[Patrick Steinhardt]

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. While at it, make it a bit more robust when
called on an already-blank repository.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 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

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH 7/7] setup: enforce that passed-in repo does not carry relevant state

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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;
 

-- 
2.56.0.rc2.329.gd58861e689.dirty

Re: [PATCH 2/7] path: introduce `safe_create_leading_directories_no_share_const()`

[Kaartic Sivaraam]

On 9/24/26 14:49, Patrick Steinhardt wrote:
> 
> diff --git a/path.h b/path.h
> index 7e7408dd05..e2d62c4978 100644
> --- a/path.h
> +++ b/path.h
> @@ -254,6 +254,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);
> 

nit: All other variants are mentioned in the documentation blurb just 
above the declarations. Would it also be worth mentioning this new one 
there?

-- 
Sivaraam

Re: [PATCH 3/7] builtin/init: refactor messy creation of leading directories

[Kaartic Sivaraam]

On 9/24/26 14:49, Patrick Steinhardt wrote:
> 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);
> +

Even though this patch does not aim to do so, we lost a bunch of 
'the_repository' references with this change which is nice.

The patch also looks good to me.

-- 
Sivaraam

Re: [PATCH 4/7] builtin/init: move handling of "core.sharedRepository" into "setup.c"

[Kaartic Sivaraam]

On 9/24/26 14:49, Patrick Steinhardt wrote:
 >
> 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.

I was wondering if there'll be any function that between here and the 
create_repository call that may use the_repository. I could not find any 
from my reading of the code, though. So, this looks good to me.

-
Sivaraam

Re: [PATCH 0/7] setup: enforce repo passed to `create_repository()` has no state

[Kaartic Sivaraam]

On 9/24/26 14:49, Patrick Steinhardt wrote:
> 
> when creating a new repository via `create_repository()` we pass in a
> repository. This repository is acting as an in/out parameter: the caller
> expects that it will be fully configured after the call, but the
> function itself also uses some information from the passed-in repository
> to figure out how exactly we want to create it.
> 
> This interface is quite confusing, as it's not obvious at all what
> configuration of the repository is relevant. We have thus over a couple
> of patch series reduced the use of the parameter as in/out parameter. So
> now, the only piece of info that is still being propagated via the repo
> is "core.sharedRepository".
> 
> This patch series cleans up that last remaining part so that the repo
> becomes purely an out-parameter. To ensure that this is the case we also
> start to `repo_clear()` it as a first step.
> 
> Besides simplifying the interface, the intent is also to go further into
> the direction of unifying repository initialization in a follow-up patch
> series.
> 

The patches seem to be well-split and the changes look good. It was a 
nice read.

Overall, this series seems to look good to me. Thank you for making 
create_repository not rely on state from the repo given to it!

-- 
Sivaraam

Re: [PATCH 2/7] path: introduce `safe_create_leading_directories_no_share_const()`

[Patrick Steinhardt]

On Sat, Sep 26, 2026 at 01:19:42AM +0530, Kaartic Sivaraam wrote:
> On 9/24/26 14:49, Patrick Steinhardt wrote:
> > 
> > diff --git a/path.h b/path.h
> > index 7e7408dd05..e2d62c4978 100644
> > --- a/path.h
> > +++ b/path.h
> > @@ -254,6 +254,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);
> > 
> 
> nit: All other variants are mentioned in the documentation blurb just above
> the declarations. Would it also be worth mentioning this new one there?

That's fair. I find the comment to be somewhat unwieldy overall. How
about this diff?

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,

Patrick

Re: [PATCH 1/7] path: drop useless `safe_create_leading_directories_1()`

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 431 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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.
>

Nice, always happy to see '_1()' functions go away or be renamed.

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 4/7] builtin/init: move handling of "core.sharedRepository" into "setup.c"

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2836 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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.
>

So this works, becaus we already pass in the `init_shared_repository`
value to `create_repository()`. Which is currently used while creating
the default files, now we also extend it to set the adequate permissions
on the repository too.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  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)
>
> --
> 2.56.0.rc2.329.gd58861e689.dirty

With that context, this patch makes sense.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 6/7] repository: adapt `repo_clear()` to fully reset the repository

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 4440 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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. While at it, make it a bit more robust when
> called on an already-blank repository.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  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));

The reason we swap `FREE_AND_NULL()` with `free()` is because we anyways
set everything to 0. Okay.

Or was this referring to the 'already blank' repository? Since
FREE_AND_NULL() can already handle NULL values.

>  }
>
>  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);
>

This is a purely cosmetic move to bring it closer to `repo_init()`,
right? I think it makes sense.

>  /*
>   * Populates the repository's index from its index_file, an index struct will
>
> --
> 2.56.0.rc2.329.gd58861e689.dirty

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 0/7] setup: enforce repo passed to `create_repository()` has no state

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2528 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> Hi,
>
> when creating a new repository via `create_repository()` we pass in a
> repository. This repository is acting as an in/out parameter: the caller
> expects that it will be fully configured after the call, but the
> function itself also uses some information from the passed-in repository
> to figure out how exactly we want to create it.
>
> This interface is quite confusing, as it's not obvious at all what
> configuration of the repository is relevant. We have thus over a couple
> of patch series reduced the use of the parameter as in/out parameter. So
> now, the only piece of info that is still being propagated via the repo
> is "core.sharedRepository".
>
> This patch series cleans up that last remaining part so that the repo
> becomes purely an out-parameter. To ensure that this is the case we also
> start to `repo_clear()` it as a first step.
>
> Besides simplifying the interface, the intent is also to go further into
> the direction of unifying repository initialization in a follow-up patch
> series.
>
> The series is built on top of 0f8e75abeb (Revert "Merge branch
> 'en/no-amend-during-conflicts'", 2026-09-23) with
> ps/odb-alternates-at-creation at d1019ac894 (odb/source: remove the
> ability to write alternates, 2026-09-10) merged into it.
>
> Thanks!
>
> Patrick
>

The series was a good read and I didn't see anything that needed
changes. Thanks

> ---
> Patrick Steinhardt (7):
>       path: drop useless `safe_create_leading_directories_1()`
>       path: introduce `safe_create_leading_directories_no_share_const()`
>       builtin/init: refactor messy creation of leading directories
>       builtin/init: move handling of "core.sharedRepository" into "setup.c"
>       builtin/clone: don't apply "core.sharedRepository" to leading dirs
>       repository: adapt `repo_clear()` to fully reset the repository
>       setup: enforce that passed-in repo does not carry relevant state
>
>  builtin/clone.c        |  4 ++--
>  builtin/init-db.c      | 15 ++-------------
>  path.c                 | 13 ++++++-------
>  path.h                 |  1 +
>  repository.c           | 37 ++++++++++++++++++-------------------
>  repository.h           |  2 +-
>  setup.c                |  6 ++++++
>  t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
>  8 files changed, 78 insertions(+), 42 deletions(-)
>
>
> ---
> base-commit: 6b6fe25b12e5324f2fdaf8c73816b9d2207e9404
> change-id: 20260916-pks-create-repository-stateless-f0ca03cca689

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 2/7] path: introduce `safe_create_leading_directories_no_share_const()`

[Kaartic Sivaraam]

On 9/28/26 12:45, Patrick Steinhardt wrote:
> On Sat, Sep 26, 2026 at 01:19:42AM +0530, Kaartic Sivaraam wrote:
>>
>> nit: All other variants are mentioned in the documentation blurb just above
>> the declarations. Would it also be worth mentioning this new one there?
> 
> That's fair. I find the comment to be somewhat unwieldy overall. How
> about this diff?
> 
> 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`.
>    */

Reads much better to me. Thanks.

-- 
Sivaraam

[PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state

[Patrick Steinhardt]

Hi,

when creating a new repository via `create_repository()` we pass in a
repository. This repository is acting as an in/out parameter: the caller
expects that it will be fully configured after the call, but the
function itself also uses some information from the passed-in repository
to figure out how exactly we want to create it.

This interface is quite confusing, as it's not obvious at all what
configuration of the repository is relevant. We have thus over a couple
of patch series reduced the use of the parameter as in/out parameter. So
now, the only piece of info that is still being propagated via the repo
is "core.sharedRepository".

This patch series cleans up that last remaining part so that the repo
becomes purely an out-parameter. To ensure that this is the case we also
start to `repo_clear()` it as a first step.

Besides simplifying the interface, the intent is also to go further into
the direction of unifying repository initialization in a follow-up patch
series.

The series is built on top of 0f8e75abeb (Revert "Merge branch
'en/no-amend-during-conflicts'", 2026-09-23) with
ps/odb-alternates-at-creation at d1019ac894 (odb/source: remove the
ability to write alternates, 2026-09-10) merged into it.

Changes in v2:
  - Adapt documentation of `safe_create_leading_directories()`.
  - Better explain change to fully clear repos.
  - Link to v1: https://patch.msgid.link/20260924-pks-create-repository-stateless-v1-0-11499557cf31@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (7):
      path: drop useless `safe_create_leading_directories_1()`
      path: introduce `safe_create_leading_directories_no_share_const()`
      builtin/init: refactor messy creation of leading directories
      builtin/init: move handling of "core.sharedRepository" into "setup.c"
      builtin/clone: don't apply "core.sharedRepository" to leading dirs
      repository: adapt `repo_clear()` to fully reset the repository
      setup: enforce that passed-in repo does not carry relevant state

 builtin/clone.c        |  4 ++--
 builtin/init-db.c      | 15 ++-------------
 path.c                 | 13 ++++++-------
 path.h                 | 14 ++++++--------
 repository.c           | 37 ++++++++++++++++++-------------------
 repository.h           |  2 +-
 setup.c                |  6 ++++++
 t/t1301-shared-repo.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 83 insertions(+), 50 deletions(-)

Range-diff versus v1:

1:  6ec41760de = 1:  0f9513d5c6 path: drop useless `safe_create_leading_directories_1()`
2:  85ac056b80 ! 2:  3294cdb6b6 path: introduce `safe_create_leading_directories_no_share_const()`
    @@ path.c: enum scld_error safe_create_leading_directories_no_share(char *path)
      {
     
      ## path.h ##
    +@@ path.h: 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,
     @@ path.h: 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);
3:  3a7c197f1b = 3:  8dd89f144a builtin/init: refactor messy creation of leading directories
4:  c3ced666bd = 4:  f37db1b17d builtin/init: move handling of "core.sharedRepository" into "setup.c"
5:  25918a4ff6 = 5:  db76d32f2c builtin/clone: don't apply "core.sharedRepository" to leading dirs
6:  a742852675 ! 6:  19388a188c repository: adapt `repo_clear()` to fully reset the repository
    @@ Commit message
         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. While at it, make it a bit more robust when
    -    called on an already-blank repository.
    +    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 <ps@pks.im>
     
7:  760058e9c5 = 7:  0d4819f005 setup: enforce that passed-in repo does not carry relevant state

---
base-commit: 6b6fe25b12e5324f2fdaf8c73816b9d2207e9404
change-id: 20260916-pks-create-repository-stateless-f0ca03cca689

[PATCH v2 1/7] path: drop useless `safe_create_leading_directories_1()`

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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,

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH v2 2/7] path: introduce `safe_create_leading_directories_no_share_const()`

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH v2 3/7] builtin/init: refactor messy creation of leading directories

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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;

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH v2 4/7] builtin/init: move handling of "core.sharedRepository" into "setup.c"

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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)

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH v2 5/7] builtin/clone: don't apply "core.sharedRepository" to leading dirs

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH v2 6/7] repository: adapt `repo_clear()` to fully reset the repository

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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

-- 
2.56.0.rc2.329.gd58861e689.dirty

[PATCH v2 7/7] setup: enforce that passed-in repo does not carry relevant state

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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;
 

-- 
2.56.0.rc2.329.gd58861e689.dirty

Re: [PATCH 6/7] repository: adapt `repo_clear()` to fully reset the repository

[Patrick Steinhardt]

On Mon, Sep 28, 2026 at 09:18:52AM +0000, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/repository.c b/repository.c
> > index b857e1c580..e67ff00550 100644
> > --- a/repository.c
> > +++ b/repository.c
> > @@ -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));
> 
> The reason we swap `FREE_AND_NULL()` with `free()` is because we anyways
> set everything to 0. Okay.
> 
> Or was this referring to the 'already blank' repository? Since
> FREE_AND_NULL() can already handle NULL values.

Yeah, the only reason I swap to plain free(3p) calls is because it's
redundant now with the final call to memset(3p). I think the part about
already-blank repositories is not accurate anymore, but it used to be at
one point. Let me reword it.

> > 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);
> >
> 
> This is a purely cosmetic move to bring it closer to `repo_init()`,
> right? I think it makes sense.

Yes, it is.

Patrick

Re: [PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state

[Kaartic Sivaraam]

On 9/28/26 15:21, Patrick Steinhardt wrote:
> 
> [... snip ...]
 >
> 3:  3a7c197f1b = 3:  8dd89f144a builtin/init: refactor messy creation of leading directories
> 4:  c3ced666bd = 4:  f37db1b17d builtin/init: move handling of "core.sharedRepository" into "setup.c"
> 5:  25918a4ff6 = 5:  db76d32f2c builtin/clone: don't apply "core.sharedRepository" to leading dirs
> 6:  a742852675 ! 6:  19388a188c repository: adapt `repo_clear()` to fully reset the repository
>      @@ Commit message
>           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. While at it, make it a bit more robust when
>      -    called on an already-blank repository.
>      +    any kind of leaking state. Replace calls of `FREE_AND_NULL()` to instead
>      +    use free(3p) to avoid zeroing out the data twice.
>

s/free(3p)/free/
Rest of the inter-diff looks neat.

-- 
Sivaraam

Re: [PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state

[Kaartic Sivaraam]

On 9/28/26 17:45, Kaartic Sivaraam wrote:
> On 9/28/26 15:21, Patrick Steinhardt wrote:
>>
>> [... snip ...]
>  >
>> 3:  3a7c197f1b = 3:  8dd89f144a builtin/init: refactor messy creation 
>> of leading directories
>> 4:  c3ced666bd = 4:  f37db1b17d builtin/init: move handling of 
>> "core.sharedRepository" into "setup.c"
>> 5:  25918a4ff6 = 5:  db76d32f2c builtin/clone: don't apply 
>> "core.sharedRepository" to leading dirs
>> 6:  a742852675 ! 6:  19388a188c repository: adapt `repo_clear()` to 
>> fully reset the repository
>>      @@ Commit message
>>           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. While at it, make it a bit more 
>> robust when
>>      -    called on an already-blank repository.
>>      +    any kind of leaking state. Replace calls of 
>> `FREE_AND_NULL()` to instead
>>      +    use free(3p) to avoid zeroing out the data twice.
>>
> 
> s/free(3p)/free/

Oops. I meant s/free(3p)/free(3)/

 >
> Rest of the inter-diff looks neat.
> 

-- 
Sivaraam

Re: [PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state

[Patrick Steinhardt]

On Mon, Sep 28, 2026 at 05:45:00PM +0530, Kaartic Sivaraam wrote:
> On 9/28/26 15:21, Patrick Steinhardt wrote:
> > 
> > [... snip ...]
> >
> > 3:  3a7c197f1b = 3:  8dd89f144a builtin/init: refactor messy creation of leading directories
> > 4:  c3ced666bd = 4:  f37db1b17d builtin/init: move handling of "core.sharedRepository" into "setup.c"
> > 5:  25918a4ff6 = 5:  db76d32f2c builtin/clone: don't apply "core.sharedRepository" to leading dirs
> > 6:  a742852675 ! 6:  19388a188c repository: adapt `repo_clear()` to fully reset the repository
> >      @@ Commit message
> >           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. While at it, make it a bit more robust when
> >      -    called on an already-blank repository.
> >      +    any kind of leaking state. Replace calls of `FREE_AND_NULL()` to instead
> >      +    use free(3p) to avoid zeroing out the data twice.
> > 
> 
> s/free(3p)/free/
> Rest of the inter-diff looks neat.

The "(3p)" is intentional, as we use that to refer to man pages. In this
case, it's free as specified in the POSIX programmer's manual.

Thanks!

Patrick

Re: [PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state

[Patrick Steinhardt]

On Mon, Sep 28, 2026 at 05:49:20PM +0530, Kaartic Sivaraam wrote:
> On 9/28/26 17:45, Kaartic Sivaraam wrote:
> > On 9/28/26 15:21, Patrick Steinhardt wrote:
> > > 
> > > [... snip ...]
> >  >
> > > 3:  3a7c197f1b = 3:  8dd89f144a builtin/init: refactor messy
> > > creation of leading directories
> > > 4:  c3ced666bd = 4:  f37db1b17d builtin/init: move handling of
> > > "core.sharedRepository" into "setup.c"
> > > 5:  25918a4ff6 = 5:  db76d32f2c builtin/clone: don't apply
> > > "core.sharedRepository" to leading dirs
> > > 6:  a742852675 ! 6:  19388a188c repository: adapt `repo_clear()` to
> > > fully reset the repository
> > >      @@ Commit message
> > >           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. While at it, make it a bit more
> > > robust when
> > >      -    called on an already-blank repository.
> > >      +    any kind of leaking state. Replace calls of
> > > `FREE_AND_NULL()` to instead
> > >      +    use free(3p) to avoid zeroing out the data twice.
> > > 
> > 
> > s/free(3p)/free/
> 
> Oops. I meant s/free(3p)/free(3)/

Ah. 3p is correct though and refers to the POSIX man pages.

Patrick

Re: [PATCH v2 0/7] setup: enforce repo passed to `create_repository()` has no state

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>> Oops. I meant s/free(3p)/free(3)/
>
> Ah. 3p is correct though and refers to the POSIX man pages.

If used in a context where you really care about posix specified
behaviour and are interferred by differences among generic C
library's free() implementations, free(3p) may be the right way to
spell it out concisely.

Everywhere else, like in this patch where you do not care about the
distinction, the extra 'p' is merely a noise, I would have to say.

If you are writing a wrapper that _depends_ on your platform free()
being strictly posix compliant, then you might write something like

        #ifdef WE_HAVE_POSIX_FREE
        #define safe_free(x) free(x)
        #else
        static void safe_free(void *x)
        {
                ...
        }
        #endif

and your commit log message may say "We use free(3p) where
available, but otherwise emulate it via platform free() with some
safety knob".