Date: Fri, 1 Apr 2022 14:38:51 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Matthias van de Meent <boekewurm+postgres@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Preventing indirection for IndexPageGetOpaque for known-size
 page special areas
Message-ID: <YkaP64JvZTMgcHtq@paquier.xyz>
References: 
 <CAEze2WjE3+tGO9Fs9+iZMU+z6mMZKo54W1Zt98WKqbEUHbHOBg@mail.gmail.com>
 <YkE6FNTng8a1SVas@paquier.xyz>
 <CAEze2Wjjx4REsge5kxNjDjKDVvkqbruqxVLykxj_UufZ0xzbOw@mail.gmail.com>
 <YkVY82G3Jwmp5Vby@paquier.xyz>
 <CAEze2WiYp=-1bN=7jZ4_wHxi41HCuOFzNNaT1y193f+dCY4UNw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="G806gToVJUOclRG4"
Content-Disposition: inline
In-Reply-To: 
 <CAEze2WiYp=-1bN=7jZ4_wHxi41HCuOFzNNaT1y193f+dCY4UNw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/YkaP64JvZTMgcHtq%40paquier.xyz>
Precedence: bulk


--G806gToVJUOclRG4
Content-Type: multipart/mixed; boundary="2tb1raO4Nn03KMxM"
Content-Disposition: inline


--2tb1raO4Nn03KMxM
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Mar 31, 2022 at 12:09:35PM +0200, Matthias van de Meent wrote:
> PageInit MAXALIGNs the size of the special area that it receives as an
> argument; so any changes to the page header that would misalign the
> value would be AM-specific; in which case it is quite unlikely that
> this is the right accessor for your page's special area.

Right.  I'd still be tempted to keep that per-AM rather than making
the checks deeper with one extra macro layer in the page header or
with a different macro that would depend on the opaque type, though.
Like in the attached, for example.
--
Michael

--2tb1raO4Nn03KMxM
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="opaque-macros.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/include/access/ginblock.h b/src/include/access/ginblock.h
index 9347f464f3..050fff80dc 100644
--- a/src/include/access/ginblock.h
+++ b/src/include/access/ginblock.h
@@ -108,7 +108,11 @@ typedef struct GinMetaPageData
 /*
  * Macros for accessing a GIN index page's opaque data
  */
-#define GinPageGetOpaque(page) ( (GinPageOpaque) PageGetSpecialPointer(pag=
e) )
+#define GinPageGetOpaque(page) \
+( \
+	AssertMacro(PageGetSpecialSize(page) =3D=3D MAXALIGN(sizeof(GinPageOpaque=
Data))), \
+	(GinPageOpaque) PageGetSpecialPointer(page) \
+)
=20
 #define GinPageIsLeaf(page)    ( (GinPageGetOpaque(page)->flags & GIN_LEAF=
) !=3D 0 )
 #define GinPageSetLeaf(page)   ( GinPageGetOpaque(page)->flags |=3D GIN_LE=
AF )
diff --git a/src/include/access/gist.h b/src/include/access/gist.h
index a3337627b8..a10caf8ae9 100644
--- a/src/include/access/gist.h
+++ b/src/include/access/gist.h
@@ -164,7 +164,11 @@ typedef struct GISTENTRY
 	bool		leafkey;
 } GISTENTRY;
=20
-#define GistPageGetOpaque(page) ( (GISTPageOpaque) PageGetSpecialPointer(p=
age) )
+#define GistPageGetOpaque(page) \
+( \
+	AssertMacro(PageGetSpecialSize(page) =3D=3D MAXALIGN(sizeof(GISTPageOpaqu=
eData))), \
+	(GISTPageOpaque) PageGetSpecialPointer(page) \
+)
=20
 #define GistPageIsLeaf(page)	( GistPageGetOpaque(page)->flags & F_LEAF)
 #define GIST_LEAF(entry) (GistPageIsLeaf((entry)->page))
diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index da372841c4..b442ddc4c8 100644
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -85,7 +85,11 @@ typedef struct HashPageOpaqueData
=20
 typedef HashPageOpaqueData *HashPageOpaque;
=20
-#define HashPageGetOpaque(page) ((HashPageOpaque) PageGetSpecialPointer(pa=
ge))
+#define HashPageGetOpaque(page) \
+( \
+	AssertMacro(PageGetSpecialSize(page) =3D=3D MAXALIGN(sizeof(HashPageOpaqu=
eData))), \
+	(HashPageOpaque) PageGetSpecialPointer(page) \
+)
=20
 #define H_NEEDS_SPLIT_CLEANUP(opaque)	(((opaque)->hasho_flag & LH_BUCKET_N=
