Subject: Re: [PATCH v3 1/2] bloom: replace struct bloom_key * with struct bloom_keyvec

From: Junio C Hamano <[email-masked]>
Date: Wed, 02 Jul 2025 11:28:08 -0700

Patrick Steinhardt <[email-masked]> writes:

>> +static inline void fill_bloom_keyvec_key(const char *data, size_t len,
>> +					 struct bloom_keyvec *vec, size_t nr,
>> +					 const struct bloom_filter_settings *settings)
>> +{
>> +	assert(nr < vec->count);
>> +	fill_bloom_key(data, len, &vec->key[nr], settings);
>> +}
>> +
>
> Similarly, this should probably be called `bloom_keyvec_fill_key()`.

If so, a preliminary clean-up patch in front of the series is in
order, as the <bloom.h> header file, without these patches, already
has the follwoing external API functions and structures declared,
that do not follow your naming rules at all (I have removed the ones
that begin with "bloom_" from the below):

    int load_bloom_filter_from_graph()
    uint32_t murmur3_seeded_v2();
    void fill_bloom_key();
    void clear_bloom_key(s);
    void add_key_to_filter();
    void init_bloom_filters(void);
    void deinit_bloom_filters(void);
    struct bloom_filter *get_or_compute_bloom_filter();
    struct bloom_filter *get_bloom_filter();

It is very dubious that murmur3_seeded_v2() is exposed (nobody would
know it is for Bloom filter subsystem from that name); as far as I
can tell, it is only needed for t/helper testing, and makes me
wonder if we can come up with a better division between the
production code and t/helper/ code around there.

Thanks.