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

Re: [PATCH 1/9] cache-tree: clean up cache_tree_update()

From
Derrick Stolee <stolee@gmail.com>
Date
Jan 20, 2021, 19:10 UTC
Message-ID
<bbed0d73-fa9d-92a0-3bc4-1d44263bb2fb@gmail.com>
In-Reply-To
<CABPp-BFiNvPaAffQ0uBfpkHY1WM2gR3ssvpwzxpDhrZmZZjiDA@mail.gmail.com>
On 1/20/2021 12:21 PM, Elijah Newren wrote:
Show 19 quoted lines
> On Wed, Jan 20, 2021 at 8:54 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:...
>> +
>> +       if (!istate->cache_tree)
>> +               istate->cache_tree = cache_tree();
> 
> This is the only substantive change.  It seems fairly innocuous, but
> it makes me wonder the reasoning...I don't know/remember enough about
> cache_tree handling to know when this would or wouldn't have already
> been allocated.  It seems that this would have had to segfault below
> if istate->cache_tree were ever NULL, and I don't see you mentioning
> any bug you are fixing, so I presume this means you are going to be
> adding new codepaths somewhere that cause this function to be reached
> under different circumstances than previously had been and you need it
> to be more safe for those.  Is that correct?  Or is it just an
> abundance of caution thing that you're adding?  If the latter, any
> reason you chose to allocate one rather than assume it's a violation
> of design invariants and BUG() instead?  (Perhaps the commit message
> could add a sentence about the rationale for the extra safety?)

It's something I need in the future when I use the cache_tree_update() in more places. I think I call it two times, and either I need to initialize the cache_tree member outside of both, or just make it a feature of the method that it will re-initialize the cache-tree.

Note: the implementation treats an initialized, but empty cache-tree
as "invalid" so update_one() correctly populates the full tree.

Thanks, -Stolee

Previous: Elijah NewrenNext: Derrick Stolee via GitGitGadget
Message 4 of 65 in “More index cleanups”
  1. 0/9 More index cleanupsDerrick Stolee via GitGitGadget, Jan 20, 2021
  2. 1/9 cache-tree: clean up cache_tree_update()Derrick Stolee via GitGitGadget, Jan 20, 2021
  3. Elijah NewrenJan 20, 2021
  4. Derrick StoleeJan 20, 2021
  5. 3/9 fsmonitor: de-duplicate BUG()s around dirty bitsDerrick Stolee via GitGitGadget, Jan 20, 2021
  6. Elijah NewrenJan 20, 2021
  7. Chris TorekJan 21, 2021
  8. Derrick StoleeJan 21, 2021
  9. 4/9 repository: add repo reference to index_stateDerrick Stolee via GitGitGadget, Jan 20, 2021
  10. Elijah NewrenJan 20, 2021
  11. Derrick StoleeJan 20, 2021
  12. Elijah NewrenJan 20, 2021
  13. 5/9 name-hash: use trace2 regions for initDerrick Stolee via GitGitGadget, Jan 20, 2021
  14. Elijah NewrenJan 20, 2021
  15. 6/9 sparse-checkout: load sparse-checkout patternsDerrick Stolee via GitGitGadget, Jan 20, 2021
  16. Elijah NewrenJan 20, 2021
  17. 7/9 sparse-checkout: hold pattern list in indexDerrick Stolee via GitGitGadget, Jan 20, 2021
  18. Elijah NewrenJan 20, 2021
  19. Derrick StoleeJan 20, 2021
  20. 8/9 test-lib: test_region looks for trace2 regionsDerrick Stolee via GitGitGadget, Jan 20, 2021
  21. Elijah NewrenJan 20, 2021
  22. Derrick StoleeJan 20, 2021
  23. 2/9 cache-tree: extract subtree_pos()Derrick Stolee via GitGitGadget, Jan 20, 2021
  24. Elijah NewrenJan 20, 2021
  25. 9/9 t1092: test interesting sparse-checkout scenariosDerrick Stolee via GitGitGadget, Jan 20, 2021
  26. Elijah NewrenJan 20, 2021
  27. Derrick StoleeJan 21, 2021
  28. 0/8 More index cleanupsDerrick Stolee via GitGitGadget, Jan 22, 2021
  29. 7/8 test-lib: test_region looks for trace2 regionsDerrick Stolee via GitGitGadget, Jan 22, 2021
  30. Junio C HamanoJan 22, 2021
  31. Derrick StoleeJan 23, 2021
  32. Junio C HamanoJan 23, 2021
  33. 8/8 t1092: test interesting sparse-checkout scenariosDerrick Stolee via GitGitGadget, Jan 22, 2021
  34. 6/8 sparse-checkout: load sparse-checkout patternsDerrick Stolee via GitGitGadget, Jan 22, 2021
  35. 4/8 repository: add repo reference to index_stateDerrick Stolee via GitGitGadget, Jan 22, 2021
  36. Junio C HamanoJan 22, 2021
  37. 5/8 name-hash: use trace2 regions for initDerrick Stolee via GitGitGadget, Jan 22, 2021
  38. 3/8 fsmonitor: de-duplicate BUG()s around dirty bitsDerrick Stolee via GitGitGadget, Jan 22, 2021
  39. Junio C HamanoJan 22, 2021
  40. 2/8 cache-tree: extract subtree_pos()Derrick Stolee via GitGitGadget, Jan 22, 2021
  41. 1/8 cache-tree: clean up cache_tree_update()Derrick Stolee via GitGitGadget, Jan 22, 2021
  42. Junio C HamanoJan 22, 2021
  43. Elijah NewrenJan 22, 2021
  44. Derrick StoleeJan 23, 2021
  45. 0/9 More index cleanupsDerrick Stolee via GitGitGadget, Jan 23, 2021
  46. 3/9 cache-tree: extract subtree_pos()Derrick Stolee via GitGitGadget, Jan 23, 2021
  47. 2/9 cache-tree: simplify verify_cache() prototypeDerrick Stolee via GitGitGadget, Jan 23, 2021
  48. Elijah NewrenJan 23, 2021
  49. Derrick StoleeJan 23, 2021
  50. Elijah NewrenJan 23, 2021
  51. Junio C HamanoJan 23, 2021
  52. Junio C HamanoJan 23, 2021
  53. Derrick StoleeJan 23, 2021
  54. 4/9 fsmonitor: de-duplicate BUG()s around dirty bitsDerrick Stolee via GitGitGadget, Jan 23, 2021
  55. 1/9 cache-tree: clean up cache_tree_update()Derrick Stolee via GitGitGadget, Jan 23, 2021
  56. 5/9 repository: add repo reference to index_stateDerrick Stolee via GitGitGadget, Jan 23, 2021
  57. 6/9 name-hash: use trace2 regions for initDerrick Stolee via GitGitGadget, Jan 23, 2021
  58. 8/9 test-lib: test_region looks for trace2 regionsDerrick Stolee via GitGitGadget, Jan 23, 2021
  59. Derrick StoleeJan 23, 2021
  60. 7/9 sparse-checkout: load sparse-checkout patternsDerrick Stolee via GitGitGadget, Jan 23, 2021
  61. 9/9 t1092: test interesting sparse-checkout scenariosDerrick Stolee via GitGitGadget, Jan 23, 2021
  62. Elijah NewrenJan 25, 2021
  63. Elijah NewrenJan 23, 2021
  64. Derrick StoleeJan 23, 2021
  65. Junio C HamanoJan 23, 2021

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.