Taylor Blau <me@xxxxxxxxxxxx> writes:
> The function `find_reference_location()` is used to perform a
> binary search-like function over the contents of a repository's
> `$GIT_DIR/packed-refs` file.
>
> The search it implements is unlike a standard binary search in that the
> records it searches over are not of a fixed width, so the comparison
> must locate the end of a record before comparing it.
>
> Extract the core routine of `find_reference_location()` in order to
> implement a function in the following patch which will find the first
> location in the `packed-refs` file that *doesn't* match the given
> pattern.
>
> The behavior of `find_reference_location()` is unchanged.
The splitting out of this step is rather unfortunate in that the
extra parameter "start" given to cmp_record_to_refname(), together
with a rather curious returning to "-1" when the parameter is false,
are not justified at all by looking at the caller, because the only
caller of the function, find_reference_location_1() always passes
"start" it got from its caller without modifying, and the sole
caller of that passes "1" to "start". IOW, the behaviour of
cmp_record_to_refname() when "start" is false can be any arbitrary
garbage (it could even be BUG("")) and the claim "the behaviour is
unchanged" will still be true.
But that does not help the readers all that much.
So, yes I can agree that this does not introduce any new bug, it is
a mysterious no-op, and why we want to pass different values in "start"
in future steps in order to achieve what is not explained and leaves
the readers frustrated.
I'll stop here for now and come back to the rest laster.
Thanks.
