On Tue, Jun 20, 2023 at 11:31:01AM -0700, Junio C Hamano wrote:
> In hindsight, I think (1) the first one should probably fail the
> "git log" process (not just the lazy fetch subprocess), and (2)
> there should be an explicit way, e.g. giving an empty string, to
> "clear" the list of .url accumulated so far.
Agreed. I think in general that any "list" config that is not doing
last-one-wins should have let a blank entry reset the list.
> (2) may look something silly like this:
I know you said "silly", but in case anybody wants to follow up on
this...
> diff --git c/remote.c w/remote.c
> index 0764fca0db..ecc146856a 100644
> --- c/remote.c
> +++ w/remote.c
> @@ -64,12 +64,22 @@ static const char *alias_url(const char *url, struct rewrites *r)
>
> static void add_url(struct remote *remote, const char *url)
> {
> + if (!*url) {
> + remote->url_nr = 0;
> + return;
> + }
...it probably needs to free() the entries from 0..url_nr, and then also
free the unused empty-string "url".
But I think there's some questionable memory-ownership stuff going on
here. The main config caller passes in a newly allocated string, handing
over ownership to this list (despite the "const" parameter!). But in
add_url_alias(), we take a single allocated "url" string and put it in
both the "url" and "pushurl" lists. The former goes through alias_url()
which sometimes allocates a new string and sometimes does not.
Probably both of these would be better off as strvecs or string_lists.
-Peff
