From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
cc: Xing Guo <higuoxing@gmail.com>, pgsql-hackers@lists.postgresql.org
Subject: Re: Fix an incorrect assertion condition in mdwritev().
In-reply-to: <ZlKArtPhzn868no_@paquier.xyz>
References: <CACpMh+BM-VgKeO7suPG-VHTtpzJ+zsbDPwVHu42PLp-iTk0z+A@mail.gmail.com> <ZlKArtPhzn868no_@paquier.xyz>
Comments: In-reply-to Michael Paquier <michael@paquier.xyz>
	message dated "Sun, 26 May 2024 09:22:06 +0900"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1932.1716695945.1@sss.pgh.pa.us>
Date: Sat, 25 May 2024 23:59:05 -0400
Message-ID: <1933.1716695945@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/1933.1716695945%40sss.pgh.pa.us>
Precedence: bulk

Michael Paquier <michael@paquier.xyz> writes:
> On Sat, May 25, 2024 at 11:52:22PM +0800, Xing Guo wrote:
>> #ifdef CHECK_WRITE_VS_EXTEND
>> -	Assert(blocknum < mdnblocks(reln, forknum));
>> +	Assert(blocknum + nblocks <= mdnblocks(reln, forknum));
>> #endif

> Yes, it looks like you're right that this can be made stricter,
> computing the number of blocks we're adding in the number calculated
> (aka adding one block to this number fails immediately at initdb).

Hmm ... I agree that this is better normally.  But there's an
edge case where it would fail to notice a problem that the
existing code does notice: if blocknum is close to UINT32_MAX
and adding nblocks causes it to wrap around to a small value.
Is there an inexpensive way to catch that?  (If not, it's
not a reason to block this patch; but let's think about it
while we're here.)

			regards, tom lane