@matheusbsbs / matheus-bagliano / commits / 5fff35d880

submodule: fix latent check_has_commit() bug

When check_has_commit() is called on a missing submodule, initialization of the struct repository fails, but it attempts to clear the struct anyway (which is a fatal error). This bug is masked by its only caller, submodule_has_commits(), first calling add_submodule_odb(). The latter fails if the submodule does not exist, making submodule_has_commits() exit early and not invoke check_has_commit(). Fix this bug, and because calling add_submodule_odb() is no longer necessary as of 13a2f620b2 (submodule: pass repo to check_has_commit(), 2021-10-08), remove that call too. This is the last caller of add_submodule_odb(), so remove that function. (Submodule ODBs are still added as alternates via add_submodule_odb_by_path().) Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Glen Choo committed Mar 7, 2022 at 16:14 UTC 5fff35d880df2bb4cfce032c54a95abadce3f881

2 files changed +6 -38

submodule.c

+2 -33

@@ -167,26 +167,6 @@ void stage_updated_gitmodules(struct index_state *istate)

167

168 static struct string_list added_submodule_odb_paths = STRING_LIST_INIT_NODUP;

169

170 -/* TODO: remove this function, use repo_submodule_init instead. */

171 -int add_submodule_odb(const char *path)

172 -{

173 - struct strbuf objects_directory = STRBUF_INIT;

174 - int ret = 0;

175 -

176 - ret = strbuf_git_path_submodule(&objects_directory, path, "objects/");

177 - if (ret)

178 - goto done;

179 - if (!is_directory(objects_directory.buf)) {

180 - ret = -1;

181 - goto done;

182 - }

183 - string_list_insert(&added_submodule_odb_paths,

184 - strbuf_detach(&objects_directory, NULL));

185 -done:

186 - strbuf_release(&objects_directory);

187 - return ret;

188 -}

189 -

170 void add_submodule_odb_by_path(const char *path)

171 {

172 string_list_insert(&added_submodule_odb_paths, xstrdup(path));

@@ -986,7 +966,8 @@ static int check_has_commit(const struct object_id *oid, void *data)

966

967 if (repo_submodule_init(&subrepo, cb->repo, cb->path, cb->super_oid)) {

968 cb->result = 0;

989 - goto cleanup;

969 + /* subrepo failed to init, so don't clean it up. */

970 + return 0;

971 }

972

973 type = oid_object_info(&subrepo, oid, NULL);

@@ -1022,18 +1003,6 @@ static int submodule_has_commits(struct repository *r,

1003 .super_oid = super_oid

1004 };

1005

1025 - /*

1026 - * Perform a cheap, but incorrect check for the existence of 'commits'.

1027 - * This is done by adding the submodule's object store to the in-core

1028 - * object store, and then querying for each commit's existence. If we

1029 - * do not have the commit object anywhere, there is no chance we have

1030 - * it in the object store of the correct submodule and have it

1031 - * reachable from a ref, so we can fail early without spawning rev-list

1032 - * which is expensive.

1033 - */

1034 - if (add_submodule_odb(path))

1035 - return 0;

1036 -

1006 oid_array_for_each_unique(commits, check_has_commit, &has_commit);

1007

1008 if (has_commit.result) {

submodule.h

+4 -5

@@ -103,12 +103,11 @@ int submodule_uses_gitfile(const char *path);

103 int bad_to_remove_submodule(const char *path, unsigned flags);

104

105 /*

106 - * Call add_submodule_odb() to add the submodule at the given path to a list.

107 - * When register_all_submodule_odb_as_alternates() is called, the object stores

108 - * of all submodules in that list will be added as alternates in

109 - * the_repository.

106 + * Call add_submodule_odb_by_path() to add the submodule at the given

107 + * path to a list. When register_all_submodule_odb_as_alternates() is

108 + * called, the object stores of all submodules in that list will be

109 + * added as alternates in the_repository.

110 */

111 -int add_submodule_odb(const char *path);

111 void add_submodule_odb_by_path(const char *path);

112 int register_all_submodule_odb_as_alternates(void);

113