Re: Re*: [PATCH] change contract between system_path and it's callers
- From
Alexander Kuleshov <kuleshovmail@gmail.com>
- Date
- Nov 25, 2014, 17:59 UTC
- Message-ID
- <CANCZXo53s4X4sZbQ1Tj3bVinbPVTodMvGiJR6kevYtZKkJvPYg@mail.gmail.com>
- In-Reply-To
- <CANCZXo4C_6Zfob9VnxjGxPbsRnUioVqC10z3Hhv09_xtrx-Pog@mail.gmail.com>
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.... (https://github.com/git/git/blob/master/builtin/config.c#L513) i have prefix NULL and next if clause doesn't execute, so if i'll put free(given_config_source.file) after config action i'll get error because given_config_source.file wasn't allocated. How to be with this?
Thank you.
2014-11-25 13:04 GMT+06:00 Alexander Kuleshov <kuleshovmail@gmail.com>:
Show 62 quoted lines
> 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 <kuleshovmail@gmail.com>:
>> 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
>> have only git_check_attr and git_all_stars functions which are through
>> prepare_attr_stack and bootstrap_attr_stack in it uses path which
>> returned
>> by system_path. So you're right if i free etc_attributes like this
>> and git_etc_gitattributes will return static value we'll have access
>> to freed memory if it will called again. But we have no access to
>> etc_attributes outside bootstrap_attr_stack so where will be best
>> place to free it?
>>
>> Thank you
>>
>> 2014-11-25 2:50 GMT+06:00 Junio C Hamano <gitster@pobox.com>:
>>> Alex Kuleshov <kuleshovmail@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;
>>>>> if (!system_wide)
>>>>> system_wide = system_path(ETC_GITATTRIBUTES);
>>>>> return system_wide;
>>>>> }
>>>>>
>>>>> at the point of process exit as a "leak".
>>>>
>>>> But why? We allocated memory to "system_wide" with system_path, next git
>>>> will exit somewhere with die, but system_wide didn't free... Or i'm
>>>> wrong here too?
>>>
>>> It is in the same league as "static const char *git_dir" and friends
>>> that appear in the file-scope-static of environment.c. Keeping small
>>> things around to be cleaned up by exit() is not a crime.
>>
>>
>>
>> --
>> _________________________
>> 0xAX
>
>
>
> --
> _________________________
> 0xAX-- _________________________ 0xAX