Re: Improper use about DatumGetInt32

On Thu, Nov 26, 2020 at 9:57 PM Peter Eisentraut <
peter.eisentraut@enterprisedb.com> wrote:

> On 2020-11-26 14:27, Alvaro Herrera wrote:
> > On 2020-Nov-26, Peter Eisentraut wrote:
> >
> >> The point of the patch is to have the range check somewhere.  If you
> just
> >> cast it, then you won't notice out of range arguments.  Note that other
> >> contrib modules that take block numbers work the same way.
> >
> > I'm not saying not to do that; just saying we should not propagate it to
> > places that don't need it.  get_raw_page gets its page number from
> > PG_GETARG_INT64(), and the range check should be there.  But then it
> > calls get_raw_page_internal, and it could pass a BlockNumber -- there's
> > no need to pass an int64.  So get_raw_page_internal does not need a
> > range check.
>
> Yeah, I had it like that for a moment, but then you need to duplicate
> the check in get_raw_page() and get_raw_page_fork().  I figured since
> get_raw_page_internal() does all the other argument checking also, it
> seems sensible to put the block range check there too.  But it's not a
> big deal either way.
>

FWIW, my 2c. Though I agree with both sides, I
prefer get_raw_page_internal() accepting BlockNumber, since that's what it
deals with and not the entire int8.

--
Best Wishes,
Ashutosh