EEDS_SPLIT_CLEANUP) !=3D 0)
 #define H_BUCKET_BEING_SPLIT(opaque)	(((opaque)->hasho_flag & LH_BUCKET_BE=
ING_SPLIT) !=3D 0)
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index 93f8267b48..e11839857d 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -70,7 +70,11 @@ typedef struct BTPageOpaqueData
=20
 typedef BTPageOpaqueData *BTPageOpaque;
=20
-#define BTPageGetOpaque(page) ((BTPageOpaque) PageGetSpecialPointer(page))
+#define BTPageGetOpaque(page) \
+( \
+	AssertMacro(PageGetSpecialSize(page) =3D=3D MAXALIGN(sizeof(BTPageOpaqueD=
ata))), \
+	(BTPageOpaque) PageGetSpecialPointer(page) \
+)
=20
 /* Bits defined in btpo_flags */
 #define BTP_LEAF		(1 << 0)	/* leaf page, i.e. not internal page */
diff --git a/src/include/access/spgist_private.h b/src/include/access/spgis=
t_private.h
index eb56b1c6b8..d61433b5af 100644
--- a/src/include/access/spgist_private.h
+++ b/src/include/access/spgist_private.h
@@ -75,7 +75,12 @@ typedef SpGistPageOpaqueData *SpGistPageOpaque;
 #define SPGIST_LEAF			(1<<2)
 #define SPGIST_NULLS		(1<<3)
=20
-#define SpGistPageGetOpaque(page) ((SpGistPageOpaque) PageGetSpecialPointe=
r(page))
+#define SpGistPageGetOpaque(page) \
+( \
+	AssertMacro(PageGetSpecialSize(page) =3D=3D MAXALIGN(sizeof(SpGistPageOpa=
queData))), \
+	(SpGistPageOpaque) PageGetSpecialPointer(page) \
+)
+
 #define SpGistPageIsMeta(page) (SpGistPageGetOpaque(page)->flags & SPGIST_=
META)
 #define SpGistPageIsDeleted(page) (SpGistPageGetOpaque(page)->flags & SPGI=
ST_DELETED)
 #define SpGistPageIsLeaf(page) (SpGistPageGetOpaque(page)->flags & SPGIST_=
LEAF)

--2tb1raO4Nn03KMxM--

--G806gToVJUOclRG4
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmJGj+sACgkQnvQgOdby
QH0GrA//Zy1eENBAOfzxmCFIosPHdN+fzoNcHODzcCdPG0z5YHc3L9toexe0Fvcl
jqrKCBWQ/DlVhg3cRk6/WID6R52eW7VvkZSyrOJhmcCladxndWgr9aHOsU7Xlwqj
Nvh0jK4jicTc7HhpYAQRpFkqt8H0wGPrwNRO+FahHDpxPDo2dHaLs/u0/bMQOHqd
Hu9wKvY2lVo4rBDPxF3NtKbnV4a7hE04f9BsefLJ6CYrBPSIMdBfk/Sn/sG83Ocb
PsO3lL7bJGFX6QEJHnNW6eNj2PKXa72VYQqdm9wYnUebu4FtJRrt33Nu3e9SEzwY
opMP+TASrq7VOyP3N7bFpjEbgg1Q9JSqLxPp9Xjuxo6E0t/7zF/w/OqTbdXARCBh
djK8uDnJwWsWM8+9yrFTy7MHzL/mcwQx5BNRtmsum/8iM+c6vpujDZEgFcrFC5IP
SNXOihyzvHAyx7pk1y21Obp31oSivOatGTdFPEj8Ws4JOW0o789cBc8U85wJXYGE
6a3tmh0krnQU9cAD44Mx51gbbdDsAYfMcm41P8yxhEdFTH8o558Fv3EM4AdEj7i3
qoHxN/6YTdNrUR1DStT7uxer9NLvyvoCkmQ1Ju8qWJyeATyq23TDMaemUlQw1sDu
6JqMFWUqaOIjGYrbVMGT43oN1D491wKegmr+dB5tTK52X37RKYw=
=cOwr
-----END PGP SIGNATURE-----

--G806gToVJUOclRG4--