> diff --git a/builtin/grep.c b/builtin/grep.c > index 7da8fef31a..ba7634258a 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -418,7 +418,10 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject, > const struct object_id *oid, > const char *filename, const char *path) > { > - struct repository submodule; > + struct repository subrepo; > + const struct submodule *sub = submodule_from_path(superproject, > + &null_oid, path); [snip] > - if (repo_submodule_init(&submodule, superproject, path)) { > + if (repo_submodule_init(&subrepo, superproject, sub)) { The last argument to repo_submodule_init is now "submodule_from_path(superproject, &null_oid, path)" instead of "path", and looking forward into the patch, we do not need a NULL check because repo_submodule_init() tolerates NULL in that argument. Let's see if the rest of the code follows the pattern - a call to submodule_from_path() with the 3 expected arguments (repo, null OID, path). > diff --git a/builtin/ls-files.c b/builtin/ls-files.c > index 7f9919a362..4d1649c1b3 100644 > --- a/builtin/ls-files.c > +++ b/builtin/ls-files.c > @@ -206,17 +206,19 @@ static void show_files(struct repository *repo, struct dir_struct *dir); > static void show_submodule(struct repository *superproject, > struct dir_struct *dir, const char *path) > { > - struct repository submodule; > + struct repository subrepo; > + const struct submodule *sub = submodule_from_path(superproject, > + &null_oid, path); > > - if (repo_submodule_init(&submodule, superproject, path)) > + if (repo_submodule_init(&subrepo, superproject, sub)) So far so good. > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index 5f8a804a6e..015aa1471f 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -2037,7 +2037,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix) > if (!sub) > BUG("We could get the submodule handle before?"); > > - if (repo_submodule_init(&subrepo, the_repository, path)) > + if (repo_submodule_init(&subrepo, the_repository, sub)) The definition of "sub" is not quoted here in this e-mail, but it is indeed "submodule_from_path(the_repository, &null_oid, path)". ("the_repository" in the invocation to submodule_from_path() is correct because the 2nd argument to the invocation of repo_submodule_init() is "the_repository".) > -int repo_submodule_init(struct repository *submodule, > +int repo_submodule_init(struct repository *subrepo, > struct repository *superproject, > - const char *path) > + const struct submodule *sub) > { > - const struct submodule *sub; > struct strbuf gitdir = STRBUF_INIT; > struct strbuf worktree = STRBUF_INIT; > int ret = 0; > > - sub = submodule_from_path(superproject, &null_oid, path); As expected, this line is removed. > if (!sub) { > ret = -1; > goto out; > } > > - strbuf_repo_worktree_path(&gitdir, superproject, "%s/.git", path); > - strbuf_repo_worktree_path(&worktree, superproject, "%s", path); > + strbuf_repo_worktree_path(&gitdir, superproject, "%s/.git", sub->path); > + strbuf_repo_worktree_path(&worktree, superproject, "%s", sub->path); path and sub->path are the same, so this is fine. (This can be seen from cache_put_path() and cache_lookup_path() in submodule-config.c.) > - submodule->submodule_prefix = xstrfmt("%s%s/", > - superproject->submodule_prefix ? > - superproject->submodule_prefix : > - "", path); > + subrepo->submodule_prefix = xstrfmt("%s%s/", > + superproject->submodule_prefix ? > + superproject->submodule_prefix : > + "", sub->path); Likewise. > +/* > + * Initialize the repository 'subrepo' as the submodule given by the > + * struct submodule 'sub' in parent repository 'superproject'. > + * Return 0 upon success and a non-zero value upon failure, which may happen > + * if the submodule is not found, or 'sub' is NULL. > + */ > +struct submodule; > +int repo_submodule_init(struct repository *subrepo, > struct repository *superproject, > - const char *path); > + const struct submodule *sub); Here is where it says that the last argument can be NULL. > diff --git a/t/helper/test-submodule-nested-repo-config.c b/t/helper/test-submodule-nested-repo-config.c > index a31e2a9bea..bc97929bbc 100644 > --- a/t/helper/test-submodule-nested-repo-config.c > +++ b/t/helper/test-submodule-nested-repo-config.c > @@ -10,19 +10,21 @@ static void die_usage(int argc, const char **argv, const char *msg) > > int cmd__submodule_nested_repo_config(int argc, const char **argv) > { > - struct repository submodule; > + struct repository subrepo; > + const struct submodule *sub; > > if (argc < 3) > die_usage(argc, argv, "Wrong number of arguments."); > > setup_git_directory(); > > - if (repo_submodule_init(&submodule, the_repository, argv[1])) { > + sub = submodule_from_path(the_repository, &null_oid, argv[1]); > + if (repo_submodule_init(&subrepo, the_repository, sub)) { The expected pattern. This patch looks good to me.