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 <sulamul@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/5b558da8-99fb-0a99-83dd-f72f05388517%40enterprisedb.com

4 files changed, 169 insertions, 110 deletions

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,17 +204,11 @@ 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
  */
 #define SizeOfPageHeaderData (offsetof(PageHeaderData, pd_linp))
@@ -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,21 +251,18 @@ 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