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