On 8/3/2020 2:57 PM, Taylor Blau wrote: > 'get_bloom_filter' takes a flag to control whether it will compute a > Bloom filter if the requested one is missing. In the next patch, we'll > add yet another flag to this method, which would force all but one > caller to specify an extra 'NULL' parameter at the end. > > Instead of doing this, split 'get_bloom_filter' into two functions: > 'get_bloom_filter' and 'get_or_compute_bloom_filter'. The former only > looks up a Bloom filter (and does not compute one if it's missing, > thus dropping the 'compute_if_not_present' flag). The latter does > compute missing Bloom filters, with an additional parameter to store > whether or not it needed to do so. > > This simplifies many call-sites, since the majority of existing callers > to 'get_bloom_filter' do not want missing Bloom filters to be computed > (so they can drop the parameter entirely and use the simpler version of > the function). > +struct bloom_filter *get_or_compute_bloom_filter(struct repository *r, > + struct commit *c, > + int compute_if_not_present, > + int *computed) Could we further simplify this by letting "computed" be the indicator for whether we should compute the filter? If "computed" is NULL, then we won't compute it directly. This allows us to reduce the "1, NULL)" to "NULL)" in these callers: > + struct bloom_filter *filter = get_or_compute_bloom_filter(ctx->r, c, 1, NULL); > + filter = get_or_compute_bloom_filter(the_repository, c, 1, NULL); Thanks, -Stolee
