Re: [PATCH] exec_cmd: system_path memory leak fix

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

 



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