Re: [PATCH v2 4/4] convert: use size_t for text statistics and ident counts

From: Elia Pinto <gitter.spiros@gmail.com>
Date: Oct 31, 2026, 10:16 UTC
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:

Show 7 quoted lines

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

Show 8 quoted lines

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

Show 6 quoted lines

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