Re: [PATCH] exec_cmd: system_path memory leak fix

From: Jeff King <peff@peff.net>
Date: Nov 23, 2014, 19:42 UTC
Message-ID: <20141123194247.GA16605@peff.net>
In-Reply-To: <87sih9en65.fsf@gmail.com>

On Mon, Nov 24, 2014 at 01:19:29AM +0600, Alex Kuleshov wrote:

Show 27 quoted lines

> 
> Signed-off-by: Alex Kuleshov <kuleshovmail@gmail.com>
> ---
>  exec_cmd.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/exec_cmd.c b/exec_cmd.c
> index 698e752..7ed9bcc 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -13,7 +13,7 @@ const char *system_path(const char *path)
>  #else
>  	static const char *prefix = PREFIX;
>  #endif
> -	struct strbuf d = STRBUF_INIT;
> +	static struct strbuf d = STRBUF_INIT;
> 
>  	if (is_absolute_path(path))
>  		return path;
> @@ -34,8 +34,7 @@ const char *system_path(const char *path)
>  #endif
> 
>  	strbuf_addf(&d, "%s/%s", prefix, path);
> -	path = strbuf_detach(&d, NULL);
> -	return path;
> +	return d.buf;
>  }

If I am reading this right, calls to system_path() will always reuse the same buffer, even if they are called with another "path" argument. So all callers must make sure to make a copy if they are going to hold on to it for a long time. Grepping for callers shows us saving the result to a static variable in at least git_etc_gitattributes, copy_templates, and get_html_page_path. Don't these all need to learn to xstrdup the return value?

-Peff