On Mon, Apr 29, 2024 at 04:43:15PM -0400, Taylor Blau wrote:
> diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
> index 9bc41a9e145..fef02cd745a 100644
> --- a/pack-bitmap-write.c
> +++ b/pack-bitmap-write.c
> @@ -24,7 +24,7 @@ struct bitmapped_commit {
> struct ewah_bitmap *write_as;
> int flags;
> int xor_offset;
> - uint32_t commit_pos;
> + unsigned pseudo_merge : 1;
> };
The addition of the bit flag here makes sense, but dropping commit_pos
caught me by surprise. But...it looks like that flag is simply unused
cruft even before this patch?
It might be worth noting that in the commit message, or better still,
pulling its removal out to a preparatory patch.
> +static inline int bitmap_writer_selected_nr(void)
> +{
> + return writer.selected_nr - writer.pseudo_merges_nr;
> +}
OK, so now most spots should use this new function instead of looking at
writer.selected_nr directly. But if anybody accidentally uses the old
field directly, it is presumably disastrous. Is it worth renaming it to
make sure we caught all references?
The downside would be that spots which _do_ want the complete
selected_nr would need to be updated to use the new name. It doesn't
look like there are that many, though. OTOH, that means that it's also
easy to inspect them and see that you covered all of the relevant cases
(as far as I can see). I guess the biggest value in changing the field
name would be catching any topics in flight (or long-running forks).
-Peff
