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

Re: [PATCH 1/2] commit-graph: clean up leaked memory during write

From
Stefan Beller <sbeller@google.com>
Date
Oct 2, 2018, 22:44 UTC
Message-ID
<CAGZ79kaatUeHEGnW9HXv7Xgsj1Sf_M6hcgQ_5oVAOM85rqn34Q@mail.gmail.com>
In-Reply-To
<20181002223434.GA5588@sigill.intra.peff.net>
Show 13 quoted lines
>
> My preference is to avoid them in the name of simplicity. If you're
> using "make SANITIZE=leak test" to check for leaks, it will skip these
> cases. If you're using valgrind, I think these may be reported as
> "reachable". But that number already isn't useful for finding real
> leaks, because it includes cases like the "foo" above as well as
> program-lifetime globals.
>
> The only argument (IMHO) for such an UNLEAK() is that it annotates the
> location for when somebody later changes the function to "return -1"
> instead of dying. But if we are going to do such annotation, we may as
> well actually call free(), which is what the "return" version will
> ultimately have to do.
Heh, that was part of my reasoning why we'd want to have *something*.
Show 6 quoted lines
> I'd actually be _more_ favorable to calling free() instead of UNLEAK()
> there, but I'm still mildly negative, just because it may go stale (and
> our leak-checking wouldn't usefully notice these cases). Anybody
> converting that die() to a return needs to re-analyze the function for
> what might need to be released (and that includes non-memory bits like
> descriptors, too).

Sounds reasonable, so then the consensus (between Martin, you and me) is to drop the UNLEAK.

Previous: Jeff KingNext: Derrick Stolee
Message 8 of 20 in “Clean up leaks in commit-graph.c”
  1. 0/2 Clean up leaks in commit-graph.cDerrick Stolee via GitGitGadget, Oct 2, 2018
  2. 1/2 commit-graph: clean up leaked memory during writeDerrick Stolee via GitGitGadget, Oct 2, 2018
  3. Martin ÅgrenOct 2, 2018
  4. Stefan BellerOct 2, 2018
  5. Martin ÅgrenOct 2, 2018
  6. Stefan BellerOct 2, 2018
  7. Jeff KingOct 2, 2018
  8. Stefan BellerOct 2, 2018
  9. Derrick StoleeOct 3, 2018
  10. 0/2 commit-graph: more leak fixesMartin Ågren, Oct 3, 2018
  11. 1/2 commit-graph: free `struct packed_git` after closing itMartin Ågren, Oct 3, 2018
  12. 2/2 builtin/commit-graph.c: UNLEAK variablesMartin Ågren, Oct 3, 2018
  13. Derrick StoleeOct 3, 2018
  14. Martin ÅgrenOct 3, 2018
  15. Jeff KingOct 2, 2018
  16. 2/2 commit-graph: reduce initial oid allocationDerrick Stolee via GitGitGadget, Oct 2, 2018
  17. 0/3 Clean up leaks in commit-graph.cDerrick Stolee via GitGitGadget, Oct 3, 2018
  18. 1/3 commit-graph: clean up leaked memory during writeDerrick Stolee via GitGitGadget, Oct 3, 2018
  19. 2/3 builtin/commit-graph.c: UNLEAK variablesMartin Ågren via GitGitGadget, Oct 3, 2018
  20. 3/3 commit-graph: reduce initial oid allocationDerrick Stolee via GitGitGadget, Oct 3, 2018

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.