Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-24 Thread Alex Kuleshov
Now system_path returns path which is allocated string to callers; It prevents memory leaks in some places. All callers of system_path are owners of path string and they must release it. Added new parameter to wrapper.c/int access_or_die - etc_config, because only etc_config in this case use syst

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-24 Thread Alexander Kuleshov
Now system_path returns path which is allocated string to callers; It prevents memory leaks in some places. All callers of system_path are owners of path string and they must release it. Signed-off-by: Alexander Kuleshov --- attr.c| 8 +++--- builtin/config.c | 73

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-24 Thread Alex Kuleshov
Junio C Hamano @ 2014-11-24 13:37 ALMT: > [jc: added those who were mentioned but were missing back to Cc] > > On Sun, Nov 23, 2014 at 11:02 PM, Alex Kuleshov > wrote: >> >> Junio C Hamano: >> >>>Fixing these callers are done as separate patches, that can be >>>applied either before or after t

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-23 Thread Junio C Hamano
[jc: added those who were mentioned but were missing back to Cc] On Sun, Nov 23, 2014 at 11:02 PM, Alex Kuleshov wrote: > > 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, f

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-23 Thread Alex Kuleshov
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 res

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-23 Thread Junio C Hamano
Alex Kuleshov writes: > Signed-off-by: Alex Kuleshov So this one does not change the contract between the caller and the callee. It does not change the fact that you need to explain your change, though. The story should read something like this: system_path(): always return a volatile re

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-23 Thread Eric Sunshine
On Sun, Nov 23, 2014 at 2:19 PM, Alex Kuleshov wrote: > > Signed-off-by: Alex Kuleshov > --- > 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

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-23 Thread Jeff King
On Mon, Nov 24, 2014 at 01:19:29AM +0600, Alex Kuleshov wrote: > > Signed-off-by: Alex Kuleshov > --- > 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 > @@

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-23 Thread Alex Kuleshov
Signed-off-by: Alex Kuleshov --- 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 c

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-23 Thread Alex Kuleshov
Junio C Hamano @ 2014-11-24 00:51 ALMT: > 0xAX writes: > >> Signed-off-by: 0xAX >> --- > > The comment on names I've already mentioned elsewhere. Yes, i understand about names. > > You need a better explanation than a "no log message", as you are > not doing "system-path memory leak fix". >

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-23 Thread Junio C Hamano
0xAX writes: > Signed-off-by: 0xAX > --- The comment on names I've already mentioned elsewhere. You need a better explanation than a "no log message", as you are not doing "system-path memory leak fix". You are doing a lot more. Perhaps the story would start like this: system_path(): ma

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

2014-11-23 Thread Junio C Hamano
0xAX 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

Re: [PATCH] exec_cmd: system_path memory leak fix

2014-11-23 Thread 0xAX
For example some testing with valgrind: 1. pu (952f0aaadff4afca1497450911587a973c8cca02) valgrind git help ==27034== Memcheck, a memory error detector ==27034== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==27034== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright

[PATCH] exec_cmd: system_path memory leak fix

2014-11-23 Thread 0xAX
Signed-off-by: 0xAX --- exec_cmd.c | 25 - exec_cmd.h | 4 ++-- git.c | 12 +--- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/exec_cmd.c b/exec_cmd.c index 698e752..08f8f80 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -6,7 +6,7 @@ static c

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 anothe