Re: PageGetMaxOffsetNumber on uninitialized pages

Tom Lane wrote:

> I was just looking at this macro:
>
> /*
> * PageGetMaxOffsetNumber
> * Returns the maximum offset number used by the given page.
> * Since offset numbers are 1-based, this is also the number
> * of items on the page.
> *
> * NOTE: to ensure sane behavior if the page is not initialized
> * (pd_lower == 0), cast the unsigned values to int before dividing.
> * That way we get -1 or so, not a huge positive number...
> */
> #define PageGetMaxOffsetNumber(page) \
> (((int) (((PageHeader) (page))->pd_lower - SizeOfPageHeaderData)) \
> / ((int) sizeof(ItemIdData)))
>
> The macro does the right thing on its own terms when applied to a zeroed
> page, but in some places it's used like this:
>
> OffsetNumber n;
> OffsetNumber maxoff;
>
> maxoff = PageGetMaxOffsetNumber(p);
> for (n = FirstOffsetNumber; n <= maxoff; n++)
>
> and OffsetNumber is uint16 not int. So a loop like this would go nuts
> instead of treating the zeroed page as if it were empty. This is not
> good (see the comments for PageHeaderIsValid in bufpage.c if you
> disremember why).
>
> We could fix this by changing the declarations of the "maxoff" variables
> to int, but I think it's probably cleaner to recode
> PageGetMaxOffsetNumber like so:
>
> #define PageGetMaxOffsetNumber(page) \
> (((PageHeader) (page))->pd_lower <= SizeOfPageHeaderData ? 0 : \
> ((((PageHeader) (page))->pd_lower - SizeOfPageHeaderData) \
> / sizeof(ItemIdData)))

Well I think that is safe change:

a <= b ? 0 : ( a-b ) /c

in

max( 0, (a-b)/c )

so I think (not tested) you can rewrite that macro in:

#define PageGetMaxOffsetNumber(page) \
(max(0, ((((PageHeader) (page))->pd_lower - SizeOfPageHeaderData) \
/ sizeof(ItemIdData))))

Regards
Gaeatano Mendola