Hi Nitin,

Thanks for the review, and good catch.

>  Since both values can be
> misaligned on a build with 8-byte  MAXALIGN, the condition may
> short-circuit at the alignment check without exercising the
> page-boundary checks. Would it make sense to use aligned values such
> as 32736  and 128,so that either bit-field interpretation reaches a
> boundary check?

You are right. With the old value, lp_off decodes as either 32767 or
100, and neither one is a multiple of 8. So on a build with 8-byte
MAXALIGN the code stops at the alignment check and never reaches the
pd_upper / pd_special checks. The test looked like it passed, but it
only covered one of the three branches.

v4 uses 32736 and 128 as you suggested. Both are MAXALIGNed and both
are at least as long as a heap tuple header, so the code always gets
as far as a page boundary check

I also make a few cosmetic changes for errcode and variable declaration.

Given we had 2 LGTM for this patch, I will mark it Ready For Committer

Thanks,
Shihao

Attachment: v4-0001-pg_surgery-skip-blocks-and-line-pointers-that-are.patch
Description: Binary data

Reply via email to