All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Alexander Kuleshov <kuleshovmail@gmail.com>
Cc: "git\@vger.kernel.org" <git@vger.kernel.org>,
	Jeff King <peff@peff.net>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: Re*: [PATCH] change contract between system_path and it's callers
Date: Tue, 25 Nov 2014 12:20:10 -0800	[thread overview]
Message-ID: <xmqqk32jrq11.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CANCZXo4=D=RPnGeAfcvD0O1hX0B43z1b11gyzjbJYGXzusfFYg@mail.gmail.com> (Alexander Kuleshov's message of "Wed, 26 Nov 2014 00:03:54 +0600")

Alexander Kuleshov <kuleshovmail@gmail.com> writes:

> Ah, you little ahead me, so we do not care about config.c in this way,
> because it takes git_etc_gitconfig() in the same way as
> git_etc_gitattributes

Yes, but looking at the file, you would notice that the
"use_local_config" codepath assigns a string obtained from
git_pathdup() to given_config_source.file, which means that somebody
later in the code path would not know if given_config_source.file is
merely pointing at a piece of memory owned by somebody else
(e.g. came from git_etc_gitconfig()) or if it owns the piece of
memory (e.g. came from git_pathdup()).  In the former case the
memory should never be freed, but not freeing in the latter would
leak the memory.

Assignment to given_config_source.file I see in that function that
borrows (i.e. the current code that does not fee is correct) are:

  - getenv(CONFIG_ENVIRONMENT)
  - NULL ;-) obviously
  - git_etc_gitconfig()
  
All the other assignment to given_config_source.file (including the
ones obtained from a call to home_config_paths()) makes us
responsible to free that piece memory, so they are technically
leaks.

But cmd_config() is a moral equivalent of main() in a typical
program, so it might not be worth worrying about allocating a single
piece of memory that is used throughout the lifetime of that
function and leaving it without freeing.

    

  parent reply	other threads:[~2014-11-25 20:20 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-23 13:56 GIT: [PATCH] exec_cmd: system_path memory leak fix 0xAX
2014-11-23 13:56 ` 0xAX
2014-11-23 14:01   ` 0xAX
2014-11-23 18:51   ` Junio C Hamano
2014-11-23 19:06     ` Alex Kuleshov
2014-11-23 19:19     ` Alex Kuleshov
2014-11-23 19:42       ` Jeff King
2014-11-23 20:07       ` Eric Sunshine
2014-11-23 21:58       ` Junio C Hamano
2014-11-24  7:02         ` Alex Kuleshov
2014-11-24  7:37           ` Junio C Hamano
2014-11-24  8:12             ` Alex Kuleshov
2014-11-24 13:11             ` Alexander Kuleshov
2014-11-24 14:00             ` Alex Kuleshov
2014-11-24 14:07               ` [PATCH] change contract between system_path and it's callers 0xAX
2014-11-24 19:33                 ` Re*: " Junio C Hamano
2014-11-24 19:53                   ` Alex Kuleshov
2014-11-24 20:20                     ` Junio C Hamano
2014-11-24 20:50                     ` Junio C Hamano
2014-11-25  6:45                       ` Alexander Kuleshov
2014-11-25  7:04                         ` Alexander Kuleshov
2014-11-25 17:55                           ` Junio C Hamano
2014-11-25 18:03                             ` Alexander Kuleshov
2014-11-25 18:24                               ` [PATCH 1/1] " Alexander Kuleshov
2014-11-25 21:13                                 ` Junio C Hamano
2014-11-26  3:53                                   ` Alexander Kuleshov
2014-11-26  9:42                                     ` Alexander Kuleshov
2014-11-26 14:00                                       ` Alexander Kuleshov
2014-11-26 17:53                                     ` Junio C Hamano
2014-11-28 13:09                                     ` Philip Oakley
2014-11-25 20:20                               ` Junio C Hamano [this message]
2014-11-25 17:59                           ` Re*: [PATCH] " Alexander Kuleshov
2014-11-23 18:28 ` GIT: [PATCH] exec_cmd: system_path memory leak fix Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqk32jrq11.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=kuleshovmail@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.