git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Alexander Kuleshov <kuleshovmail@gmail.com>
Date
Nov 25, 2014, 06:45 UTC
Message-ID
<CANCZXo7yDJCuhKVFG3QfSSoem+KN_9VCbGerTd+5tqQuzA7dbg@mail.gmail.com>
In-Reply-To
<xmqqbnnwwcg0.fsf@gitster.dls.corp.google.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>:
Show 22 quoted lines
> 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
Previous: Junio C HamanoNext: Alexander Kuleshov
Message 20 of 33 in “GIT: [PATCH] exec_cmd: system_path memory leak fix”
  1. 0xAXNov 23, 2014
  2. exec_cmd: system_path memory leak fix0xAX, Nov 23, 2014
  3. 0xAXNov 23, 2014
  4. Junio C HamanoNov 23, 2014
  5. Alex KuleshovNov 23, 2014
  6. Alex KuleshovNov 23, 2014
  7. Jeff KingNov 23, 2014
  8. Eric SunshineNov 23, 2014
  9. Junio C HamanoNov 23, 2014
  10. Alex KuleshovNov 24, 2014
  11. Junio C HamanoNov 24, 2014
  12. Alex KuleshovNov 24, 2014
  13. Alexander KuleshovNov 24, 2014
  14. Alex KuleshovNov 24, 2014
  15. change contract between system_path and it's callers0xAX, Nov 24, 2014
  16. Re*: [PATCH] change contract between system_path and it's callersJunio C Hamano, Nov 24, 2014
  17. Alex KuleshovNov 24, 2014
  18. Junio C HamanoNov 24, 2014
  19. Junio C HamanoNov 24, 2014
  20. Alexander KuleshovNov 25, 2014
  21. Alexander KuleshovNov 25, 2014
  22. Junio C HamanoNov 25, 2014
  23. Alexander KuleshovNov 25, 2014
  24. 1/1 change contract between system_path and it's callersAlexander Kuleshov, Nov 25, 2014
  25. Junio C HamanoNov 25, 2014
  26. Alexander KuleshovNov 26, 2014
  27. Alexander KuleshovNov 26, 2014
  28. Alexander KuleshovNov 26, 2014
  29. Junio C HamanoNov 26, 2014
  30. Philip OakleyNov 28, 2014
  31. Junio C HamanoNov 25, 2014
  32. Alexander KuleshovNov 25, 2014
  33. Junio C HamanoNov 23, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.