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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 22, 2023 at 11:01:37AM -0800, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@xxxxxx> writes:
>
> Just this part.
>
> > 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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux