Thread (2 messages) 2 messages, 2 authors, 2024-01-02

Re: [PATCH] sideband.c: replace int with size_t for clarity

From: Taylor Blau <hidden>
Date: 2024-01-02 22:31:19

On Fri, Dec 22, 2023 at 11:01:37AM -0800, Junio C Hamano wrote:
Torsten Bögershausen [off-list ref] writes:

Just this part.
quoted
Further down, we read
	for (i = 0; i < ARRAY_SIZE(keywords); i++) {

However, a size of an array can never be negative, so that
an unsigned data type is a better choice than a signed.
And, arrays can have more elements than an int can address,
at least in theory.
For a reader it makes more sense, to replace
int i;
with
size_t i;
It is a very good discipline to use size_t to index into an array
whose size is externally controled (e.g., we slurp what the end user
or the server on the other end of the connection gave us into a
piece of memory we allocate) to avoid integer overflows as "int" is
often narrower than "size_t".  But this particular one is a Meh; the
keywords[] is a small hardcoded array whose size and contents are
totally under our control.
I certainly agree in theory, though I've always erred on the side of
always using size_t for indexing into arrays, even if they're small. It
removes a potential pitfall if you are working with an
externally-controlled array and happen to forget to use size_t.

But if there is an existing index variable with type "int", and we can
easily validate that it's small, I probably wouldn't bother changing it
if I was editing nearby code.

Thanks,
Taylor
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help