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

revision.c alters commit object state ; also no cleanup

From
Mike Hommey <mh@glandium.org>
Date
Aug 26, 2019, 08:58 UTC
Message-ID
<20190826085844.ue77hen2z6rggpe5@glandium.org>
Hi,

First, a little context: As you may have noticed, I've recently found a small bunch of memory leaks in different parts of the code base. The reason I did is that git-cinnabar[1] uses libgit.a, and I very recently upgraded its CI to use a slightly more recent version of GCC, which either enabled leak detection by default in ASAN or detect more leaks than before. Anyways, my CI was busted because of leaks detected after the upgrade, which made me look around.

git-cinnabar does make some specific uses of libgit.a that git itself doesn't, and this is where things kind of fall apart:

First, revision.c doesn't come with a function to clear a struct rev_info. I did create a function that does enough cleanup for my own use (which turned out to be not enough), but I'm wondering if git shouldn't itself have one in revision.c (and seeing that there's now e.g. repo_clear, which didn't exist when I started git-cinnabar, I think the answer would be that it should).

Then, revision.c kind of does nasty things to commit objects:
- remove_duplicate_parents alters commit->parents to skip duplicates[2]
- try_to_simplify_commit removes all parents but one[3] or all of them
  [4] in some cases
That's for the case that I did encounter. Maybe there's more.

One problem this causes is that it leaks the commit_list items that used to contain the removed parents.

Another problem is that if something else uses the commits afterwards, those commits parents don't match what you'd get before revision walking. And I don't know how this should be addressed.

Ideas welcome.

Relatedly, the subthread that started at <xmqqlfvlne3k.fsf@gitster-ct.c.googlers.com> is kind of relevant. There are two main reasons I'm using libgit.a: one is that I'm doing things low-level enough that I don't know any other library that would work for my use case. Another is that I was hoping that in the long term, it could be merged into git.git. I guess the more time passes, the less likely the latter is to happen. Which is fine, but it's good to know.

Mike
1. https://github.com/glandium/git-cinnabar/
2. https://github.com/git/git/blob/745f6812895b31c02b29bdfe4ae8e5498f776c26/revision.c#L2742
3. https://github.com/git/git/blob/745f6812895b31c02b29bdfe4ae8e5498f776c26/revision.c#L869
4. https://github.com/git/git/blob/745f6812895b31c02b29bdfe4ae8e5498f776c26/revision.c#L888
Next: Junio C Hamano
Message 1 of 3 in “revision.c alters commit object state ; also no cleanup”
  1. Mike HommeyAug 26, 2019
  2. Junio C HamanoAug 26, 2019
  3. Mike HommeyAug 27, 2019

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.