From: Elia Pinto <gitter.spiros@gmail.com>
Date: Sat, 31 Oct 2026 10:16:19 GMT
Subject: Re: [PATCH v2 4/4] convert: use size_t for text statistics and ident counts
Message-ID: <20261031101619.172770-2-gitter.spiros@gmail.com>
In-Reply-To: <auSDKKHJONZjx2LX@pks.im>

On Fri, Oct 30, 2026 at 12:44:40PM +0100, Patrick Steinhardt wrote:
> > gather_stats() counts NULs, CRs, LFs, CRLFs and (non)printable
> > characters in "unsigned" fields of struct text_stat, and
> > count_ident() counts "$Id$" markers in an "int". Now that the buffer
> > sizes they work on are size_t, a large enough buffer overflows them.
>
> This sounds a bit too deterministic: it doesn't necessarily overflow
> them, but the possibility exists in theory.

Indeed, it's a theoretical possibility rather than something that
automatically happens. In v3 I've adjusted it to say "large inputs can
overflow these counters".

>
> > Unlike the previous patches, this is not specific to LLP64: "unsigned"
> > [...]
> > as needed.
>
> These paragraphs very much read like AI to me. I would appreciate a
> human touch, also to demonstrate that a human was involved between us
> and the AI that tries to understand what was done.

You're completely right. I used AI tooling to help draft that commit
message, and it definitely shows. Fair cop.

To clarify my involvement: I review the code, check the cases, run the
tests, and take full responsibility for what gets submitted. But the
wording here missed the mark. For v3 I rewrote the message myself,
keeping it much shorter, focusing on a single example (lone LFs), and
dropping the heavy-handed explanations.

>
> > Use size_t for all of them, and compute the size hint in
> > ident_to_worktree() with st_add() and st_mult(), so that it dies
> > instead of wrapping around.
>
> That's awfully specific.

Agreed, it spells out the implementation details which are already
obvious from the diff. In v3 I've simplified it to just state that the
size hint is checked for overflow.

>
> The patch itself looks good to me, thanks.

Thanks for the careful review! v3 will follow shortly in the same
thread.

Elia