Re: [PATCH v2 01/13] reftable/block: use `size_t` to track restart point index

From: Patrick Steinhardt <ps@pks.im>
Date: May 22, 2024, 07:23 UTC
Message-ID: <Zk2ddTsVLXbjDmmh@tanuki>
In-Reply-To: <CAOLa=ZQb3if7vcvRcOm8qHSMGo=dSv0nsjCk0R_hhRWU_d7wfw@mail.gmail.com>

On Tue, May 21, 2024 at 01:34:48PM +0000, Karthik Nayak wrote:

Show 12 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > The function `block_reader_restart_offset()` gets the offset of the
> > `i`th restart point. `i` is a signed integer though, which is certainly
> > not the correct type to track indices like this. Furthermore, both
> > callers end up passing a `size_t`.
> >
> > Refactor the code to use a `size_t` instead.
> 
> More of a question for my understanding: Why use `size_t` vs `uint16_t`
> here? I'm asking since the restart count is defined as `uint16_t
> restart_count` in `struct block_reader`.

Mostly because all callers already use `size_t`, and it's customary to use them when talking about indices. We could use `uint16_t`, too, but it didn't really feel worth it to adjust all callers.

Patrick