PageGetItemIdCareful - should we MAXALIGN sizeof(BTPageOpaqueData)? - Mailing list pgsql-hackers

From Bharath Rupireddy
Subject PageGetItemIdCareful - should we MAXALIGN sizeof(BTPageOpaqueData)?
Date
Msg-id CALj2ACUMqTR9nErh99FbOBmzCXE9=gXNqhBiwYOhejJJS1LXqQ@mail.gmail.com
Whole thread Raw
Responses Re: PageGetItemIdCareful - should we MAXALIGN sizeof(BTPageOpaqueData)?  (Dilip Kumar <dilipbalaut@gmail.com>)
Re: PageGetItemIdCareful - should we MAXALIGN sizeof(BTPageOpaqueData)?  (Peter Geoghegan <pg@bowt.ie>)
List pgsql-hackers
Hi,

In the PageGetItemIdCareful() introduced by commit a9ce839a, it seems
like we are using btree page pd_special structure BTPageOpaqueData for
error case without max aligning it.
    if (ItemIdGetOffset(itemid) + ItemIdGetLength(itemid) >
        BLCKSZ - sizeof(BTPageOpaqueData))
        ereport(ERROR,

I'm not sure if it is intentional. ISTM that this was actually not a
problem because the BTPageOpaqueData already has all-aligned(???)
members (3 uint32, 2 uint16). But it might be a problem if we add
unaligned bytes. PageInit always max aligns this structure, when we
initialize the btree page in _bt_pageini and in all other places we
max align it before use. Since this is an error throwing path, I think
we should max align it  just to be on the safer side. While on it, I
think we can also replace BLCKSZ with PageGetPageSize(page).

Attaching a small patch. Thoughts?

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com

Attachment

pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Replication slot stats misgivings
Next
From: Ian Zagorskikh
Date:
Subject: Procedure of modification of autoconf scripts