Subject: Re: GIT: [PATCH] exec_cmd: system_path memory leak fix

From: Junio C Hamano <[email-masked]>
Date: Sun, 23 Nov 2014 10:28:17 -0800

0xAX <[email-masked]> writes:

> Hello All,
> I found memory leak at exec_cmd.c in system_path function with valgrind.
> After this patch valgrind doesn't show any memory leaks, but I'm not sure
> that it is the best way to solve this problem. There are a couple of places
> that uses system_path, if this way will be good, i'll make another patches
> to fix this leak.
>
> Waiting for feedback.

It is a bit sad that the callers of the function are prepared for
the returned value to be volatile (that is why the ones that want to
do something else between the time they call it and the time they
use the value returned do make copies), while other callers that
immediately use the returned value do not have to be worried about
freeing it, but with the change you have to force everybody to
remember freeing the returned value.

If you instead limited to your change only to these two points:

 - make "struct strbuf d" static;
 - return d.buf instead of the result of strbuf_detach(&d)

the updated system_path() will keep the promise all the caller
expects from it, i.e. the value out of the function is volatile but
the callers do not have to free it, in all cases, no?

> Signed-off-by: Alex Kuleshov <[email-masked]>

Please have that S-o-b: line (and the same name in GIT_AUTHOR_NAME)
on the patches that we'd use "git am" on, not on the cover letter
;-).  Nom de guerre nobody else recognizes outside your close
friends may be fun, but I feel that it is not quite appropriate to
appear in "git shortlog -s" output on a public project like ours.

Thanks.