Date: Tue, 7 Jan 2020 10:27:41 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, starbugs@qq.com,
	pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #16190: The usage of NULL pointer in refint.c
Message-ID: <20200107012741.GD2386@paquier.xyz>
References: <16190-70181c803641c3dc@postgresql.org>
 <20200106051800.GL3598@paquier.xyz>
 <1234.1578291695@sss.pgh.pa.us>
 <20200106174443.3pwnmshbxrbwggrm@alap3.anarazel.de>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="IU5/I01NYhRvwH70"
Content-Disposition: inline
In-Reply-To: <20200106174443.3pwnmshbxrbwggrm@alap3.anarazel.de>
Precedence: bulk


--IU5/I01NYhRvwH70
Content-Type: multipart/mixed; boundary="ZJcv+A0YCCLh2VIg"
Content-Disposition: inline


--ZJcv+A0YCCLh2VIg
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Jan 06, 2020 at 09:44:43AM -0800, Andres Freund wrote:
> I think we should consider either moving this out of contrib, or fixing
> it up. test/example code is fine, but contrib gets installed by default
> for a lot of people... And yea, this isn't just about contrib/spi.

No idea about moving that out of contrib/, but here is a patch to fix
things that just moves the allocations to TopMemoryContext and removes
the system calls.
--
Michael

--ZJcv+A0YCCLh2VIg
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="refint-alloc-fixes.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/contrib/spi/refint.c b/contrib/spi/refint.c
index adf0490f85..05595c52c3 100644
--- a/contrib/spi/refint.c
+++ b/contrib/spi/refint.c
@@ -12,6 +12,7 @@
 #include "commands/trigger.h"
 #include "executor/spi.h"
 #include "utils/builtins.h"
+#include "utils/memutils.h"
 #include "utils/rel.h"
=20
 PG_MODULE_MAGIC;
@@ -186,12 +187,13 @@ check_primary_key(PG_FUNCTION_ARGS)
=20
 		/*
 		 * Remember that SPI_prepare places plan in current memory context -
-		 * so, we have to save plan in Top memory context for later use.
+		 * so, we have to save plan in TopMemoryContext for later use.
 		 */
 		if (SPI_keepplan(pplan))
 			/* internal error */
 			elog(ERROR, "check_primary_key: SPI_keepplan failed");
-		plan->splan =3D (SPIPlanPtr *) malloc(sizeof(SPIPlanPtr));
+		plan->splan =3D (SPIPlanPtr *) MemoryContextAlloc(TopMemoryContext,
+														sizeof(SPIPlanPtr));
 		*(plan->splan) =3D pplan;
 		plan->nplans =3D 1;
 	}
@@ -417,7 +419,8 @@ check_foreign_key(PG_FUNCTION_ARGS)
 		char		sql[8192];
 		char	  **args2 =3D args;
=20
-		plan->splan =3D (SPIPlanPtr *) malloc(nrefs * sizeof(SPIPlanPtr));
+		plan->splan =3D (SPIPlanPtr *) MemoryContextAlloc(TopMemoryContext,
+														nrefs * sizeof(SPIPlanPtr));
=20
 		for (r =3D 0; r < nrefs; r++)
 		{
@@ -614,6 +617,13 @@ find_plan(char *ident, EPlan **eplan, int *nplans)
 {
 	EPlan	   *newp;
 	int			i;
+	MemoryContext	oldcontext;
+
+	/*
+	 * All allocations done for the plans need to happen in a session-safe
+	 * context.
+	 */
+	oldcontext =3D MemoryContextSwitchTo(TopMemoryContext);
=20
 	if (*nplans > 0)
 	{
@@ -623,20 +633,24 @@ find_plan(char *ident, EPlan **eplan, int *nplans)
 				break;
 		}
 		if (i !=3D *nplans)
+		{
+			MemoryContextSwitchTo(oldcontext);
 			return (*eplan + i);
-		*eplan =3D (EPlan *) realloc(*eplan, (i + 1) * sizeof(EPlan));
+		}
+		*eplan =3D (EPlan *) repalloc(*eplan, (i + 1) * sizeof(EPlan));
 		newp =3D *eplan + i;
 	}
 	else
 	{
-		newp =3D *eplan =3D (EPlan *) malloc(sizeof(EPlan));
+		newp =3D *eplan =3D (EPlan *) palloc(sizeof(EPlan));
 		(*nplans) =3D i =3D 0;
 	}
=20
-	newp->ident =3D strdup(ident);
+	newp->ident =3D pstrdup(ident);
 	newp->nplans =3D 0;
 	newp->splan =3D NULL;
 	(*nplans)++;
=20
+	MemoryContextSwitchTo(oldcontext);
 	return newp;
 }

--ZJcv+A0YCCLh2VIg--

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAl4T3o0ACgkQnvQgOdby
QH1CLA/+IpLnl0DOXED0fAOETR9lSeyGVgkON3jPEBnJ+33Th2ord8nkbQCbkbFW
lno6wL1UO31bk5UO18fN0lh/cS1DJqNMbaBazUgI0i9dTdHSHH2YuS5BcMjN0XQ8
JNeCtlQ7VK7k3YBxDXFqqm8TD2XH3Y+Ccvd8//7GjqBoAhYpex9OzVGA9C2NMamg
ZYDt/PrRp2IL8vDerLqKeufv+MXbLd7H2+9qyfhEhRBhw2Z2IGuDqWUbvjokwB3w
PcjJIPaFESu1yGPsLAcHC8z/3Xo5+TEn1lsNvOfBL4f712FPIMvqZCOoAQPH7rIL
/0IVgSCScPK8w0COOwb2Fqo1sS67r7MoOxR6fzdV6+yArINnSKWu7vEG9MzpxZ5B
bJf5JEiaDl2Hlfmct6Na6CAlRyMezEssQcbpoDDOZM6JGCHhi+mo2dnXFTrcj9CB
PBX/j7CkF+piirk6ksXgBYT49PrYGQUGMC+VCXJRuBnocwqgxdSqu7I0OS0N3Eqg
FrlfIdGtCC3no02mc056027VY7x3dCJuRtK1166riofAthYNYrfh4drNM7g0Jdm0
kzva3qHbXuqkG9JqqigijPGshzAQRjyJ2JuWSLAV4u3q7zH6l9HKkLZo2wSxFiZ4
BBDQlZ14rwGFw/pxyIoFt2FpgopyRlzBG8k68BM6kUu6+bS+n1M=
=XhA6
-----END PGP SIGNATURE-----

--IU5/I01NYhRvwH70--