remote: use strvecs to store remote url/pushurl

Now that the url/pushurl fields of "struct remote" own their strings, we can switch from bare arrays to strvecs. This has a few advantages: - push/clear are now one-liners - likewise the free+assigns in alias_all_urls() can use strvec_replace() - we now use size_t for storage, avoiding possible overflow - this will enable some further cleanups in future patches There's quite a bit of fallout in the code that reads these fields, as it tends to access these arrays directly. But it's mostly a mechanical replacement of "url_nr" with "url.nr", and "url[i]" with "url.v[i]", with a few variations (e.g. "*url" could become "*url.v", but I used "url.v[0]" for consistency). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jun 14, 2024 at 06:28 UTC 8e804415fd3183f56ced0dcc168f8cb4aa790473
10 files changed +68 -84
builtin/archive.c
+2 -2
@@ -31,9 +31,9 @@ static int run_remote_archiver(int argc, const char **argv,
31 struct packet_reader reader;
32
33 _remote = remote_get(remote);
34 - if (!_remote->url_nr)
34 + if (!_remote->url.nr)
35 die(_("git archive: Remote with no URL"));
36 - transport = transport_get(_remote, _remote->url[0]);
36 + transport = transport_get(_remote, _remote->url.v[0]);
37 transport_connect(transport, "git-upload-archive", exec, fd);
38
39 /*
builtin/clone.c
+2 -2
@@ -1290,7 +1290,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1290 refspec_appendf(&remote->fetch, "+%s*:%s*", src_ref_prefix,
1291 branch_top.buf);
1292
1293 - path = get_repo_path(remote->url[0], &is_bundle);
1293 + path = get_repo_path(remote->url.v[0], &is_bundle);
1294 is_local = option_local != 0 && path && !is_bundle;
1295 if (is_local) {
1296 if (option_depth)
@@ -1312,7 +1312,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
1312 if (option_local > 0 && !is_local)
1313 warning(_("--local is ignored"));
1314
1315 - transport = transport_get(remote, path ? path : remote->url[0]);
1315 + transport = transport_get(remote, path ? path : remote->url.v[0]);
1316 transport_set_verbosity(transport, option_verbosity, option_progress);
1317 transport->family = family;
1318 transport->cloning = 1;
builtin/ls-remote.c
+3 -3
@@ -109,11 +109,11 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
109 die("bad repository '%s'", dest);
110 die("No remote configured to list refs from.");
111 }
112 - if (!remote->url_nr)
112 + if (!remote->url.nr)
113 die("remote %s has no configured URL", dest);
114
115 if (get_url) {
116 - printf("%s\n", *remote->url);
116 + printf("%s\n", remote->url.v[0]);
117 return 0;
118 }
119
@@ -130,7 +130,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
130 }
131
132 if (!dest && !quiet)
133 - fprintf(stderr, "From %s\n", *remote->url);
133 + fprintf(stderr, "From %s\n", remote->url.v[0]);
134 for ( ; ref; ref = ref->next) {
135 struct ref_array_item *item;
136 if (!check_ref_type(ref, flags))
builtin/push.c
+5 -5
@@ -143,12 +143,12 @@ static void set_refspecs(const char **refs, int nr, const char *repo)
143
144 static int push_url_of_remote(struct remote *remote, const char ***url_p)
145 {
146 - if (remote->pushurl_nr) {
147 - *url_p = remote->pushurl;
148 - return remote->pushurl_nr;
146 + if (remote->pushurl.nr) {
147 + *url_p = remote->pushurl.v;
148 + return remote->pushurl.nr;
149 }
150 - *url_p = remote->url;
151 - return remote->url_nr;
150 + *url_p = remote->url.v;
151 + return remote->url.nr;
152 }
153
154 static NORETURN void die_push_simple(struct branch *branch,
builtin/remote.c
+28 -28
@@ -619,8 +619,8 @@ static int migrate_file(struct remote *remote)
619 int i;
620
621 strbuf_addf(&buf, "remote.%s.url", remote->name);
622 - for (i = 0; i < remote->url_nr; i++)
623 - git_config_set_multivar(buf.buf, remote->url[i], "^$", 0);
622 + for (i = 0; i < remote->url.nr; i++)
623 + git_config_set_multivar(buf.buf, remote->url.v[i], "^$", 0);
624 strbuf_reset(&buf);
625 strbuf_addf(&buf, "remote.%s.push", remote->name);
626 for (i = 0; i < remote->push.raw_nr; i++)
@@ -1002,8 +1002,8 @@ static int get_remote_ref_states(const char *name,
1002 struct transport *transport;
1003 const struct ref *remote_refs;
1004
1005 - transport = transport_get(states->remote, states->remote->url_nr > 0 ?
1006 - states->remote->url[0] : NULL);
1005 + transport = transport_get(states->remote, states->remote->url.nr > 0 ?
1006 + states->remote->url.v[0] : NULL);
1007 remote_refs = transport_get_remote_refs(transport, NULL);
1008
1009 states->queried = 1;
@@ -1216,12 +1216,12 @@ static int get_one_entry(struct remote *remote, void *priv)
1216 const char **url;
1217 int i, url_nr;
1218
1219 - if (remote->url_nr > 0) {
1219 + if (remote->url.nr > 0) {
1220 struct strbuf promisor_config = STRBUF_INIT;
1221 const char *partial_clone_filter = NULL;
1222
1223 strbuf_addf(&promisor_config, "remote.%s.partialclonefilter", remote->name);
1224 - strbuf_addf(&remote_info_buf, "%s (fetch)", remote->url[0]);
1224 + strbuf_addf(&remote_info_buf, "%s (fetch)", remote->url.v[0]);
1225 if (!git_config_get_string_tmp(promisor_config.buf, &partial_clone_filter))
1226 strbuf_addf(&remote_info_buf, " [%s]", partial_clone_filter);
1227
@@ -1230,12 +1230,12 @@ static int get_one_entry(struct remote *remote, void *priv)
1230 strbuf_detach(&remote_info_buf, NULL);
1231 } else
1232 string_list_append(list, remote->name)->util = NULL;
1233 - if (remote->pushurl_nr) {
1234 - url = remote->pushurl;
1235 - url_nr = remote->pushurl_nr;
1233 + if (remote->pushurl.nr) {
1234 + url = remote->pushurl.v;
1235 + url_nr = remote->pushurl.nr;
1236 } else {
1237 - url = remote->url;
1238 - url_nr = remote->url_nr;
1237 + url = remote->url.v;
1238 + url_nr = remote->url.nr;
1239 }
1240 for (i = 0; i < url_nr; i++)
1241 {
@@ -1301,14 +1301,14 @@ static int show(int argc, const char **argv, const char *prefix)
1301 get_remote_ref_states(*argv, &info.states, query_flag);
1302
1303 printf_ln(_("* remote %s"), *argv);
1304 - printf_ln(_(" Fetch URL: %s"), info.states.remote->url_nr > 0 ?
1305 - info.states.remote->url[0] : _("(no URL)"));
1306 - if (info.states.remote->pushurl_nr) {
1307 - url = info.states.remote->pushurl;
1308 - url_nr = info.states.remote->pushurl_nr;
1304 + printf_ln(_(" Fetch URL: %s"), info.states.remote->url.nr > 0 ?
1305 + info.states.remote->url.v[0] : _("(no URL)"));
1306 + if (info.states.remote->pushurl.nr) {
1307 + url = info.states.remote->pushurl.v;
1308 + url_nr = info.states.remote->pushurl.nr;
1309 } else {
1310 - url = info.states.remote->url;
1311 - url_nr = info.states.remote->url_nr;
1310 + url = info.states.remote->url.v;
1311 + url_nr = info.states.remote->url.nr;
1312 }
1313 for (i = 0; i < url_nr; i++)
1314 /*
@@ -1454,8 +1454,8 @@ static int prune_remote(const char *remote, int dry_run)
1454
1455 printf_ln(_("Pruning %s"), remote);
1456 printf_ln(_("URL: %s"),
1457 - states.remote->url_nr
1458 - ? states.remote->url[0]
1457 + states.remote->url.nr
1458 + ? states.remote->url.v[0]
1459 : _("(no URL)"));
1460
1461 for_each_string_list_item(item, &states.stale)
@@ -1647,15 +1647,15 @@ static int get_url(int argc, const char **argv, const char *prefix)
1647
1648 url_nr = 0;
1649 if (push_mode) {
1650 - url = remote->pushurl;
1651 - url_nr = remote->pushurl_nr;
1650 + url = remote->pushurl.v;
1651 + url_nr = remote->pushurl.nr;
1652 }
1653 /* else fetch mode */
1654
1655 /* Use the fetch URL when no push URLs were found or requested. */
1656 if (!url_nr) {
1657 - url = remote->url;
1658 - url_nr = remote->url_nr;
1657 + url = remote->url.v;
1658 + url_nr = remote->url.nr;
1659 }
1660
1661 if (!url_nr)
@@ -1718,12 +1718,12 @@ static int set_url(int argc, const char **argv, const char *prefix)
1718
1719 if (push_mode) {
1720 strbuf_addf(&name_buf, "remote.%s.pushurl", remotename);
1721 - urlset = remote->pushurl;
1722 - urlset_nr = remote->pushurl_nr;
1721 + urlset = remote->pushurl.v;
1722 + urlset_nr = remote->pushurl.nr;
1723 } else {
1724 strbuf_addf(&name_buf, "remote.%s.url", remotename);
1725 - urlset = remote->url;
1726 - urlset_nr = remote->url_nr;
1725 + urlset = remote->url.v;
1726 + urlset_nr = remote->url.nr;
1727 }
1728
1729 /* Special cases that add new entry. */
remote-curl.c
+1 -1
@@ -1574,7 +1574,7 @@ int cmd_main(int argc, const char **argv)
1574 if (argc > 2) {
1575 end_url_with_slash(&url, argv[2]);
1576 } else {
1577 - end_url_with_slash(&url, remote->url[0]);
1577 + end_url_with_slash(&url, remote->url.v[0]);
1578 }
1579
1580 http_init(remote, url.buf, 0);
remote.c
+21 -31
@@ -32,7 +32,7 @@ struct counted_string {
32
33 static int valid_remote(const struct remote *remote)
34 {
35 - return (!!remote->url) || (!!remote->foreign_vcs);
35 + return (!!remote->url.nr) || (!!remote->foreign_vcs);
36 }
37
38 static char *alias_url(const char *url, struct rewrites *r)
@@ -63,14 +63,12 @@ static char *alias_url(const char *url, struct rewrites *r)
63
64 static void add_url(struct remote *remote, const char *url)
65 {
66 - ALLOC_GROW(remote->url, remote->url_nr + 1, remote->url_alloc);
67 - remote->url[remote->url_nr++] = xstrdup(url);
66 + strvec_push(&remote->url, url);
67 }
68
69 static void add_pushurl(struct remote *remote, const char *pushurl)
70 {
72 - ALLOC_GROW(remote->pushurl, remote->pushurl_nr + 1, remote->pushurl_alloc);
73 - remote->pushurl[remote->pushurl_nr++] = xstrdup(pushurl);
71 + strvec_push(&remote->pushurl, pushurl);
72 }
73
74 static void add_pushurl_alias(struct remote_state *remote_state,
@@ -150,18 +148,12 @@ static struct remote *make_remote(struct remote_state *remote_state,
148
149 static void remote_clear(struct remote *remote)
150 {
153 - int i;
154 -
151 free((char *)remote->name);
152 free((char *)remote->foreign_vcs);
153
158 - for (i = 0; i < remote->url_nr; i++)
159 - free((char *)remote->url[i]);
160 - FREE_AND_NULL(remote->url);
154 + strvec_clear(&remote->url);
155 + strvec_clear(&remote->pushurl);
156
162 - for (i = 0; i < remote->pushurl_nr; i++)
163 - free((char *)remote->pushurl[i]);
164 - FREE_AND_NULL(remote->pushurl);
157 free((char *)remote->receivepack);
158 free((char *)remote->uploadpack);
159 FREE_AND_NULL(remote->http_proxy);
@@ -493,27 +485,25 @@ static void alias_all_urls(struct remote_state *remote_state)
485 int add_pushurl_aliases;
486 if (!remote_state->remotes[i])
487 continue;
496 - for (j = 0; j < remote_state->remotes[i]->pushurl_nr; j++) {
497 - char *alias = alias_url(remote_state->remotes[i]->pushurl[j],
488 + for (j = 0; j < remote_state->remotes[i]->pushurl.nr; j++) {
489 + char *alias = alias_url(remote_state->remotes[i]->pushurl.v[j],
490 &remote_state->rewrites);
499 - if (alias) {
500 - free((char *)remote_state->remotes[i]->pushurl[j]);
501 - remote_state->remotes[i]->pushurl[j] = alias;
502 - }
491 + if (alias)
492 + strvec_replace(&remote_state->remotes[i]->pushurl,
493 + j, alias);
494 }
504 - add_pushurl_aliases = remote_state->remotes[i]->pushurl_nr == 0;
505 - for (j = 0; j < remote_state->remotes[i]->url_nr; j++) {
495 + add_pushurl_aliases = remote_state->remotes[i]->pushurl.nr == 0;
496 + for (j = 0; j < remote_state->remotes[i]->url.nr; j++) {
497 char *alias;
498 if (add_pushurl_aliases)
499 add_pushurl_alias(
500 remote_state, remote_state->remotes[i],
510 - remote_state->remotes[i]->url[j]);
511 - alias = alias_url(remote_state->remotes[i]->url[j],
501 + remote_state->remotes[i]->url.v[j]);
502 + alias = alias_url(remote_state->remotes[i]->url.v[j],
503 &remote_state->rewrites);
513 - if (alias) {
514 - free((char *)remote_state->remotes[i]->url[j]);
515 - remote_state->remotes[i]->url[j] = alias;
516 - }
504 + if (alias)
505 + strvec_replace(&remote_state->remotes[i]->url,
506 + j, alias);
507 }
508 }
509 }
@@ -653,10 +643,10 @@ static void validate_remote_url(struct remote *remote)
643 else
644 die(_("unrecognized value transfer.credentialsInUrl: '%s'"), value);
645
656 - for (i = 0; i < remote->url_nr; i++) {
646 + for (i = 0; i < remote->url.nr; i++) {
647 struct url_info url_info = { 0 };
648
659 - if (!url_normalize(remote->url[i], &url_info) ||
649 + if (!url_normalize(remote->url.v[i], &url_info) ||
650 !url_info.passwd_off)
651 goto loop_cleanup;
652
@@ -830,8 +820,8 @@ struct ref *ref_remove_duplicates(struct ref *ref_map)
820 int remote_has_url(struct remote *remote, const char *url)
821 {
822 int i;
833 - for (i = 0; i < remote->url_nr; i++) {
834 - if (!strcmp(remote->url[i], url))
823 + for (i = 0; i < remote->url.nr; i++) {
824 + if (!strcmp(remote->url.v[i], url))
825 return 1;
826 }
827 return 0;
remote.h
+3 -9
@@ -4,6 +4,7 @@
4 #include "hash-ll.h"
5 #include "hashmap.h"
6 #include "refspec.h"
7 +#include "strvec.h"
8
9 struct option;
10 struct transport_ls_refs_options;
@@ -68,16 +69,9 @@ struct remote {
69 char *foreign_vcs;
70
71 /* An array of all of the url_nr URLs configured for the remote */
71 - const char **url;
72 -
73 - int url_nr;
74 - int url_alloc;
75 -
72 + struct strvec url;
73 /* An array of all of the pushurl_nr push URLs configured for the remote */
77 - const char **pushurl;
78 -
79 - int pushurl_nr;
80 - int pushurl_alloc;
74 + struct strvec pushurl;
75
76 struct refspec push;
77
t/helper/test-bundle-uri.c
+1 -1
@@ -88,7 +88,7 @@ static int cmd_ls_remote(int argc, const char **argv)
88 die(_("bad repository '%s'"), dest);
89 die(_("no remote configured to get bundle URIs from"));
90 }
91 - if (!remote->url_nr)
91 + if (!remote->url.nr)
92 die(_("remote '%s' has no configured URL"), dest);
93
94 transport = transport_get(remote, NULL);
transport.c
+2 -2
@@ -1127,8 +1127,8 @@ struct transport *transport_get(struct remote *remote, const char *url)
1127 ret->remote = remote;
1128 helper = remote->foreign_vcs;
1129
1130 - if (!url && remote->url)
1131 - url = remote->url[0];
1130 + if (!url && remote->url.nr)
1131 + url = remote->url.v[0];
1132 ret->url = url;
1133
1134 /* maybe it is a foreign URL? */