Re: [PATCH v2 4/9] cache-tree: avoid strtol() on non-string buffer

From: Jeff King <peff@peff.net>
Date: Nov 26, 2025, 15:09 UTC
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:

Show 11 quoted lines

> 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).

Show 7 quoted lines

> 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