@matheusbsbs / matheus-bagliano / commits / aa0595fbd6

remote: refactor alias_url() memory ownership

The alias_url() function may return either a newly allocated string (which the caller must take ownership of), or the original const "url" parameter that was passed in. This often works OK because callers are generally passing in a "url" that they expect to retain ownership of anyway. So whether we got back the original or a new string, we're always interested in storing it forever. But I suspect there are some possible leaks here (e.g., add_url_alias() may end up discarding the original "url"). Whether there are active leaks or not, this is a confusing setup that makes further refactoring of memory ownership harder. So instead of returning the original string, return NULL, forcing callers to decide what to do with it explicitly. We can then build further cleanups on top of that. 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:26 UTC aa0595fbd635bc2d16a4921244cf2bc94ca7761e
1 file changed +15 -11
remote.c
+15 -11
@@ -35,7 +35,7 @@ static int valid_remote(const struct remote *remote)
35 return (!!remote->url) || (!!remote->foreign_vcs);
36 }
37
38 -static const char *alias_url(const char *url, struct rewrites *r)
38 +static char *alias_url(const char *url, struct rewrites *r)
39 {
40 int i, j;
41 struct counted_string *longest;
@@ -56,7 +56,7 @@ static const char *alias_url(const char *url, struct rewrites *r)
56 }
57 }
58 if (!longest)
59 - return url;
59 + return NULL;
60
61 return xstrfmt("%s%s", r->rewrite[longest_i]->base, url + longest->len);
62 }
@@ -76,15 +76,16 @@ static void add_pushurl(struct remote *remote, const char *pushurl)
76 static void add_pushurl_alias(struct remote_state *remote_state,
77 struct remote *remote, const char *url)
78 {
79 - const char *pushurl = alias_url(url, &remote_state->rewrites_push);
80 - if (pushurl != url)
81 - add_pushurl(remote, pushurl);
79 + char *alias = alias_url(url, &remote_state->rewrites_push);
80 + if (alias)
81 + add_pushurl(remote, alias);
82 }
83
84 static void add_url_alias(struct remote_state *remote_state,
85 struct remote *remote, const char *url)
86 {
87 - add_url(remote, alias_url(url, &remote_state->rewrites));
87 + char *alias = alias_url(url, &remote_state->rewrites);
88 + add_url(remote, alias ? alias : url);
89 add_pushurl_alias(remote_state, remote, url);
90 }
91
@@ -492,19 +493,22 @@ static void alias_all_urls(struct remote_state *remote_state)
493 if (!remote_state->remotes[i])
494 continue;
495 for (j = 0; j < remote_state->remotes[i]->pushurl_nr; j++) {
495 - remote_state->remotes[i]->pushurl[j] =
496 - alias_url(remote_state->remotes[i]->pushurl[j],
497 - &remote_state->rewrites);
496 + char *alias = alias_url(remote_state->remotes[i]->pushurl[j],
497 + &remote_state->rewrites);
498 + if (alias)
499 + remote_state->remotes[i]->pushurl[j] = alias;
500 }
501 add_pushurl_aliases = remote_state->remotes[i]->pushurl_nr == 0;
502 for (j = 0; j < remote_state->remotes[i]->url_nr; j++) {
503 + char *alias;
504 if (add_pushurl_aliases)
505 add_pushurl_alias(
506 remote_state, remote_state->remotes[i],
507 remote_state->remotes[i]->url[j]);
505 - remote_state->remotes[i]->url[j] =
506 - alias_url(remote_state->remotes[i]->url[j],
508 + alias = alias_url(remote_state->remotes[i]->url[j],
509 &remote_state->rewrites);
510 + if (alias)
511 + remote_state->remotes[i]->url[j] = alias;
512 }
513 }
514 }