On Wed, Jan 20, 2021 at 8:54 AM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>
> 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.
>
> The end result is that the code is simpler to read while also preserving
> these assertions for developers in the FSMonitor space.
Indeed, looking through the patch, the end result is simpler to read.
Nice cleanup.
>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> fsmonitor.c | 27 +++++++++++++--------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/fsmonitor.c b/fsmonitor.c
> index ca031c3abb8..52a50a9545a 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);
>
> 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
>
