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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 26, 2025, 17:22 UTC
Message-ID: <xmqqldjsogip.fsf@gitster.g>
In-Reply-To: <20251126150931.GC4143292@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

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

Yup, but the thing is, I didn't want something "clever". I prefer "clean and obvious" if we add extra code for safety.

Show 7 quoted lines

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

That is my preference. While the topic is still in 'next', or after the topic graduates to 'master'. Either is fine. And it is fine if such an update did not come, too. After all, this is to deal with contents in a locally generated file (.git/index), so a maliciously corrupt string that lack the expected whitespace character after the digit string is a sign that you are trying to burn yourself and you have only yourself to blame, isn't it? An attacker that can put garbage in your .git/index has better ways to fool you by updating your .git/config file that sits next to it. Or teach the sanitizer that this code path is already OK somehow?

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

Well, we already use plenty of names beginning with 'str' followed by a lowercase letter, like strbuf_foo() and string_list_init().