[PATCH] submodule: use strvec_pushf() for --super-prefix

[PATCH] submodule: use strvec_pushf() for --super-prefix

[René Scharfe]

absorb_git_dir_into_superproject() uses a strbuf and strvec_pushl() to
build and add the --super-prefix option and its argument.  Use a single
strvec_pushf() call to add the stuck form instead, which reduces the
code size and avoids a strbuf allocation and release.  The same is
already done in submodule_reset_index() and submodule_move_head().

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 submodule.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/submodule.c b/submodule.c
index bf7a2c7918..0a82aa9bdd 100644
--- a/submodule.c
+++ b/submodule.c
@@ -2361,26 +2361,20 @@ void absorb_git_dir_into_superproject(const char *path,

 	if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
 		struct child_process cp = CHILD_PROCESS_INIT;
-		struct strbuf sb = STRBUF_INIT;

 		if (flags & ~ABSORB_GITDIR_RECURSE_SUBMODULES)
 			BUG("we don't know how to pass the flags down?");

-		strbuf_addstr(&sb, get_super_prefix_or_empty());
-		strbuf_addstr(&sb, path);
-		strbuf_addch(&sb, '/');
-
 		cp.dir = path;
 		cp.git_cmd = 1;
 		cp.no_stdin = 1;
-		strvec_pushl(&cp.args, "--super-prefix", sb.buf,
-			     "submodule--helper",
+		strvec_pushf(&cp.args, "--super-prefix=%s%s/",
+			     get_super_prefix_or_empty(), path);
+		strvec_pushl(&cp.args, "submodule--helper",
 			     "absorbgitdirs", NULL);
 		prepare_submodule_repo_env(&cp.env);
 		if (run_command(&cp))
 			die(_("could not recurse into submodule '%s'"), path);
-
-		strbuf_release(&sb);
 	}
 }

--
2.38.1

Re: [PATCH] submodule: use strvec_pushf() for --super-prefix

[Jeff King]

On Sun, Oct 23, 2022 at 08:47:35AM +0200, René Scharfe wrote:

> absorb_git_dir_into_superproject() uses a strbuf and strvec_pushl() to
> build and add the --super-prefix option and its argument.  Use a single
> strvec_pushf() call to add the stuck form instead, which reduces the
> code size and avoids a strbuf allocation and release.  The same is
> already done in submodule_reset_index() and submodule_move_head().

Nice. Unlike some of the earlier discussion around child_process args,
this one seems like a very clear win.

-Peff