Re: PageGetItemIdCareful - should we MAXALIGN sizeof(BTPageOpaqueData)?

On Thu, Apr 22, 2021 at 10:40 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> 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?

+1 for changing to MAXALIGN(sizeof(BTPageOpaqueData)).

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com