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

Re: [PATCH 1/5] xdiff: clean up read_mmfile() allocations on error

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 29, 2026, 18:37 UTC
Message-ID
<xmqqbj9fhpkj.fsf@gitster.g>
In-Reply-To
<20260929065131.GA1697497@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 19 quoted lines
> When read_mmfile() returns an error, it may or may not have allocated a
> buffer in the passed-in mmfile_t. So callers must initialize the pointer
> to NULL and free it even on error.
>
> Most callers do this already, but rerere's diff_two() does not, and
> would leak the buffer after a read error. We could fix it directly, but
> let's instead try to make the interface less error-prone by freeing the
> memory when returning failure from read_mmfile().
>
> This fixes (part of) the leak in diff_two(). In theory it also lets us
> simplify other callers to skip initializing the mmfile. But in practice
> most still need zero-initialization because they may jump to free()
> before even calling read_mmfile (e.g., in try_merge()). But we can at
> least simplify rerere_forget_one_path() a bit.
>
> I said "part of" earlier. There's a related leak in diff_two(): if
> reading the first file succeeds but reading the second fails, we return
> early and leak the first buffer. We can fix that by checking each
> individually.
Nice.  Thanks for plugging my leaks.
Previous: Jeff KingNext: Jeff King
Message 3 of 37 in “use size_t for xdiff mmfile_t”
  1. 0/5 use size_t for xdiff mmfile_tJeff King, Sep 29, 2026
  2. 1/5 xdiff: clean up read_mmfile() allocations on errorJeff King, Sep 29, 2026
  3. Junio C HamanoSep 29, 2026
  4. 2/5 xdiff: replace mmbuffer_t with mmfile_tJeff King, Sep 29, 2026
  5. D. Ben KnobleSep 29, 2026
  6. Junio C HamanoSep 29, 2026
  7. Patrick SteinhardtSep 30, 2026
  8. Jeff KingSep 30, 2026
  9. Junio C HamanoOct 1, 2026
  10. 3/5 xdiff: use size_t for buffer sizesJeff King, Sep 29, 2026
  11. 4/5 merge-ll: use read_mmfile() to read external merge resultsJeff King, Sep 29, 2026
  12. Junio C HamanoSep 29, 2026
  13. Jeff KingSep 29, 2026
  14. Jeff KingSep 29, 2026
  15. 6/5 merge-ll: handle external driver status before reading resultJeff King, Sep 29, 2026
  16. 7/5 merge-ll: report an error when reading external merge results failsJeff King, Sep 29, 2026
  17. Junio C HamanoSep 29, 2026
  18. Jeff KingSep 29, 2026
  19. Junio C HamanoSep 30, 2026
  20. Jeff KingSep 30, 2026
  21. Junio C HamanoOct 1, 2026
  22. Patrick SteinhardtSep 30, 2026
  23. Jeff KingSep 30, 2026
  24. 5/5 xdiff: NUL-terminate buffers read by read_mmfile()Jeff King, Sep 29, 2026
  25. Patrick SteinhardtSep 30, 2026
  26. Junio C HamanoSep 30, 2026
  27. Jeff KingSep 30, 2026
  28. 0/7 use size_t for xdiff mmfile_tJeff King, Sep 30, 2026
  29. 1/7 xdiff: clean up read_mmfile() allocations on errorJeff King, Sep 30, 2026
  30. 2/7 xdiff: replace mmbuffer_t with mmfile_tJeff King, Sep 30, 2026
  31. 3/7 xdiff: use size_t for buffer sizesJeff King, Sep 30, 2026
  32. 4/7 xdiff: NUL-terminate buffers read by read_mmfile()Jeff King, Sep 30, 2026
  33. Patrick SteinhardtOct 1, 2026
  34. 5/7 merge-ll: use read_mmfile() to read external merge resultsJeff King, Sep 30, 2026
  35. Patrick SteinhardtOct 1, 2026
  36. 6/7 merge-ll: handle external driver status before reading resultJeff King, Sep 30, 2026
  37. 7/7 merge-ll: report an error when reading external merge results failsJeff King, Sep 30, 2026

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.