Re: [PATCH 05/16] path: refactor `repo_submodule_path()` family of functions
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 7, 2025, 06:16 UTC
- Message-ID
- <Z6WlONTM0lNaXTH5@pks.im>
- In-Reply-To
- <CAOLa=ZTJ7ef9rP3fQfNwSCD54zeVPL1Rd_hQfzDpEpFiStx_PQ@mail.gmail.com>
On Thu, Feb 06, 2025 at 04:05:13AM -0800, Karthik Nayak wrote:
Show 35 quoted lines
> 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.
I'll add a comment to the commit message.
Patrick