Thread (142 messages) 142 messages, 3 authors, 2022-09-01

Re: [PATCH 13/20] submodule--helper: stop conflating "sb" in clone_submodule()

From: Glen Choo <hidden>
Date: 2022-07-29 17:08:53

Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted hunk ↗ jump to hunk
Refactor the two uses of a "struct strbuf sb" such that each of them
exists in its own scope. This makes the control flow clearer.

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
 builtin/submodule--helper.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f74957444e1..6cedcc5b239 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1557,16 +1557,24 @@ static void prepare_possible_alternates(const char *sm_name,
 	free(error_strategy);
 }
 
-static int clone_submodule(struct module_clone_data *clone_data)
+static char *clone_submodule_sm_gitdir(const char *name)
 {
-	char *p, *sm_gitdir;
-	char *sm_alternate = NULL, *error_strategy = NULL;
 	struct strbuf sb = STRBUF_INIT;
-	struct child_process cp = CHILD_PROCESS_INIT;
+	char *sm_gitdir;
 
-	submodule_name_to_gitdir(&sb, the_repository, clone_data->name);
+	submodule_name_to_gitdir(&sb, the_repository, name);
 	sm_gitdir = absolute_pathdup(sb.buf);
-	strbuf_reset(&sb);
+	strbuf_release(&sb);
+
+	return sm_gitdir;
+}
+
+static int clone_submodule(struct module_clone_data *clone_data)
+{
+	char *p;
+	char *sm_gitdir = clone_submodule_sm_gitdir(clone_data->name);
+	char *sm_alternate = NULL, *error_strategy = NULL;
+	struct child_process cp = CHILD_PROCESS_INIT;
 
 	if (!is_absolute_path(clone_data->path))
 		clone_data->path = xstrfmt("%s/%s", get_git_work_tree(),
@@ -1624,6 +1632,8 @@ static int clone_submodule(struct module_clone_data *clone_data)
 			die(_("clone of '%s' into submodule path '%s' failed"),
 			    clone_data->url, clone_data->path);
 	} else {
+		struct strbuf sb = STRBUF_INIT;
+
 		if (clone_data->require_init && !access(clone_data->path, X_OK) &&
 		    !is_empty_dir(clone_data->path))
 			die(_("directory not empty: '%s'"), clone_data->path);
@@ -1631,7 +1641,7 @@ static int clone_submodule(struct module_clone_data *clone_data)
 			die(_("could not create directory '%s'"), clone_data->path);
 		strbuf_addf(&sb, "%s/index", sm_gitdir);
 		unlink_or_warn(sb.buf);
-		strbuf_reset(&sb);
+		strbuf_release(&sb);
 	}
As Junio mentioned in
https://lore.kernel.org/git/xmqqlesmf9or.fsf@gitster.g (local), we could also
replace this with xstrfmt(). I think that gets rid of all of the
users of "sb", so I doubt we'll even need this patch once we
make that change :)
quoted hunk ↗ jump to hunk
 
 	connect_work_tree_and_git_dir(clone_data->path, sm_gitdir, 0);
@@ -1653,7 +1663,6 @@ static int clone_submodule(struct module_clone_data *clone_data)
 	free(sm_alternate);
 	free(error_strategy);
 
-	strbuf_release(&sb);
 	free(sm_gitdir);
 	free(p);
 	return 0;
-- 
2.37.1.1167.g38fda70d8c4
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help