Re: [PATCH 05/16] path: refactor `repo_submodule_path()` family of functions

> On Thu, Feb 06, 2025 at 04:05:13AM -0800, Karthik Nayak wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>>
>> [snip]
>>
>> > diff --git a/path.c b/path.c
>> > index d918d0409e..d721507be8 100644
>> > --- a/path.c
>> > +++ b/path.c
>> > @@ -560,14 +560,15 @@ const char *repo_worktree_path_replace(const struct repository *repo,
>> >  }
>> >
>> >  /* Returns 0 on success, negative on failure. */
>> > -static int do_submodule_path(struct strbuf *buf, const char *path,
>> > +static int do_submodule_path(struct repository *repo,
>> > +			     struct strbuf *buf, const char *path,
>> >  			     const char *fmt, va_list args)
>> >  {
>> >  	struct strbuf git_submodule_common_dir = STRBUF_INIT;
>> >  	struct strbuf git_submodule_dir = STRBUF_INIT;
>> >  	int ret;
>> >
>> > -	ret = submodule_to_gitdir(the_repository, &git_submodule_dir, path);
>> > +	ret = submodule_to_gitdir(repo, &git_submodule_dir, path);
>> >  	if (ret)
>> >  		goto cleanup;
>> >
>> > @@ -586,13 +587,14 @@ static int do_submodule_path(struct strbuf *buf, const char *path,
>> >  	return ret;
>> >  }
>> >
>> > -char *git_pathdup_submodule(const char *path, const char *fmt, ...)
>> > +char *repo_submodule_path(struct repository *repo,
>>
>> To stay consistent with the other repo_* functions, should we change
>> `struct repository *repo` to `const struct repository *repo`?
>
> Somebody noticed :) But no, we cannot, we need to internally pass the
> repo to functions that expect a non-const pointer. This is because deep
> down in the callstack we end up calling `repo_read_gitmodules()`, which
> modifies the repository.
>