Re: [PATCH 2/2] remote: fix trivial memory leak
- From
Jeff King <peff@peff.net>
- Date
- Sep 24, 2013, 05:19 UTC
- Message-ID
- <20130924051908.GG2766@sigill.intra.peff.net>
- In-Reply-To
- <1379772563-11000-3-git-send-email-felipe.contreras@gmail.com>
On Sat, Sep 21, 2013 at 09:09:23AM -0500, Felipe Contreras wrote:
Show 19 quoted lines
> diff --git a/remote.c b/remote.c
> index efcba93..654e7f5 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -480,7 +480,6 @@ static void read_config(void)
> int flag;
> if (default_remote_name) /* did this already */
> return;
> - default_remote_name = xstrdup("origin");
> current_branch = NULL;
> head_ref = resolve_ref_unsafe("HEAD", sha1, 0, &flag);
> if (head_ref && (flag & REF_ISSYMREF) &&
> @@ -489,6 +488,8 @@ static void read_config(void)
> make_branch(head_ref + strlen("refs/heads/"), 0);
> }
> git_config(handle_config, NULL);
> + if (!default_remote_name)
> + default_remote_name = xstrdup("origin");
> alias_all_urls();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.
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@gmail.com> Signed-off-by: Jeff King <peff@peff.net> --- 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) &&
-- 1.8.4.rc3.19.g9da5bf6