From: Jeff King <peff@peff.net>
Date: Wed, 26 Nov 2025 15:09:31 GMT
Subject: Re: [PATCH v2 4/9] cache-tree: avoid strtol() on non-string buffer
Message-ID: <20251126150931.GC4143292@coredump.intra.peff.net>
In-Reply-To: <xmqqms4buix0.fsf@gitster.g>

On Mon, Nov 24, 2025 at 03:09:47PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Looking over what you wrote below, it seems pretty reasonable to me.
> > What do you consider unfinished in it?
> 
> Two things I am unhappy about are that (1) parsing the digit
> sequence that represents abs(x) into unsigned int while catching
> wraparound and (2) checking if 'val' that has abs(x) would fit in a
> signed int when 'negate' is applied.  For both of them, there ought
> to be a better way to write, and perhaps there may be a clean way to
> do both at the same time that is easier reason about.

Hmm, I thought both of those things were reasonably clever. The other
obvious way to do it, AFAICT, is to used checked-operation intrinsics or
add unsigned_add_overflows() before every operation.

It is true that for the general case of: "x = y + z" or "x = y * z", you
cannot determine overflow strictly from checking that x < y. But I think
given that we know "z" must be small, it works in this case.

It looks like you merged what I had into 'next'. Where do you want to go
from there? I am mostly content to let it be, but we can also try to
replace with something like your version. Or even, I guess, work on a
global strntoi() that could be used everywhere, if we think it is robust
enough. (Though technically that name is reserved by the standard, which
is a shame, because that is really what this thing is).

> Or perhaps introduce cache-tree-version-2 index extension.  If there
> are other things we may want to fix while we are at it, that would
> be a better way to spend our engineering resource, but I offhand do
> not know of anything gravely lacking there that we may want to fix
> (there are little things like how the pathnames are sorted that I
> regret the way it was implemented, but that does not motivate me
> enough).

I read your other email laying out the v2 concept, and I didn't disagree
with anything. It just feels like a bigger engineering effort and a
bigger risk that the transition does not go as smoothly as we expect for
solving a very small implementation problem. But like you say, I do not
have a laundry list of cache-tree things I'd like to fix either. I think
the transition being worth it would depend on that kind of list.

-Peff