Re: [PATCH] exec_cmd: system_path memory leak fix

[Alex Kuleshov <kuleshovmail@gmail.com>]

Jeff King:


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

Hello Jeff, yes as i wrote in previous message i saw that there some
places uses system_path function and I just wanted to find correct way
to solve problem with system_path, in near time i'll check and adapt if
need all of this places for updated system_path.

Eric Sunshine:

>Curious. Did the unit tests pass with this change?

Yes i launched unit tests and there are the same result as in origin/pu.

>Not sure what this change is about. The last couple lines of this function are:
>
>    setenv("PATH", new_path.buf, 1);
>    strbuf_release(&new_path);
>
>which means that the buffer held by the strbuf is being released
>anyhow, whether static or not.

Ah, yes, just a type, will update it.

Junio C Hamano:

>Fixing these callers are done as separate patches, that can be
>applied either before or after this patch.

How to do it better? Update this patch, fix all callers which broken and
concat this patches to one or make separate patches?

Thanks all for feedback.

--
Best regards.
0xAX