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

[PATCH 0/14] combine-diff cleanups

From
Jeff King <peff@peff.net>
Date
Jan 9, 2025, 08:27 UTC
Message-ID
<20250109082723.GA2748497@coredump.intra.peff.net>
In-Reply-To
<20250104033210.GA892381@coredump.intra.peff.net>

Since Wink successfully nerd-sniped me into digging into the combine-diff code, and since I had such a hard time figuring out some of its logic, I spent a little time trying to put that puzzling to good use to make it more readable.

Aside from a minor leak fix in the first patch, I didn't find any bugs. So arguably this whole thing could be discarded as churn. But I hope at least some of it is worthwhile, and I tried to order it to keep the less controversial bits near the top.

The series can be split into a few sections:
  [01/14]: run_diff_files(): delay allocation of combine_diff_path
  [02/14]: combine-diff: add combine_diff_path_new()
  [03/14]: tree-diff: clear parent array in path_appendnew()
  [04/14]: combine-diff: use pointer for parent paths
  [05/14]: diff: add a comment about combine_diff_path.parent.path
  [06/14]: run_diff_files(): de-mystify the size of combine_diff_path struct
    These first six clean up most of the allocation and initialization
    confusion that started this thread. They can't go all the way
    because of the scariness in path_appendnew().
  [07/14]: tree-diff: drop path_appendnew() alloc optimization
  [08/14]: tree-diff: pass whole path string to path_appendnew()
  [09/14]: tree-diff: inline path_appendnew()
  [10/14]: combine-diff: drop public declaration of combine_diff_path_size()
    And these ones take it further, but at the cost of losing an
    optimization in patch 07. I don't think it was doing much (and I
    gave some timings there). But it's a judgement call on whether the
    cleaner code is worthwhile.
  [11/14]: tree-diff: drop list-tail argument to diff_tree_paths()
  [12/14]: tree-diff: use the name "tail" to refer to list tail
  [13/14]: tree-diff: simplify emit_path() list management
  [14/14]: tree-diff: make list tail-passing more explicit
    And these last four fix some confusion I had while reading the
    functions. I think they _could_ be done independent of 7-14,
    but there'd be some kinks to work out in emit_path().
    The final one is probably a matter of taste, and I'm not sure if
    people find it easier to understand than the original or not. If
    not, it can easily be dropped.
 combine-diff.c |  80 +++++++++++++-------------
 diff-lib.c     |  36 ++++--------
 diff.h         |  18 ++++--
 tree-diff.c    | 152 ++++++++++++-------------------------------------
 4 files changed, 102 insertions(+), 184 deletions(-)
-Peff
Previous: Wink SavilleNext: Jeff King
Message 9 of 38 in “[BUGREPORT] git diff-tree --cc SEGFAUTs”
  1. Wink SavilleJan 3, 2025
  2. Jeff KingJan 3, 2025
  3. Wink SavilleJan 3, 2025
  4. Jeff KingJan 4, 2025
  5. Junio C HamanoJan 4, 2025
  6. Jeff KingJan 4, 2025
  7. Wink SavilleJan 4, 2025
  8. Wink SavilleJan 5, 2025
  9. 0/14 combine-diff cleanupsJeff King, Jan 9, 2025
  10. 01/14 run_diff_files(): delay allocation of combine_diff_pathJeff King, Jan 9, 2025
  11. Junio C HamanoJan 9, 2025
  12. 02/14 combine-diff: add combine_diff_path_new()Jeff King, Jan 9, 2025
  13. Junio C HamanoJan 9, 2025
  14. Patrick SteinhardtJan 13, 2025
  15. Jeff KingJan 14, 2025
  16. 03/14 tree-diff: clear parent array in path_appendnew()Jeff King, Jan 9, 2025
  17. Junio C HamanoJan 9, 2025
  18. Jeff KingJan 10, 2025
  19. 04/14 combine-diff: use pointer for parent pathsJeff King, Jan 9, 2025
  20. Junio C HamanoJan 9, 2025
  21. 05/14 diff: add a comment about combine_diff_path.parent.pathJeff King, Jan 9, 2025
  22. Patrick SteinhardtJan 13, 2025
  23. 06/14 run_diff_files(): de-mystify the size of combine_diff_path structJeff King, Jan 9, 2025
  24. Junio C HamanoJan 10, 2025
  25. 07/14 tree-diff: drop path_appendnew() alloc optimizationJeff King, Jan 9, 2025
  26. Patrick SteinhardtJan 13, 2025
  27. Jeff KingJan 14, 2025
  28. 08/14 tree-diff: pass whole path string to path_appendnew()Jeff King, Jan 9, 2025
  29. Patrick SteinhardtJan 13, 2025
  30. Jeff KingJan 14, 2025
  31. 09/14 tree-diff: inline path_appendnew()Jeff King, Jan 9, 2025
  32. Junio C HamanoJan 11, 2025
  33. 10/14 combine-diff: drop public declaration of combine_diff_path_size()Jeff King, Jan 9, 2025
  34. 11/14 tree-diff: drop list-tail argument to diff_tree_paths()Jeff King, Jan 9, 2025
  35. Junio C HamanoJan 18, 2025
  36. 12/14 tree-diff: use the name "tail" to refer to list tailJeff King, Jan 9, 2025
  37. 13/14 tree-diff: simplify emit_path() list managementJeff King, Jan 9, 2025
  38. 14/14 tree-diff: make list tail-passing more explicitJeff King, Jan 9, 2025

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.