On Mon, Nov 3, 2025 at 10:59 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> > Not sure. That changes the posture from "can't happen" to "shouldn't
> > happen, but if it does, don't cause a disaster". Even with the latter,
> > the assert still seems appropriate for catching developer mistakes.
>
> You are suggesting keeping the assert and this line after it?
>
> num_offsets = Min(num_offsets, lengthof(offsets));
My "not sure" was referring to this line.
> The current contract of TidStoreGetBlockOffsets() is that it won't
> return a value larger than max_offsets passed in, so it is a good idea
> to have an assert in case it changes.
I suspect the contract is the way it is in order to enable the assert to work.
> But, if we take the minimum,
> then is the assert there to keep developers from changing
> TidStoreGetBlockOffsets() from behaving differently? I don't know if I
> like that, but I don't feel strongly enough to object. Anyway, I think
> we should add the line Tom suggested.
This line seems strange to me (and maybe even stranger to have both
the min and the assert), but maybe I don't understand Tom's rationale
well enough. Do we need it to silence Coverity?
--
John Naylor
Amazon Web Services
