Re: [PATCH] exec_cmd: system_path memory leak fix

[Jeff King <peff@peff.net>]

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

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