Re: [PATCH 2/2] remote: fix trivial memory leak

Jeff King [off-list ref] writes:

I wondered if we might also leak when seeing duplicate config options
(i.e., leaking the old one when replacing it with the new). But we don't
actually strdup() the configured remote names, but instead just point
into the "struct branch", which owns the data.

In addition, we do not copy this string to remote->name in make_remote(),
so even if we start allowing destruction of existing remote[], the
resulting code will stay safe.

So I think an even better fix would be:

-- >8 --
Subject: remote: do not copy "origin" string literal

Our default_remote_name starts at "origin", but may be
overridden by the config file. In the former case, we
allocate a new string, but in the latter case, we point to
the remote name in an existing "struct branch".

This gives the variable inconsistent free() semantics (we
are sometimes responsible for freeing the string and
sometimes pointing to somebody else's storage), and causes a
small leak when the allocated string is overridden by
config.

We can fix both by simply dropping the extra copy and
pointing to the string literal.

Noticed-by: Felipe Contreras [off-list ref]
Signed-off-by: Jeff King <redacted>

Sounds sensible. Thanks.