Thread (142 messages) 142 messages, 3 authors, 2022-09-01
STALE1373d
Revisions (2)
  1. v3 current
  2. v4 [diff vs current]

[PATCH v3 31/32] submodule--helper: libify even more "die" paths for module_update()

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-08-21 14:00:18
Subsystem: the rest · Maintainer: Linus Torvalds

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
on-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).

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
 builtin/submodule--helper.c | 41 ++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 9e4069b36cb..65909255760 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2267,42 +2267,49 @@ static int run_update_procedure(struct update_data *ud)
 	return run_update_command(ud, subforce);
 }
 
-static const char *remote_submodule_branch(const char *path)
+static int remote_submodule_branch(const char *path, const char **branch)
 {
 	const struct submodule *sub;
-	const char *branch = NULL;
 	char *key;
+	*branch = NULL;
 
 	sub = submodule_from_path(the_repository, null_oid(), path);
 	if (!sub)
-		return NULL;
+		return die_message(_("could not initialize submodule at path '%s'"),
+				   path);
 
 	key = xstrfmt("submodule.%s.branch", sub->name);
-	if (repo_config_get_string_tmp(the_repository, key, &branch))
-		branch = sub->branch;
+	if (repo_config_get_string_tmp(the_repository, key, branch))
+		*branch = sub->branch;
 	free(key);
 
-	if (!branch)
-		return "HEAD";
+	if (!*branch) {
+		*branch = "HEAD";
+		return 0;
+	}
 
-	if (!strcmp(branch, ".")) {
+	if (!strcmp(*branch, ".")) {
 		const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
 
 		if (!refname)
-			die(_("No such ref: %s"), "HEAD");
+			return die_message(_("No such ref: %s"), "HEAD");
 
 		/* detached HEAD */
 		if (!strcmp(refname, "HEAD"))
-			die(_("Submodule (%s) branch configured to inherit "
-			      "branch from superproject, but the superproject "
-			      "is not on any branch"), sub->name);
+			return die_message(_("Submodule (%s) branch configured to inherit "
+					     "branch from superproject, but the superproject "
+					     "is not on any branch"), sub->name);
 
 		if (!skip_prefix(refname, "refs/heads/", &refname))
-			die(_("Expecting a full ref name, got %s"), refname);
-		return refname;
+			return die_message(_("Expecting a full ref name, got %s"),
+					   refname);
+
+		*branch = refname;
+		return 0;
 	}
 
-	return branch;
+	/* Our "branch" is coming from repo_config_get_string_tmp() */
+	return 0;
 }
 
 static int ensure_core_worktree(const char *path)
@@ -2437,7 +2444,9 @@ static int update_submodule(struct update_data *update_data)
 		code = get_default_remote_submodule(update_data->sm_path, &remote_name);
 		if (code)
 			return code;
-		branch = remote_submodule_branch(update_data->sm_path);
+		code = remote_submodule_branch(update_data->sm_path, &branch);
+		if (code)
+			return code;
 		remote_ref = xstrfmt("refs/remotes/%s/%s", remote_name, branch);
 
 		if (!update_data->nofetch) {
-- 
2.37.2.1279.g64dec4e13cf
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help