Skip to content

Commit

Permalink
Fix asserts in fast-path locking code
Browse files Browse the repository at this point in the history
Commit c4d5cb7 introduced a couple asserts in the fast-path locking
code, upsetting Coverity.

The assert in InitProcGlobal() is clearly wrong, as it assigns instead
of checking the value. This is harmless, but doesn't check anything.

The asserts in FAST_PATH_ macros are written as if for signed values,
but the macros are only called for unsigned ones. That makes the check
for (val >= 0) useless. Checks written as ((uint32) x < max) work for
both signed and unsigned values. Negative values should wrap to values
greater than INT32_MAX.

Per Coverity, report by Tom Lane.

Reported-by: Tom Lane
Discussion: https://postgr.es/m/2891628.1727019959@sss.pgh.pa.us
  • Loading branch information
tvondra committed Sep 23, 2024
1 parent 40708ac commit a7e5237
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
8 changes: 4 additions & 4 deletions src/backend/storage/lmgr/lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -218,19 +218,19 @@ int FastPathLockGroupsPerBackend = 0;
* of fast-path lock slots.
*/
#define FAST_PATH_SLOT(group, index) \
(AssertMacro(((group) >= 0) && ((group) < FastPathLockGroupsPerBackend)), \
AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_GROUP)), \
(AssertMacro((uint32) (group) < FastPathLockGroupsPerBackend), \
AssertMacro((uint32) (index) < FP_LOCK_SLOTS_PER_GROUP), \
((group) * FP_LOCK_SLOTS_PER_GROUP + (index)))

/*
* Given a slot index (into the whole per-backend array), calculated using
* the FAST_PATH_SLOT macro, split it into group and index (in the group).
*/
#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))
#define FAST_PATH_INDEX(index) \
(AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_BACKEND)), \
(AssertMacro((uint32) (index) < FP_LOCK_SLOTS_PER_BACKEND), \
((index) % FP_LOCK_SLOTS_PER_GROUP))

/* Macros for manipulating proc->fpLockBits */
Expand Down
2 changes: 1 addition & 1 deletion src/backend/storage/lmgr/proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ InitProcGlobal(void)
}

/* Should have consumed exactly the expected amount of fast-path memory. */
Assert(fpPtr = fpEndPtr);
Assert(fpPtr == fpEndPtr);

/*
* Save pointers to the blocks of PGPROC structures reserved for auxiliary
Expand Down

0 comments on commit a7e5237

Please sign in to comment.