@matheusbsbs / matheus-bagliano / commits / 86e16ed3a9

submodule--helper: libify even more "die" paths for module_update()

As noted in a preceding commit the get_default_remote_submodule() and remote_submodule_branch() functions would invoke die(), and thus leave update_submodule() only partially lib-ified. We've addressed the former of those in a preceding commit, let's now address the latter. In addition to lib-ifying the function this fixes a potential (but obscure) segfault introduced by a logic error in 1012a5cbc3f (submodule--helper run-update-procedure: learn --remote, 2022-03-04): We were assuming that remote_submodule_branch() would always return non-NULL, but if the submodule_from_path() call in that function fails we'll return NULL. See its introduction in 92bbe7ccf1f (submodule--helper: add remote-branch helper, 2016-08-03). I.e. we'd previously have segfaulted in the xstrfmt() call in update_submodule() seen in the context. 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 86e16ed3a9fbf03bc8a5d4030177980193e30f57
1 file changed +25 -16
builtin/submodule--helper.c
+25 -16
@@ -2265,42 +2265,49 @@ static int run_update_procedure(const struct update_data *ud)
2265 return run_update_command(ud, subforce);
2266 }
2267
2268 -static const char *remote_submodule_branch(const char *path)
2268 +static int remote_submodule_branch(const char *path, const char **branch)
2269 {
2270 const struct submodule *sub;
2271 - const char *branch = NULL;
2271 char *key;
2272 + *branch = NULL;
2273
2274 sub = submodule_from_path(the_repository, null_oid(), path);
2275 if (!sub)
2276 - return NULL;
2276 + return die_message(_("could not initialize submodule at path '%s'"),
2277 + path);
2278
2279 key = xstrfmt("submodule.%s.branch", sub->name);
2279 - if (repo_config_get_string_tmp(the_repository, key, &branch))
2280 - branch = sub->branch;
2280 + if (repo_config_get_string_tmp(the_repository, key, branch))
2281 + *branch = sub->branch;
2282 free(key);
2283
2283 - if (!branch)
2284 - return "HEAD";
2284 + if (!*branch) {
2285 + *branch = "HEAD";
2286 + return 0;
2287 + }
2288
2286 - if (!strcmp(branch, ".")) {
2289 + if (!strcmp(*branch, ".")) {
2290 const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
2291
2292 if (!refname)
2290 - die(_("No such ref: %s"), "HEAD");
2293 + return die_message(_("No such ref: %s"), "HEAD");
2294
2295 /* detached HEAD */
2296 if (!strcmp(refname, "HEAD"))
2294 - die(_("Submodule (%s) branch configured to inherit "
2295 - "branch from superproject, but the superproject "
2296 - "is not on any branch"), sub->name);
2297 + return die_message(_("Submodule (%s) branch configured to inherit "
2298 + "branch from superproject, but the superproject "
2299 + "is not on any branch"), sub->name);
2300
2301 if (!skip_prefix(refname, "refs/heads/", &refname))
2299 - die(_("Expecting a full ref name, got %s"), refname);
2300 - return refname;
2302 + return die_message(_("Expecting a full ref name, got %s"),
2303 + refname);
2304 +
2305 + *branch = refname;
2306 + return 0;
2307 }
2308
2303 - return branch;
2309 + /* Our "branch" is coming from repo_config_get_string_tmp() */
2310 + return 0;
2311 }
2312
2313 static int ensure_core_worktree(const char *path)
@@ -2436,7 +2443,9 @@ static int update_submodule(struct update_data *update_data)
2443 code = get_default_remote_submodule(update_data->sm_path, &remote_name);
2444 if (code)
2445 return code;
2439 - branch = remote_submodule_branch(update_data->sm_path);
2446 + code = remote_submodule_branch(update_data->sm_path, &branch);
2447 + if (code)
2448 + return code;
2449 remote_ref = xstrfmt("refs/remotes/%s/%s", remote_name, branch);
2450
2451 if (!update_data->nofetch) {