Re: [PATCH 2/3] setup_path(): Free temporary buffer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

> @@ -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
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]