On Mon, Nov 3, 2025 at 10:59 PM Melanie Plageman <[email protected]> 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
