Re: Improper use about DatumGetInt32

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")));
> +