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