On 24/03/22 01:22PM, Patrick Steinhardt wrote: > The `binsearch()` test is somewhat weird in that it doesn't explicitly > spell out its expectations. Instead it does so in a rather ad-hoc way > with some hard-to-understand computations. > > Refactor the test to spell out the needle as well as expected index for > all testcases. This refactoring highlights that the `binsearch_func()` > is written somewhat weirdly to find the first integer smaller than the > needle, not smaller or equal to it. Adjust the function accordingly. > > While at it, rename the callback function to better convey its meaning. > > Signed-off-by: Patrick Steinhardt <ps@xxxxxx> > --- > reftable/basics_test.c | 55 ++++++++++++++++++++++++------------------ > 1 file changed, 31 insertions(+), 24 deletions(-) > > diff --git a/reftable/basics_test.c b/reftable/basics_test.c > index dc1c87c5df..85c4d1621c 100644 > --- a/reftable/basics_test.c > +++ b/reftable/basics_test.c > @@ -12,40 +12,47 @@ license that can be found in the LICENSE file or at > #include "test_framework.h" > #include "reftable-tests.h" > > -struct binsearch_args { > - int key; > - int *arr; > +struct integer_needle_lesseq { > + int needle; > + int *haystack; > }; This is probably just personal preference, but I think `key` and `arr` in this case are a bit more straightforward. I do like that we rename the args to be more specific. Do we want to also append `_args` to denote that it is an argument set? Maybe `integer_lesseq_args`? > > -static int binsearch_func(size_t i, void *void_args) > +static int integer_needle_lesseq(size_t i, void *_args) > { > - struct binsearch_args *args = void_args; > - > - return args->key < args->arr[i]; > + struct integer_needle_lesseq *args = _args; > + return args->needle <= args->haystack[i]; > } -Justin