Re: Improper use about DatumGetInt32

On Thu, Nov 26, 2020 at 9:57 PM Peter Eisentraut <
peter(dot)eisentraut(at)enterprisedb(dot)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