From 2cd2569c72b8920048e35c31c9be30a6170e1410 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 11 Jul 2022 07:20:35 +0200 Subject: [PATCH] Convert macros to static inline functions (bufpage.h) Remove PageIsValid() and PageSizeIsValid(), which weren't used and seem unnecessary. Some code using these formerly-macros needs some adjustments because it was previously playing loose with the Page vs. PageHeader types, which is no longer possible with the functions instead of macros. Reviewed-by: Amul Sul Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com --- contrib/pageinspect/rawpage.c | 24 +-- src/backend/storage/page/bufpage.c | 18 +- src/bin/pg_checksums/pg_checksums.c | 2 +- src/include/storage/bufpage.h | 257 +++++++++++++++++----------- src/include/storage/checksum_impl.h | 2 +- 5 files changed, 182 insertions(+), 121 deletions(-) diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c index 730a46b1d84..90942be71e8 100644 --- a/contrib/pageinspect/rawpage.c +++ b/contrib/pageinspect/rawpage.c @@ -254,7 +254,8 @@ page_header(PG_FUNCTION_ARGS) Datum values[9]; bool nulls[9]; - PageHeader page; + Page page; + PageHeader pageheader; XLogRecPtr lsn; if (!superuser()) @@ -262,7 +263,8 @@ page_header(PG_FUNCTION_ARGS) (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("must be superuser to use raw page functions"))); - page = (PageHeader) get_page_from_raw(raw_page); + page = get_page_from_raw(raw_page); + pageheader = (PageHeader) page; /* Build a tuple descriptor for our result type */ if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE) @@ -282,8 +284,8 @@ page_header(PG_FUNCTION_ARGS) } else values[0] = LSNGetDatum(lsn); - values[1] = UInt16GetDatum(page->pd_checksum); - values[2] = UInt16GetDatum(page->pd_flags); + values[1] = UInt16GetDatum(pageheader->pd_checksum); + values[2] = UInt16GetDatum(pageheader->pd_flags); /* pageinspect >= 1.10 uses int4 instead of int2 for those fields */ switch (TupleDescAttr(tupdesc, 3)->atttypid) @@ -292,18 +294,18 @@ page_header(PG_FUNCTION_ARGS) Assert(TupleDescAttr(tupdesc, 4)->atttypid == INT2OID && TupleDescAttr(tupdesc, 5)->atttypid == INT2OID && TupleDescAttr(tupdesc, 6)->atttypid == INT2OID); - values[3] = UInt16GetDatum(page->pd_lower); - values[4] = UInt16GetDatum(page->pd_upper); - values[5] = UInt16GetDatum(page->pd_special); + values[3] = UInt16GetDatum(pageheader->pd_lower); + values[4] = UInt16GetDatum(pageheader->pd_upper); + values[5] = UInt16GetDatum(pageheader->pd_special); values[6] = UInt16GetDatum(PageGetPageSize(page)); break; case INT4OID: Assert(TupleDescAttr(tupdesc, 4)->atttypid == INT4OID && TupleDescAttr(tupdesc, 5)->atttypid == INT4OID && TupleDescAttr(tupdesc, 6)->atttypid == INT4OID); - values[3] = Int32GetDatum(page->pd_lower); - values[4] = Int32GetDatum(page->pd_upper); - values[5] = Int32GetDatum(page->pd_special); + values[3] = Int32GetDatum(pageheader->pd_lower); + values[4] = Int32GetDatum(pageheader->pd_upper); + values[5] = Int32GetDatum(pageheader->pd_special); values[6] = Int32GetDatum(PageGetPageSize(page)); break; default: @@ -312,7 +314,7 @@ page_header(PG_FUNCTION_ARGS) } values[7] = UInt16GetDatum(PageGetPageLayoutVersion(page)); - values[8] = TransactionIdGetDatum(page->pd_prune_xid); + values[8] = TransactionIdGetDatum(pageheader->pd_prune_xid); /* Build and return the tuple. */ diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.c index a3d367db511..8b617c7e79d 100644 --- a/src/backend/storage/page/bufpage.c +++ b/src/backend/storage/page/bufpage.c @@ -230,7 +230,7 @@ PageAddItemExtended(Page page, { if (offsetNumber < limit) { - itemId = PageGetItemId(phdr, offsetNumber); + itemId = PageGetItemId(page, offsetNumber); if (ItemIdIsUsed(itemId) || ItemIdHasStorage(itemId)) { elog(WARNING, "will not overwrite a used ItemId"); @@ -248,7 +248,7 @@ PageAddItemExtended(Page page, { /* offsetNumber was not passed in, so find a free slot */ /* if no free slot, we'll put it at limit (1st open slot) */ - if (PageHasFreeLinePointers(phdr)) + if (PageHasFreeLinePointers(page)) { /* * Scan line pointer array to locate a "recyclable" (unused) @@ -262,7 +262,7 @@ PageAddItemExtended(Page page, offsetNumber < limit; /* limit is maxoff+1 */ offsetNumber++) { - itemId = PageGetItemId(phdr, offsetNumber); + itemId = PageGetItemId(page, offsetNumber); /* * We check for no storage as well, just to be paranoid; @@ -277,7 +277,7 @@ PageAddItemExtended(Page page, if (offsetNumber >= limit) { /* the hint is wrong, so reset it */ - PageClearHasFreeLinePointers(phdr); + PageClearHasFreeLinePointers(page); } } else @@ -322,7 +322,7 @@ PageAddItemExtended(Page page, /* * OK to insert the item. First, shuffle the existing pointers if needed. */ - itemId = PageGetItemId(phdr, offsetNumber); + itemId = PageGetItemId(page, offsetNumber); if (needshuffle) memmove(itemId + 1, itemId, @@ -1004,7 +1004,7 @@ PageGetHeapFreeSpace(Page page) nline = PageGetMaxOffsetNumber(page); if (nline >= MaxHeapTuplesPerPage) { - if (PageHasFreeLinePointers((PageHeader) page)) + if (PageHasFreeLinePointers(page)) { /* * Since this is just a hint, we must confirm that there is @@ -1139,7 +1139,7 @@ PageIndexTupleDelete(Page page, OffsetNumber offnum) nline--; /* there's one less than when we started */ for (i = 1; i <= nline; i++) { - ItemId ii = PageGetItemId(phdr, i); + ItemId ii = PageGetItemId(page, i); Assert(ItemIdHasStorage(ii)); if (ItemIdGetOffset(ii) <= offset) @@ -1374,7 +1374,7 @@ PageIndexTupleDeleteNoCompact(Page page, OffsetNumber offnum) for (i = 1; i <= nline; i++) { - ItemId ii = PageGetItemId(phdr, i); + ItemId ii = PageGetItemId(page, i); if (ItemIdHasStorage(ii) && ItemIdGetOffset(ii) <= offset) ii->lp_off += size; @@ -1473,7 +1473,7 @@ PageIndexTupleOverwrite(Page page, OffsetNumber offnum, /* adjust affected line pointers too */ for (i = FirstOffsetNumber; i <= itemcount; i++) { - ItemId ii = PageGetItemId(phdr, i); + ItemId ii = PageGetItemId(page, i); /* Allow items without storage; currently only BRIN needs that */ if (ItemIdHasStorage(ii) && ItemIdGetOffset(ii) <= offset) diff --git a/src/bin/pg_checksums/pg_checksums.c b/src/bin/pg_checksums/pg_checksums.c index 21dfe1b6ee5..dc20122c894 100644 --- a/src/bin/pg_checksums/pg_checksums.c +++ b/src/bin/pg_checksums/pg_checksums.c @@ -228,7 +228,7 @@ scan_file(const char *fn, int segmentno) current_size += r; /* New pages have no checksum yet */ - if (PageIsNew(header)) + if (PageIsNew(buf.data)) continue; csum = pg_checksum_page(buf.data, blockno + segmentno * RELSEG_SIZE); diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h index e9f253f2c8a..fc67d396b6b 100644 --- a/src/include/storage/bufpage.h +++ b/src/include/storage/bufpage.h @@ -97,8 +97,12 @@ typedef struct uint32 xrecoff; /* low bits */ } PageXLogRecPtr; -#define PageXLogRecPtrGet(val) \ - ((uint64) (val).xlogid << 32 | (val).xrecoff) +static inline XLogRecPtr +PageXLogRecPtrGet(PageXLogRecPtr val) +{ + return (uint64) val.xlogid << 32 | val.xrecoff; +} + #define PageXLogRecPtrSet(ptr, lsn) \ ((ptr).xlogid = (uint32) ((lsn) >> 32), (ptr).xrecoff = (uint32) (lsn)) @@ -200,16 +204,10 @@ typedef PageHeaderData *PageHeader; #define PG_DATA_CHECKSUM_VERSION 1 /* ---------------------------------------------------------------- - * page support macros + * page support functions * ---------------------------------------------------------------- */ -/* - * PageIsValid - * True iff page is valid. - */ -#define PageIsValid(page) PointerIsValid(page) - /* * line pointer(s) do not count as part of header */ @@ -219,21 +217,31 @@ typedef PageHeaderData *PageHeader; * PageIsEmpty * returns true iff no itemid has been allocated on the page */ -#define PageIsEmpty(page) \ - (((PageHeader) (page))->pd_lower <= SizeOfPageHeaderData) +static inline bool +PageIsEmpty(Page page) +{ + return ((PageHeader) page)->pd_lower <= SizeOfPageHeaderData; +} /* * PageIsNew * returns true iff page has not been initialized (by PageInit) */ -#define PageIsNew(page) (((PageHeader) (page))->pd_upper == 0) +static inline bool +PageIsNew(Page page) +{ + return ((PageHeader) page)->pd_upper == 0; +} /* * PageGetItemId * Returns an item identifier of a page. */ -#define PageGetItemId(page, offsetNumber) \ - ((ItemId) (&((PageHeader) (page))->pd_linp[(offsetNumber) - 1])) +static inline ItemId +PageGetItemId(Page page, OffsetNumber offsetNumber) +{ + return &((PageHeader) page)->pd_linp[offsetNumber - 1]; +} /* * PageGetContents @@ -243,20 +251,17 @@ typedef PageHeaderData *PageHeader; * Now it is. Beware of old code that might think the offset to the contents * is just SizeOfPageHeaderData rather than MAXALIGN(SizeOfPageHeaderData). */ -#define PageGetContents(page) \ - ((char *) (page) + MAXALIGN(SizeOfPageHeaderData)) +static inline char * +PageGetContents(Page page) +{ + return (char *) page + MAXALIGN(SizeOfPageHeaderData); +} /* ---------------- - * macros to access page size info + * functions to access page size info * ---------------- */ -/* - * PageSizeIsValid - * True iff the page size is valid. - */ -#define PageSizeIsValid(pageSize) ((pageSize) == BLCKSZ) - /* * PageGetPageSize * Returns the page size of a page. @@ -265,15 +270,21 @@ typedef PageHeaderData *PageHeader; * BufferGetPageSize, which can be called on an unformatted page). * however, it can be called on a page that is not stored in a buffer. */ -#define PageGetPageSize(page) \ - ((Size) (((PageHeader) (page))->pd_pagesize_version & (uint16) 0xFF00)) +static inline Size +PageGetPageSize(Page page) +{ + return (Size) (((PageHeader) page)->pd_pagesize_version & (uint16) 0xFF00); +} /* * PageGetPageLayoutVersion * Returns the page layout version of a page. */ -#define PageGetPageLayoutVersion(page) \ - (((PageHeader) (page))->pd_pagesize_version & 0x00FF) +static inline uint8 +PageGetPageLayoutVersion(Page page) +{ + return (((PageHeader) page)->pd_pagesize_version & 0x00FF); +} /* * PageSetPageSizeAndVersion @@ -282,52 +293,52 @@ typedef PageHeaderData *PageHeader; * We could support setting these two values separately, but there's * no real need for it at the moment. */ -#define PageSetPageSizeAndVersion(page, size, version) \ -( \ - AssertMacro(((size) & 0xFF00) == (size)), \ - AssertMacro(((version) & 0x00FF) == (version)), \ - ((PageHeader) (page))->pd_pagesize_version = (size) | (version) \ -) +static inline void +PageSetPageSizeAndVersion(Page page, Size size, uint8 version) +{ + Assert((size & 0xFF00) == size); + Assert((version & 0x00FF) == version); + + ((PageHeader) page)->pd_pagesize_version = size | version; +} /* ---------------- - * page special data macros + * page special data functions * ---------------- */ /* * PageGetSpecialSize * Returns size of special space on a page. */ -#define PageGetSpecialSize(page) \ - ((uint16) (PageGetPageSize(page) - ((PageHeader)(page))->pd_special)) +static inline uint16 +PageGetSpecialSize(Page page) +{ + return (PageGetPageSize(page) - ((PageHeader) page)->pd_special); +} /* * Using assertions, validate that the page special pointer is OK. * * This is intended to catch use of the pointer before page initialization. - * It is implemented as a function due to the limitations of the MSVC - * compiler, which choked on doing all these tests within another macro. We - * return true so that AssertMacro() can be used while still getting the - * specifics from the macro failure within this function. */ -static inline bool +static inline void PageValidateSpecialPointer(Page page) { - Assert(PageIsValid(page)); - Assert(((PageHeader) (page))->pd_special <= BLCKSZ); - Assert(((PageHeader) (page))->pd_special >= SizeOfPageHeaderData); - - return true; + Assert(page); + Assert(((PageHeader) page)->pd_special <= BLCKSZ); + Assert(((PageHeader) page)->pd_special >= SizeOfPageHeaderData); } /* * PageGetSpecialPointer * Returns pointer to special space on a page. */ -#define PageGetSpecialPointer(page) \ -( \ - AssertMacro(PageValidateSpecialPointer(page)), \ - (char *) ((char *) (page) + ((PageHeader) (page))->pd_special) \ -) +static inline char * +PageGetSpecialPointer(Page page) +{ + PageValidateSpecialPointer(page); + return (char *) page + ((PageHeader) page)->pd_special; +} /* * PageGetItem @@ -337,12 +348,14 @@ PageValidateSpecialPointer(Page page) * This does not change the status of any of the resources passed. * The semantics may change in the future. */ -#define PageGetItem(page, itemId) \ -( \ - AssertMacro(PageIsValid(page)), \ - AssertMacro(ItemIdHasStorage(itemId)), \ - (Item)(((char *)(page)) + ItemIdGetOffset(itemId)) \ -) +static inline Item +PageGetItem(Page page, ItemId itemId) +{ + Assert(page); + Assert(ItemIdHasStorage(itemId)); + + return (Item) (((char *) page) + ItemIdGetOffset(itemId)); +} /* * PageGetMaxOffsetNumber @@ -351,44 +364,84 @@ PageValidateSpecialPointer(Page page) * of items on the page. * * NOTE: if the page is not initialized (pd_lower == 0), we must - * return zero to ensure sane behavior. Accept double evaluation - * of the argument so that we can ensure this. + * return zero to ensure sane behavior. */ -#define PageGetMaxOffsetNumber(page) \ - (((PageHeader) (page))->pd_lower <= SizeOfPageHeaderData ? 0 : \ - ((((PageHeader) (page))->pd_lower - SizeOfPageHeaderData) \ - / sizeof(ItemIdData))) +static inline OffsetNumber +PageGetMaxOffsetNumber(Page page) +{ + PageHeader pageheader = (PageHeader) page; + + if (pageheader->pd_lower <= SizeOfPageHeaderData) + return 0; + else + return (pageheader->pd_lower - SizeOfPageHeaderData) / sizeof(ItemIdData); +} /* - * Additional macros for access to page headers. (Beware multiple evaluation - * of the arguments!) + * Additional functions for access to page headers. */ -#define PageGetLSN(page) \ - PageXLogRecPtrGet(((PageHeader) (page))->pd_lsn) -#define PageSetLSN(page, lsn) \ - PageXLogRecPtrSet(((PageHeader) (page))->pd_lsn, lsn) - -#define PageHasFreeLinePointers(page) \ - (((PageHeader) (page))->pd_flags & PD_HAS_FREE_LINES) -#define PageSetHasFreeLinePointers(page) \ - (((PageHeader) (page))->pd_flags |= PD_HAS_FREE_LINES) -#define PageClearHasFreeLinePointers(page) \ - (((PageHeader) (page))->pd_flags &= ~PD_HAS_FREE_LINES) - -#define PageIsFull(page) \ - (((PageHeader) (page))->pd_flags & PD_PAGE_FULL) -#define PageSetFull(page) \ - (((PageHeader) (page))->pd_flags |= PD_PAGE_FULL) -#define PageClearFull(page) \ - (((PageHeader) (page))->pd_flags &= ~PD_PAGE_FULL) - -#define PageIsAllVisible(page) \ - (((PageHeader) (page))->pd_flags & PD_ALL_VISIBLE) -#define PageSetAllVisible(page) \ - (((PageHeader) (page))->pd_flags |= PD_ALL_VISIBLE) -#define PageClearAllVisible(page) \ - (((PageHeader) (page))->pd_flags &= ~PD_ALL_VISIBLE) +static inline XLogRecPtr +PageGetLSN(Page page) +{ + return PageXLogRecPtrGet(((PageHeader) page)->pd_lsn); +} +static inline void +PageSetLSN(Page page, XLogRecPtr lsn) +{ + PageXLogRecPtrSet(((PageHeader) page)->pd_lsn, lsn); +} + +static inline bool +PageHasFreeLinePointers(Page page) +{ + return ((PageHeader) page)->pd_flags & PD_HAS_FREE_LINES; +} +static inline void +PageSetHasFreeLinePointers(Page page) +{ + ((PageHeader) page)->pd_flags |= PD_HAS_FREE_LINES; +} +static inline void +PageClearHasFreeLinePointers(Page page) +{ + ((PageHeader) page)->pd_flags &= ~PD_HAS_FREE_LINES; +} + +static inline bool +PageIsFull(Page page) +{ + return ((PageHeader) page)->pd_flags & PD_PAGE_FULL; +} +static inline void +PageSetFull(Page page) +{ + ((PageHeader) page)->pd_flags |= PD_PAGE_FULL; +} +static inline void +PageClearFull(Page page) +{ + ((PageHeader) page)->pd_flags &= ~PD_PAGE_FULL; +} +static inline bool +PageIsAllVisible(Page page) +{ + return ((PageHeader) page)->pd_flags & PD_ALL_VISIBLE; +} +static inline void +PageSetAllVisible(Page page) +{ + ((PageHeader) page)->pd_flags |= PD_ALL_VISIBLE; +} +static inline void +PageClearAllVisible(Page page) +{ + ((PageHeader) page)->pd_flags &= ~PD_ALL_VISIBLE; +} + +/* + * These two require "access/transam.h", so left as macros. + */ #define PageSetPrunable(page, xid) \ do { \ Assert(TransactionIdIsNormal(xid)); \ @@ -413,15 +466,6 @@ do { \ #define PIV_LOG_WARNING (1 << 0) #define PIV_REPORT_STAT (1 << 1) -#define PageAddItem(page, item, size, offsetNumber, overwrite, is_heap) \ - PageAddItemExtended(page, item, size, offsetNumber, \ - ((overwrite) ? PAI_OVERWRITE : 0) | \ - ((is_heap) ? PAI_IS_HEAP : 0)) - -#define PageIsVerified(page, blkno) \ - PageIsVerifiedExtended(page, blkno, \ - PIV_LOG_WARNING | PIV_REPORT_STAT) - /* * Check that BLCKSZ is a multiple of sizeof(size_t). In * PageIsVerifiedExtended(), it is much faster to check if a page is @@ -436,6 +480,21 @@ extern void PageInit(Page page, Size pageSize, Size specialSize); extern bool PageIsVerifiedExtended(Page page, BlockNumber blkno, int flags); extern OffsetNumber PageAddItemExtended(Page page, Item item, Size size, OffsetNumber offsetNumber, int flags); + +static inline OffsetNumber +PageAddItem(Page page, Item item, Size size, OffsetNumber offsetNumber, bool overwrite, bool is_heap) +{ + return PageAddItemExtended(page, item, size, offsetNumber, + (overwrite ? PAI_OVERWRITE : 0) | + (is_heap ? PAI_IS_HEAP : 0)); +} + +static inline bool +PageIsVerified(Page page, BlockNumber blkno) +{ + return PageIsVerifiedExtended(page, blkno, PIV_LOG_WARNING | PIV_REPORT_STAT); +} + extern Page PageGetTempPage(Page page); extern Page PageGetTempPageCopy(Page page); extern Page PageGetTempPageCopySpecial(Page page); diff --git a/src/include/storage/checksum_impl.h b/src/include/storage/checksum_impl.h index 015f0f1f837..d2eb75f769b 100644 --- a/src/include/storage/checksum_impl.h +++ b/src/include/storage/checksum_impl.h @@ -191,7 +191,7 @@ pg_checksum_page(char *page, BlockNumber blkno) uint32 checksum; /* We only calculate the checksum for properly-initialized pages */ - Assert(!PageIsNew(&cpage->phdr)); + Assert(!PageIsNew((Page) page)); /* * Save pd_checksum and temporarily set it to zero, so that the checksum -- 2.39.5