From dd74c128e4d98167e921adaee09f21c2cff7a155 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Thu, 8 Dec 2022 14:34:47 +0100 Subject: [PATCH 3/4] Move some static assertions to better places Since the addition of StaticAssertDecl(), we can put static assertions at the file level. This moves some static assertions out of function bodies, where they might have been stuck out of necessity, to perhaps better places at the file level or in header files. --- src/backend/access/heap/heapam.c | 4 ---- src/backend/access/nbtree/nbtutils.c | 7 ------- src/backend/access/transam/xlog.c | 9 --------- src/backend/storage/lmgr/lwlock.c | 9 +++------ src/backend/storage/page/itemptr.c | 14 +++++++------- src/backend/utils/adt/tsginidx.c | 2 -- src/backend/utils/adt/xid8funcs.c | 15 ++++++++------- src/common/controldata_utils.c | 8 -------- src/include/access/gin.h | 3 +++ src/include/access/htup_details.h | 3 +++ src/include/access/nbtree.h | 7 +++++++ src/include/catalog/pg_control.h | 8 ++++++++ src/include/storage/lwlock.h | 3 +++ 13 files changed, 42 insertions(+), 50 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 747db50376..383e1ddde4 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -5783,10 +5783,6 @@ heap_finish_speculative(Relation relation, ItemPointer tid) htup = (HeapTupleHeader) PageGetItem(page, lp); - /* SpecTokenOffsetNumber should be distinguishable from any real offset */ - StaticAssertStmt(MaxOffsetNumber < SpecTokenOffsetNumber, - "invalid speculative token constant"); - /* NO EREPORT(ERROR) from here till changes are logged */ START_CRIT_SECTION(); diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c index ff260c393a..10f13e6d41 100644 --- a/src/backend/access/nbtree/nbtutils.c +++ b/src/backend/access/nbtree/nbtutils.c @@ -2486,13 +2486,6 @@ _bt_check_natts(Relation rel, bool heapkeyspace, Page page, OffsetNumber offnum) Assert(offnum >= FirstOffsetNumber && offnum <= PageGetMaxOffsetNumber(page)); - /* - * Mask allocated for number of keys in index tuple must be able to fit - * maximum possible number of index attributes - */ - StaticAssertStmt(BT_OFFSET_MASK >= INDEX_MAX_KEYS, - "BT_OFFSET_MASK can't fit INDEX_MAX_KEYS"); - itup = (IndexTuple) PageGetItem(page, PageGetItemId(page, offnum)); tupnatts = BTreeTupleGetNAtts(itup, rel); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index a31fbbff78..91473b00d9 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -3883,15 +3883,6 @@ WriteControlFile(void) int fd; char buffer[PG_CONTROL_FILE_SIZE]; /* need not be aligned */ - /* - * Ensure that the size of the pg_control data structure is sane. See the - * comments for these symbols in pg_control.h. - */ - StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_MAX_SAFE_SIZE, - "pg_control is too large for atomic disk writes"); - StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE, - "sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE"); - /* * Initialize version and compatibility-check fields */ diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index a5ad36ca78..528b2e9643 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -108,6 +108,9 @@ extern slock_t *ShmemLock; /* Must be greater than MAX_BACKENDS - which is 2^23-1, so we're fine. */ #define LW_SHARED_MASK ((uint32) ((1 << 24)-1)) +StaticAssertDecl(LW_VAL_EXCLUSIVE > (uint32) MAX_BACKENDS, + "MAX_BACKENDS too big for lwlock.c"); + /* * There are three sorts of LWLock "tranches": * @@ -466,12 +469,6 @@ LWLockShmemSize(void) void CreateLWLocks(void) { - StaticAssertStmt(LW_VAL_EXCLUSIVE > (uint32) MAX_BACKENDS, - "MAX_BACKENDS too big for lwlock.c"); - - StaticAssertStmt(sizeof(LWLock) <= LWLOCK_PADDED_SIZE, - "Miscalculated LWLock padding"); - if (!IsUnderPostmaster) { Size spaceLocks = LWLockShmemSize(); diff --git a/src/backend/storage/page/itemptr.c b/src/backend/storage/page/itemptr.c index 9011337aa8..c1a84acdb6 100644 --- a/src/backend/storage/page/itemptr.c +++ b/src/backend/storage/page/itemptr.c @@ -17,6 +17,13 @@ #include "storage/itemptr.h" +/* + * We really want ItemPointerData to be exactly 6 bytes. This is rather a + * random place to check, but there is no better place. + */ +StaticAssertDecl(sizeof(ItemPointerData) == 3 * sizeof(uint16), + "ItemPointerData struct is improperly padded"); + /* * ItemPointerEquals * Returns true if both item pointers point to the same item, @@ -28,13 +35,6 @@ bool ItemPointerEquals(ItemPointer pointer1, ItemPointer pointer2) { - /* - * We really want ItemPointerData to be exactly 6 bytes. This is rather a - * random place to check, but there is no better place. - */ - StaticAssertStmt(sizeof(ItemPointerData) == 3 * sizeof(uint16), - "ItemPointerData struct is improperly padded"); - if (ItemPointerGetBlockNumber(pointer1) == ItemPointerGetBlockNumber(pointer2) && ItemPointerGetOffsetNumber(pointer1) == diff --git a/src/backend/utils/adt/tsginidx.c b/src/backend/utils/adt/tsginidx.c index e272fca075..cf23aeb5ea 100644 --- a/src/backend/utils/adt/tsginidx.c +++ b/src/backend/utils/adt/tsginidx.c @@ -236,8 +236,6 @@ gin_tsquery_consistent(PG_FUNCTION_ARGS) * query. */ gcv.first_item = GETQUERY(query); - StaticAssertStmt(sizeof(GinTernaryValue) == sizeof(bool), - "sizes of GinTernaryValue and bool are not equal"); gcv.check = (GinTernaryValue *) check; gcv.map_item_operand = (int *) (extra_data[0]); diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c index d8e40b3b96..afc3dc8a47 100644 --- a/src/backend/utils/adt/xid8funcs.c +++ b/src/backend/utils/adt/xid8funcs.c @@ -74,6 +74,14 @@ typedef struct #define PG_SNAPSHOT_MAX_NXIP \ ((MaxAllocSize - offsetof(pg_snapshot, xip)) / sizeof(FullTransactionId)) +/* + * Compile-time limits on the procarray (MAX_BACKENDS processes plus + * MAX_BACKENDS prepared transactions) guarantee nxip won't be too large. + */ +StaticAssertDecl(MAX_BACKENDS * 2 <= PG_SNAPSHOT_MAX_NXIP, + "possible overflow in pg_current_snapshot()"); + + /* * Helper to get a TransactionId from a 64-bit xid with wraparound detection. * @@ -403,13 +411,6 @@ pg_current_snapshot(PG_FUNCTION_ARGS) if (cur == NULL) elog(ERROR, "no active snapshot set"); - /* - * Compile-time limits on the procarray (MAX_BACKENDS processes plus - * MAX_BACKENDS prepared transactions) guarantee nxip won't be too large. - */ - StaticAssertStmt(MAX_BACKENDS * 2 <= PG_SNAPSHOT_MAX_NXIP, - "possible overflow in pg_current_snapshot()"); - /* allocate */ nxip = cur->xcnt; snap = palloc(PG_SNAPSHOT_SIZE(nxip)); diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c index 2d1f35bbd1..5c654746fa 100644 --- a/src/common/controldata_utils.c +++ b/src/common/controldata_utils.c @@ -149,14 +149,6 @@ update_controlfile(const char *DataDir, char buffer[PG_CONTROL_FILE_SIZE]; char ControlFilePath[MAXPGPATH]; - /* - * Apply the same static assertions as in backend's WriteControlFile(). - */ - StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_MAX_SAFE_SIZE, - "pg_control is too large for atomic disk writes"); - StaticAssertStmt(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE, - "sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE"); - /* Update timestamp */ ControlFile->time = (pg_time_t) time(NULL); diff --git a/src/include/access/gin.h b/src/include/access/gin.h index fff861e219..119ed685bb 100644 --- a/src/include/access/gin.h +++ b/src/include/access/gin.h @@ -57,6 +57,9 @@ typedef struct GinStatsData */ typedef char GinTernaryValue; +StaticAssertDecl(sizeof(GinTernaryValue) == sizeof(bool), + "sizes of GinTernaryValue and bool are not equal"); + #define GIN_FALSE 0 /* item is not present / does not match */ #define GIN_TRUE 1 /* item is present / matches */ #define GIN_MAYBE 2 /* don't know if item is present / don't know diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h index 9561c835f2..c1af814e8d 100644 --- a/src/include/access/htup_details.h +++ b/src/include/access/htup_details.h @@ -426,6 +426,9 @@ do { \ (tup)->t_choice.t_heap.t_field3.t_xvac = (xid); \ } while (0) +StaticAssertDecl(MaxOffsetNumber < SpecTokenOffsetNumber, + "invalid speculative token constant"); + #define HeapTupleHeaderIsSpeculative(tup) \ ( \ (ItemPointerGetOffsetNumberNoCheck(&(tup)->t_ctid) == SpecTokenOffsetNumber) \ diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h index 8e4f6864e5..7d5a6fa558 100644 --- a/src/include/access/nbtree.h +++ b/src/include/access/nbtree.h @@ -466,6 +466,13 @@ typedef struct BTVacState #define BT_PIVOT_HEAP_TID_ATTR 0x1000 #define BT_IS_POSTING 0x2000 +/* + * Mask allocated for number of keys in index tuple must be able to fit + * maximum possible number of index attributes + */ +StaticAssertDecl(BT_OFFSET_MASK >= INDEX_MAX_KEYS, + "BT_OFFSET_MASK can't fit INDEX_MAX_KEYS"); + /* * Note: BTreeTupleIsPivot() can have false negatives (but not false * positives) when used with !heapkeyspace indexes diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h index 06368e2366..1b648b7196 100644 --- a/src/include/catalog/pg_control.h +++ b/src/include/catalog/pg_control.h @@ -247,4 +247,12 @@ typedef struct ControlFileData */ #define PG_CONTROL_FILE_SIZE 8192 +/* + * Ensure that the size of the pg_control data structure is sane. + */ +StaticAssertDecl(sizeof(ControlFileData) <= PG_CONTROL_MAX_SAFE_SIZE, + "pg_control is too large for atomic disk writes"); +StaticAssertDecl(sizeof(ControlFileData) <= PG_CONTROL_FILE_SIZE, + "sizeof(ControlFileData) exceeds PG_CONTROL_FILE_SIZE"); + #endif /* PG_CONTROL_H */ diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index a494cb598f..dd818e16ab 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -59,6 +59,9 @@ typedef struct LWLock */ #define LWLOCK_PADDED_SIZE PG_CACHE_LINE_SIZE +StaticAssertDecl(sizeof(LWLock) <= LWLOCK_PADDED_SIZE, + "Miscalculated LWLock padding"); + /* LWLock, padded to a full cache line size */ typedef union LWLockPadded { -- 2.38.1