On Mon, Nov 06, 2023 at 11:45:53AM +0100, Patrick Steinhardt wrote:
> We're setting up the Git directory twice in the `test-tool bloom`
> helper, once at the beginning of `cmd_bloom()` and once in the local
> subcommand implementation `get_bloom_filter_for_commit()`. This can lead
> to memory leaks as we'll overwrite variables of `the_repository` with
> newly allocated data structures. On top of that it's simply unnecessary.
>
> Fix this by only setting up the Git directory once.
Makes sense. This situation was created by 094a685cd7 (t: make
test-bloom initialize repository, 2020-07-29), which added the setup
call at the start of the program.
That commit closed the door to running test-bloom outside of a
repository for sub-commands that could handle it (perhaps the murmur3
one, but I didn't test). So there are two possible directions here:
- drop the call in cmd__bloom() and make sure all of the relevant
sub-command functions do the setup.
- drop the one command-specific one (i.e., your patch)
In practice I don't think anybody cares about running this test-helper
outside of a repository, and having to do setup in each sub-command is
an extra maintenance burden. So I think your patch is the best
direction.
-Peff
