On Wed, Jun 10, 2026 at 10:36:22AM -0400, Andres Freund wrote: > I think it *should* blow up. It doesn't because we're lacking assertions in > GetBufferDescriptor(). But I don't think the assertions added in the patch are > quite right. > > We can't trivially add the correct assertions, because somebody though it was > a good idea to give GetBufferDescriptor() a uint32 parameter, which seems > completely wrong to me.
This one is not as old as I expected: 3ac88fddd92c. You're right that switching that to be signed would be a correct first step forward.
Given the discussion here, I tried making the buffer descriptor accessors take a signed index and adding range assertions.
Most callers already pass signed buffer indexes derived from Buffer values (e.g. buffer - 1, or -buffer - 1 for local buffers). The only caller I found that passes an unsigned value to GetBufferDescriptor() is ClockSweepTick(), and that normalizes the value before returning it, so it should still be less than NBuffers.
The change looked fairly straightforward to me, unless I'm missing something. I also made the same change for GetLocalBufferDescriptor(), since it has the same uint32 signature and takes local buffer indexes.
