Re: [PATCH v2 4/9] cache-tree: avoid strtol() on non-string buffer
- From
Jeff King <peff@peff.net>
- Date
- Nov 24, 2025, 22:30 UTC
- Message-ID
- <20251124223023.GA2051672@coredump.intra.peff.net>
- In-Reply-To
- <xmqqtsylz2xh.fsf@gitster.g>
On Sat, Nov 22, 2025 at 10:19:22PM -0800, Junio C Hamano wrote:
> We could try to be more careful, but it quickly became messy when I > tried. Here is an unfinished attempt of mine.
So yeah, I was hoping to avoid jumping into this rabbit hole of messiness and just do the bare minimum to give us memory safety. But it seems nobody is quite happy with the result. :(
Looking over what you wrote below, it seems pretty reasonable to me. What do you consider unfinished in it? I'm wondering if we should swap it into what my patch is doing (or do it on top if you prefer).
Another option is to scrap this approach entirely, and copy up until the trailing newline into a separate buffer, NUL-terminate it, and parse from that buffer. That feels a little dirty to me, but I suspect it is pretty performant in practice, and it pushes all of the complexity back onto strtol().
Another variant of that is: parse up to the trailing newline, making sure it's there, and then leave the rest of the code as-is. We know that strtol() will do the right thing in that case, but it does mean we cannot use ASan's strict_string_checks (it would still yield a false positive, because it does not know we've checked for the newline).
-Peff