GIT: [PATCH] exec_cmd: system_path memory leak fix

2014-11-23 Thread 0xAX
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.

Signed-off-by: Alex Kuleshov kuleshovm...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-11-23 Thread Junio C Hamano
0xAX kuleshovm...@gmail.com 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 kuleshovm...@gmail.com

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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html