Junio C Hamano <gitster@xxxxxxxxx> writes
>
> Lidong Yan <yldhome2d2@xxxxxxxxx> writes:
>
>> +void bloom_keyvec_init(struct bloom_keyvec *v, size_t initial_size)
>> +{
>> + ALLOC_ARRAY(v->keys, initial_size);
>> + v->nr = initial_size;
>> +}
>
> Hmph, does this ever grow once initialized? When I outlined the
> solution in the earlier discussion, I was wondering a structure that
> looks more like
>
> struct bloom_keyvec {
> size_t count;
> struct bloom_key key[FLEX_ARRAY];
> };
I understand. If we don't need to resize the array dynamically, we should
allocate the array elements and the array length together to gain benefits
such as spatial locality.
> Also when your primary use of an array is to use one element at a
> time (as opposed to the entire array as a single "collection"), name
> it singular, so that key[4] is more naturally understood as "4-th
> key", not keys[4].
Got it.
>
>> +void bloom_keyvec_clear(struct bloom_keyvec *v)
>> +{
>> + size_t i;
>> + if (!v->keys)
>> + return;
>> +
>> + for (i = 0; i < v->nr; i++)
>> + clear_bloom_key(&v->keys[i]);
>
> By doing
>
> for (size_t nr; nr < v->nr; nr++)
>
> you can
>
> - lose the separate local variable definition at the beginning;
> - avoid confusing "i", which hints to be an "int", to be of type "size_t"
> - limit the scope of "nr" a bit tigher.
Got it.
>
> If you make keyvec an fixed flex-array, the below would become
> unnecessary (and the check for NULL-ness of .keys[] array).
>
>> +
>> + FREE_AND_NULL(v->keys);
>> + v->nr = 0;
>> +}
Another benefit to use flex-array.
>
>> +struct bloom_key *bloom_keyvec_at(const struct bloom_keyvec *v, size_t i)
>> +{
>
> Ditto about abusing the name 'i'.
>
>> + return ret;
>> + }
>> +
>> + return 1;
>> +}
>> \ No newline at end of file
>
> Tell your editor to be more careful, perhaps?
Seems git clang-format doesn’t add newline for me, I will try
.editconfig next time.
Thanks,
Lidong
