Re: [PATCH 18/20] pack-revindex: remove unused 'find_revindex_position()'
- From
Jeff King <peff@peff.net>
- Date
- Jan 13, 2021, 13:05 UTC
- Message-ID
- <X/7wCy92mGPZB9PM@coredump.intra.peff.net>
- In-Reply-To
- <X/3VfteeF3Ok2C+S@nand.local>
On Tue, Jan 12, 2021 at 11:59:42AM -0500, Taylor Blau wrote:
Show 6 quoted lines
> > I notice these are signed ints, but we've taken care to use uint32_t > > elsewhere for positions. Shouldn't these be uint32_t, also (or at least > > unsigned)? > > I'll let both of these be an raw unsigned, since the midpoint is already > labeled as an unsigned.
Yeah, I found that existing code doubly weird, since "mi" must by definition be bounded by "lo" and "hi". :)
Looks like the bug was introduced in 92e5c77c37 (revindex: export new APIs, 2013-10-24), which hacked up the existing loop into a new function. We should have caught it then (back then we were a lot less careful about types, IMHO).
Also, the subject line of that commit is giving me deja vu. ;)
-Peff