Re: [PATCH 2/3] setup_path(): Free temporary buffer
- From
Jeff King <peff@peff.net>
- Date
- Mar 14, 2011, 20:09 UTC
- Message-ID
- <20110314200958.GC22602@sigill.intra.peff.net>
- In-Reply-To
- <1300130318-11279-3-git-send-email-cmn@elego.de>
On Mon, Mar 14, 2011 at 08:18:37PM +0100, Carlos Martín Nieto wrote:
> Make sure the pointer git_exec_path() returns is in the heap and free > it after it's no longer needed.
Could you explain the motivation a bit more? From looking at the code, it looks like the situation is that git_exec_path sometimes returns a newly allocated string and sometimes not, so the caller may leak in the former case.
That seems to be the fault of system_path, which sometimes allocates and sometimes does not. Should we also be fixing system_path, which is a source of leaks? Or instead converting system_path to use a static buffer?
Show 5 quoted lines
> @@ -99,10 +99,13 @@ void setup_path(void)
> {
> const char *old_path = getenv("PATH");
> struct strbuf new_path = STRBUF_INIT;
> + char *exec_path = (char *) git_exec_path();Ick. If it is now returning an allocated buffer that the caller is responsible for free-ing, then its declaration should drop the const in the return value.
What about other callers of git_exec_path? Aren't load_command_list and list_commands leaking, too (and possibly worse, since we now always allocated memory)?
-Peff