Re: [PATCH 9/9] commit-reach: use `size_t` to track indices when computing merge bases

[Patrick Steinhardt <ps@pks.im>]

On Thu, Jan 02, 2025 at 08:08:15PM -0600, Justin Tobler wrote:
> On 24/12/27 11:46AM, Patrick Steinhardt wrote:
> > The functions `repo_get_merge_bases_many()` and friends accepts an array
> > of commits as well as a parameter that indicates how large that array
> > is. This parameter is using a signed integer, which leads to a couple of
> > warnings with -Wsign-compare.
> > 
> > Refactor the code to use `size_t` to track indices instead and adapt
> > callers accordingly. While most callers are trivial, there are two
> > callers that require a bit more scrutiny:
> > 
> >   - builtin/merge-base.c:show_merge_base() subtracts `1` from the
> >     `rev_nr` before calling `repo_get_merge_bases_many_dirty()`, so if
> >     the variable was `0` it would wrap. This code is fine though because
> >     its only caller will execute that code only when `argc >= 2`, and it
> >     follows that `rev_nr >= 2`, as well.
> > 
> >   - bisect.ccheck_merge_bases() similarly subtracts `1` from `rev_nr`.
> 
> s/ccheck/check/
> 
> Small typo, but probably not worth rerolling.
> 
> >     Again, there is only a single caller that populates `rev_nr` with
> >     `good_revs.nr`. And because a bisection always requires at least one
> >     good revision it follws that `rev_nr >= 1`.
> > 
> > Mark the file as -Wsign-compare-clean.
> > 
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> 
> Thanks Patrick! I reviewed the series and overall it looks good to me :)

Thanks for your review!

Patrick