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

[Junio C Hamano <gitster@pobox.com>]

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.

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