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@xxxxxx> > > Thanks Patrick! I reviewed the series and overall it looks good to me :) Thanks for your review! Patrick
