submodule--helper: check repo{_submodule,}_init() return values

Fix code added in ce125d431aa (submodule: extract path to submodule gitdir func, 2021-09-15) and a77c3fcb5ec (submodule--helper: get remote names from any repository, 2022-03-04) which failed to check the return values of repo_init() and repo_submodule_init(). If we failed to initialize the repository or submodule we could segfault when trying to access the invalid repository structs. Let's also check that these were the only such logic errors in the codebase by making use of the "warn_unused_result" attribute. This is valid as of GCC 3.4.0 (and clang will catch it via its faking of __GNUC__ ). As the comment being added to git-compat-util.h we're piggy-backing on the LAST_ARG_MUST_BE_NULL version check out of lazyness. See 9fe3edc47f1 (Add the LAST_ARG_MUST_BE_NULL macro, 2013-07-18) for its addition. The marginal benefit of covering gcc 3.4.0..4.0.0 is near-zero (or zero) at this point. It mostly matters that we catch this somewhere. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Sep 1, 2022 at 01:18 UTC 1e8697b5c4ed7c2762a0858251efa8feb8126038
3 files changed +13 -2
builtin/submodule--helper.c
+7 -2
@@ -63,7 +63,10 @@ static char *get_default_remote_submodule(const char *module_path)
63 {
64 struct repository subrepo;
65
66 - repo_submodule_init(&subrepo, the_repository, module_path, null_oid());
66 + if (repo_submodule_init(&subrepo, the_repository, module_path,
67 + null_oid()) < 0)
68 + die(_("could not get a repository handle for submodule '%s'"),
69 + module_path);
70 return repo_get_default_remote(&subrepo);
71 }
72
@@ -1480,7 +1483,9 @@ static int add_possible_reference_from_superproject(
1483 struct strbuf err = STRBUF_INIT;
1484 strbuf_add(&sb, odb->path, len);
1485
1483 - repo_init(&alternate, sb.buf, NULL);
1486 + if (repo_init(&alternate, sb.buf, NULL) < 0)
1487 + die(_("could not get a repository handle for gitdir '%s'"),
1488 + sb.buf);
1489
1490 /*
1491 * We need to end the new path with '/' to mark it as a dir,
git-compat-util.h
+3
@@ -565,8 +565,11 @@ static inline int git_has_dir_sep(const char *path)
565 /* The sentinel attribute is valid from gcc version 4.0 */
566 #if defined(__GNUC__) && (__GNUC__ >= 4)
567 #define LAST_ARG_MUST_BE_NULL __attribute__((sentinel))
568 +/* warn_unused_result exists as of gcc 3.4.0, but be lazy and check 4.0 */
569 +#define RESULT_MUST_BE_USED __attribute__ ((warn_unused_result))
570 #else
571 #define LAST_ARG_MUST_BE_NULL
572 +#define RESULT_MUST_BE_USED
573 #endif
574
575 #define MAYBE_UNUSED __attribute__((__unused__))
repository.h
+3
@@ -1,6 +1,7 @@
1 #ifndef REPOSITORY_H
2 #define REPOSITORY_H
3
4 +#include "git-compat-util.h"
5 #include "path.h"
6
7 struct config_set;
@@ -185,6 +186,7 @@ void repo_set_gitdir(struct repository *repo, const char *root,
186 void repo_set_worktree(struct repository *repo, const char *path);
187 void repo_set_hash_algo(struct repository *repo, int algo);
188 void initialize_the_repository(void);
189 +RESULT_MUST_BE_USED
190 int repo_init(struct repository *r, const char *gitdir, const char *worktree);
191
192 /*
@@ -196,6 +198,7 @@ int repo_init(struct repository *r, const char *gitdir, const char *worktree);
198 * Return 0 upon success and a non-zero value upon failure.
199 */
200 struct object_id;
201 +RESULT_MUST_BE_USED
202 int repo_submodule_init(struct repository *subrepo,
203 struct repository *superproject,
204 const char *path,