Jeff King <peff@xxxxxxxx> 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 <felipe.contreras@xxxxxxxxx>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
Sounds sensible. Thanks.
> ---
> remote.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/remote.c b/remote.c
> index e9fedfa..9f1a8aa 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -483,7 +483,7 @@ static void read_config(void)
> int flag;
> if (default_remote_name) /* did this already */
> return;
> - default_remote_name = xstrdup("origin");
> + default_remote_name = "origin";
> current_branch = NULL;
> head_ref = resolve_ref_unsafe("HEAD", sha1, 0, &flag);
> if (head_ref && (flag & REF_ISSYMREF) &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html