From: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
Cc: | Ashutosh Bapat <ashutosh(dot)bapat(at)2ndquadrant(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, "Hou, Zhijie" <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Improper use about DatumGetInt32 |
Date: | 2020-11-25 19:04:09 |
Message-ID: | 20201125190409.GA9643@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2020-Nov-25, Peter Eisentraut wrote:
> bt_page_stats(PG_FUNCTION_ARGS)
> {
> text *relname = PG_GETARG_TEXT_PP(0);
> - uint32 blkno = PG_GETARG_UINT32(1);
> + int64 blkno = PG_GETARG_INT64(1);
As a matter of style, I think it'd be better to have an int64 variable
that gets the value from PG_GETARG_INT64(), then you cast that to
another variable that's a BlockNumber and use that throughout the rest
of the code. So you'd avoid changes like this:
> static bytea *get_raw_page_internal(text *relname, ForkNumber forknum,
> - BlockNumber blkno);
> + int64 blkno);
where the previous coding was correct, and the new one is dubious and it
forces you to add unnecessary range checks in that function:
> @@ -144,11 +144,16 @@ get_raw_page_internal(text *relname, ForkNumber forknum, BlockNumber blkno)
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("cannot access temporary tables of other sessions")));
>
> + if (blkno < 0 || blkno > MaxBlockNumber)
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("invalid block number")));
> +
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2020-11-25 19:08:02 | Re: [PATCH] Add section headings to index types doc |
Previous Message | Bossart, Nathan | 2020-11-25 18:59:12 | Re: A few new options for CHECKPOINT |