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

[PATCH v3 4/9] fsmonitor: de-duplicate BUG()s around dirty bits

From
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date
Jan 23, 2021, 19:58 UTC
Message-ID
<4e688d25f8c7949dd3edbdb47b924450d9a50796.1611431900.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.839.v3.git.1611431899.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>

The index has an fsmonitor_dirty bitmap that records which index entries are "dirty" based on the response from the FSMonitor. If this bitmap ever grows larger than the index, then there was an error in how it was constructed, and it was probably a developer's bug.

There are several BUG() statements that are very similar, so replace these uses with a simpler assert_index_minimum(). Since there is one caller that uses a custom 'pos' value instead of the bit_size member, we cannot simplify it too much. However, the error string is identical in each, so this simplifies things.

Be sure to add one when checking if a position if valid, since the minimum is a bound on the expected size.

The end result is that the code is simpler to read while also preserving these assertions for developers in the FSMonitor space.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 fsmonitor.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/fsmonitor.c b/fsmonitor.c
index ca031c3abb8..fe9e9d7baf4 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -13,14 +13,19 @@
 
 struct trace_key trace_fsmonitor = TRACE_KEY_INIT(FSMONITOR);
 
+static void assert_index_minimum(struct index_state *istate, size_t pos)
+{
+	if (pos > istate->cache_nr)
+		BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %u)",
+		    (uintmax_t)pos, istate->cache_nr);
+}
+
 static void fsmonitor_ewah_callback(size_t pos, void *is)
 {
 	struct index_state *istate = (struct index_state *)is;
 	struct cache_entry *ce;
 
-	if (pos >= istate->cache_nr)
-		BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" >= %u)",
-		    (uintmax_t)pos, istate->cache_nr);
+	assert_index_minimum(istate, pos + 1);
 
 	ce = istate->cache[pos];
 	ce->ce_flags &= ~CE_FSMONITOR_VALID;
@@ -82,10 +87,8 @@ int read_fsmonitor_extension(struct index_state *istate, const void *data,
 	}
 	istate->fsmonitor_dirty = fsmonitor_dirty;
 
-	if (!istate->split_index &&
-	    istate->fsmonitor_dirty->bit_size > istate->cache_nr)
-		BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %u)",
-		    (uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
+	if (!istate->split_index)
+		assert_index_minimum(istate, istate->fsmonitor_dirty->bit_size);
 
 	trace_printf_key(&trace_fsmonitor, "read fsmonitor extension successful");
 	return 0;
@@ -110,10 +113,8 @@ void write_fsmonitor_extension(struct strbuf *sb, struct index_state *istate)
 	uint32_t ewah_size = 0;
 	int fixup = 0;
 
-	if (!istate->split_index &&
-	    istate->fsmonitor_dirty->bit_size > istate->cache_nr)
-		BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %u)",
-		    (uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
+	if (!istate->split_index)
+		assert_index_minimum(istate, istate->fsmonitor_dirty->bit_size);
 
 	put_be32(&hdr_version, INDEX_EXTENSION_VERSION2);
 	strbuf_add(sb, &hdr_version, sizeof(uint32_t));
@@ -335,9 +336,7 @@ void tweak_fsmonitor(struct index_state *istate)
 			}
 
 			/* Mark all previously saved entries as dirty */
-			if (istate->fsmonitor_dirty->bit_size > istate->cache_nr)
-				BUG("fsmonitor_dirty has more entries than the index (%"PRIuMAX" > %u)",
-				    (uintmax_t)istate->fsmonitor_dirty->bit_size, istate->cache_nr);
+			assert_index_minimum(istate, istate->fsmonitor_dirty->bit_size);
 			ewah_each_bit(istate->fsmonitor_dirty, fsmonitor_ewah_callback, istate);
 
 			refresh_fsmonitor(istate);
-- 
gitgitgadget
Previous: Derrick StoleeNext: Derrick Stolee via GitGitGadget
Message 54 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.