On 9/22/24 17:45, Tom Lane wrote: > Tomas Vondra <to...@vondra.me> writes: >> I've finally pushed this, after many rounds of careful testing to ensure >> no regressions, and polishing. > > Coverity is not terribly happy with this. "Assert(fpPtr = fpEndPtr);" > is very clearly not doing what you presumably intended. The others > look like overaggressive assertion checking. If you don't want those > macros to assume that the argument is unsigned, you could force the > issue, say with > > #define FAST_PATH_GROUP(index) \ > - (AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_BACKEND)), \ > + (AssertMacro((uint32) (index) < FP_LOCK_SLOTS_PER_BACKEND), \ > ((index) / FP_LOCK_SLOTS_PER_GROUP)) >
Ah, you're right. I'll fix those asserts tomorrow. The first is clearly wrong, of course. For the (x >= 0) asserts, doing it this way relies on negative values wrapping to large positive ones, correct? AFAIK it's guaranteed to be a very large value, so it can't accidentally be less than the slot count. regards -- Tomas Vondra