public inbox for [email protected]  
help / color / mirror / Atom feed
From: Alvaro Herrera <[email protected]>
To: Dmitry Nikitin <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] Missing Assert in the code
Date: Mon, 25 Nov 2024 20:51:31 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 2024-Nov-25, Dmitry Nikitin wrote:

> Subject: [PATCH 2/2] Add Assert to stop invalid values to pass on
> 
> PageGetMaxOffsetNumber() can legitimately return zero
> (InvalidOffsetNumber) as an indication of error. However there are no
> any checks against that. As a result, for exampe, subsequent
> PageGetItemId() will be accessing an array at -1.

> @@ -236,6 +236,8 @@ getRightMostTuple(Page page)
>  {
>  	OffsetNumber maxoff = PageGetMaxOffsetNumber(page);
>  
> +	Assert(OffsetNumberIsValid(maxoff));
> +
>  	return (IndexTuple) PageGetItem(page, PageGetItemId(page, maxoff));
>  }

Hmm, I think if we believe this to be really possible, we should have an
'if/elog' test (or maybe a full ereport with ERRCODE_DATA_CORRUPTED
errcode) rather than an assertion.  I think the assertion adds nothing
of value here, but maybe an 'if' test would.

Did you examine the other callers of PageGetMaxOffsetNumber()?  It's a
large bunch.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
[…] indem ich in meinem Leben oft an euch gedacht, euch glücklich zu machen. Seyd es!
A menudo he pensado en vosotros, en haceros felices. ¡Sedlo, pues!
		Heiligenstädter Testament, L. v. Beethoven, 1802
		https://de.wikisource.org/wiki/Heiligenstädter_Testament






view thread (5+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: [PATCH] Missing Assert in the code
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox