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

diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index 730a46b1d84fa494fa927380d8ec6095d3e2d34c..90942be71e809220bfedede32e48813f4fbf38a6 100644 (file)
--- 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 a3d367db5118e3d398419464717469b9e81d6d16..8b617c7e79d3ef91da9d5cece280f67ca9d71896 100644 (file)
--- 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 21dfe1b6ee5c560fea32ca1ea751ccba9b22b3cd..dc20122c8940c2ba960205ac56bf52781b2a4960 100644 (file)
--- 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 e9f253f2c8ad68575e34ea826705148a65b31a03..fc67d396b6b93c0a7bb6b1dcb69c5445c3f82979 100644 (file)
--- 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 015f0f1f837ea2248285c7fd62a96f233326fb76..d2eb75f769b2224ff67828639b69a19877d30b3d 100644 (file)
--- 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