[3/4] remote add: use strvec to store tracking branches

Commit Message

Phillip Wood Sept. 11, 2024, 3:18 p.m. UTC

From: Phillip Wood <[e-mail]>

Store the list of branches to track in a ’struct strvec' instead of a
'struct string_list'. This in preparation for the next commit where it
will be convenient to have them stored in a NULL terminated array. This
means that we now duplicate the strings when storing them but the
overhead is not significant.

Signed-off-by: Phillip Wood <[e-mail]>
---
 builtin/remote.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Comments

Junio C Hamano Sept. 11, 2024, 5:05 p.m. UTC | #1

"Phillip Wood via GitGitGadget" <[e-mail]> writes:

> From: Phillip Wood <[e-mail]>
>
> Store the list of branches to track in a ’struct strvec' instead of a
> 'struct string_list'. This in preparation for the next commit where it
> will be convenient to have them stored in a NULL terminated array. This
> means that we now duplicate the strings when storing them but the
> overhead is not significant.
>
> Signed-off-by: Phillip Wood <[e-mail]>
> ---
>  builtin/remote.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

This has a slight conflict with a topic that has already graduated
but nothing serious.  If you need to reroll, you may want to base it
on a bit more recent tip of 'master', younger than bb424845 (Merge
branch 'rs/remote-leakfix', 2024-09-03).

Thanks.

Patrick Steinhardt Sept. 12, 2024, 10:05 a.m. UTC | #2

On Wed, Sep 11, 2024 at 03:18:36PM +0000, Phillip Wood via GitGitGadget wrote:
> From: Phillip Wood <[e-mail]>
> 
> Store the list of branches to track in a ’struct strvec' instead of a
> 'struct string_list'. This in preparation for the next commit where it

s/in/is &/

> will be convenient to have them stored in a NULL terminated array. This
> means that we now duplicate the strings when storing them but the
> overhead is not significant.

Yup. Micro-optimizations like this typically don't really have any real
world effect anyway.

Patrick

Patch

diff --git a/builtin/remote.c b/builtin/remote.c
index 4dbf7a4c506..318701496ed 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -158,7 +158,7 @@  static int add(int argc, const char **argv, const char *prefix)
 {
 	int fetch = 0, fetch_tags = TAGS_DEFAULT;
 	unsigned mirror = MIRROR_NONE;
-	struct string_list track = STRING_LIST_INIT_NODUP;
+	struct strvec track = STRVEC_INIT;
 	const char *master = NULL;
 	struct remote *remote;
 	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
@@ -171,8 +171,8 @@  static int add(int argc, const char **argv, const char *prefix)
 			    N_("import all tags and associated objects when fetching\n"
 			       "or do not fetch any tag at all (--no-tags)"),
 			    TAGS_SET),
-		OPT_STRING_LIST('t', "track", &track, N_("branch"),
-				N_("branch(es) to track")),
+		OPT_STRVEC('t', "track", &track, N_("branch"),
+			   N_("branch(es) to track")),
 		OPT_STRING('m', "master", &master, N_("branch"), N_("master branch")),
 		OPT_CALLBACK_F(0, "mirror", &mirror, "(push|fetch)",
 			N_("set up remote as a mirror to push to or fetch from"),
@@ -210,10 +210,9 @@  static int add(int argc, const char **argv, const char *prefix)
 		strbuf_reset(&buf);
 		strbuf_addf(&buf, "remote.%s.fetch", name);
 		if (track.nr == 0)
-			string_list_append(&track, "*");
+			strvec_push(&track, "*");
 		for (i = 0; i < track.nr; i++) {
-			add_branch(buf.buf, track.items[i].string,
-				   name, mirror, &buf2);
+			add_branch(buf.buf, track.v[i], name, mirror, &buf2);
 		}
 	}
 
@@ -246,7 +245,7 @@  static int add(int argc, const char **argv, const char *prefix)
 
 	strbuf_release(&buf);
 	strbuf_release(&buf2);
-	string_list_clear(&track, 0);
+	strvec_clear(&track);
 
 	return 0;
 }