Re: Re*: [PATCH] change contract between system_path and it's callers

2014-11-25 Thread Junio C Hamano
Alexander Kuleshov kuleshovm...@gmail.com writes: But if we still static const char* for git_etc_gitattributes and will not free it in bootstrap_attr_stack there will no memory leak, just tested it, so i'll remove free for it. Leak or no leak, freeing there is wrong. It will free the piece

Re: Re*: [PATCH] change contract between system_path and it's callers

2014-11-25 Thread Alexander Kuleshov
Hello Junio, I'm working on this patch and builtin/config.c, i have a little question: Look, here - https://github.com/git/git/blob/master/builtin/config.c#L509 and in the next if clauses we get allocated path, but if i test it with given config file with git config -f

Re: Re*: [PATCH] change contract between system_path and it's callers

2014-11-25 Thread Alexander Kuleshov
Ah, you little ahead me, so we do not care about config.c in this way, because it takes git_etc_gitconfig() in the same way as git_etc_gitattributes 2014-11-25 23:55 GMT+06:00 Junio C Hamano gits...@pobox.com: Alexander Kuleshov kuleshovm...@gmail.com writes: But if we still static const char*

Re: Re*: [PATCH] change contract between system_path and it's callers

2014-11-25 Thread Junio C Hamano
Alexander Kuleshov kuleshovm...@gmail.com writes: Ah, you little ahead me, so we do not care about config.c in this way, because it takes git_etc_gitconfig() in the same way as git_etc_gitattributes Yes, but looking at the file, you would notice that the use_local_config codepath assigns a

Re: Re*: [PATCH] change contract between system_path and it's callers

2014-11-24 Thread Alex Kuleshov
Junio C Hamano gits...@pobox.com @ 2014-11-25 01:33 ALMT: -static const char *git_etc_gitattributes(void) +static char *git_etc_gitattributes(void) Hmph, I think this should keep returning const char *, as the caller is not expected to free the pointer or write into the memory held by the

Re: Re*: [PATCH] change contract between system_path and it's callers

2014-11-24 Thread Junio C Hamano
Alex Kuleshov kuleshovm...@gmail.com writes: Junio C Hamano gits...@pobox.com @ 2014-11-25 01:33 ALMT: ... if (git_attr_system()) { - elem = read_attr_from_file(git_etc_gitattributes(), 1); + char *etc_attributes = git_etc_gitattributes(); + elem =

Re: Re*: [PATCH] change contract between system_path and it's callers

2014-11-24 Thread Junio C Hamano
Alex Kuleshov kuleshovm...@gmail.com writes: One thing to note is that this illustration does not consider memory pointed at by the system_wide variable here (from attr.c) static const char *git_etc_gitattributes(void) { static const char *system_wide;

Re: Re*: [PATCH] change contract between system_path and it's callers

2014-11-24 Thread Alexander Kuleshov
Hello Junio, I returned static to git_etc_gitattributes return value, but now i can't understand where to free 'system_wide'. As i understand correctly attr.c has following API for querying attributes: git_attr git_check_attr And as i see in code git_attr doesn't use git_attr_check, so now we

Re: Re*: [PATCH] change contract between system_path and it's callers

2014-11-24 Thread Alexander Kuleshov
But if we still static const char* for git_etc_gitattributes and will not free it in bootstrap_attr_stack there will no memory leak, just tested it, so i'll remove free for it. 2014-11-25 12:45 GMT+06:00 Alexander Kuleshov kuleshovm...@gmail.com: Hello Junio, I returned static to