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

From
Patrick Steinhardt <hidden>
Date
2026-10-30 11:44:46
Reply to
Elia Pinto, 2026-10-30
On Fri, Oct 30, 2026 at 10:58:53AM +0100, Elia Pinto 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.
Unlike the previous patches, this is not specific to LLP64: "unsigned"
is 32 bits wide on all the platforms we care about.

The counters only drive decisions, but a wrapped counter makes the
wrong one. For example, a file with exactly 2^32 lone LFs gets
lonelf == 0, so will_convert_lf_to_crlf() decides there is nothing
to convert, and a wrapped "printable" or "nonprintable" can fool the
heuristic in convert_is_binary(). In count_ident(), going past
INT_MAX is undefined behaviour.

This is not a memory safety problem: the counts are only used as
hints to strbuf_grow() in crlf_to_worktree() and ident_to_worktree(),
and the buffers are then filled with strbuf_add(), which grows them
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.
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.

The patch itself looks good to me, thanks.

Patrick
Thread (24 messages) 24 messages, 3 authors, 23h ago
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help