Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 13 Feb 2025 08:56:53 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Euler Taveira <euler@eulerto.com>,
	"ranier.vf@gmail.com" <ranier.vf@gmail.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Small memory fixes for pg_createsubcriber
Message-ID: <Z601RQxTmIUohdkV@paquier.xyz>
References: <CAEudQAp=AW5dJXrGLbC_aZg_9nOo=42W7uLDRONFQE-gcgnkgQ@mail.gmail.com>
 <68d6d810-4ef1-4f62-b47f-89b21501249b@app.fastmail.com>
 <obqvpv4yx57lbnfiqvy4pmnfi2l5wn6xzoq2ftytxa426pa226@2sc46frt5xtb>
 <3337507.1739376124@sss.pgh.pa.us>
 <xseczrku4nm3p4rujapxiiuttpqrpagkbwrr2nlbnmwemh525y@3he54emhpqha>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="qjhby8jojjfBvt5H"
Content-Disposition: inline
In-Reply-To: <xseczrku4nm3p4rujapxiiuttpqrpagkbwrr2nlbnmwemh525y@3he54emhpqha>
Archived-At: <https://www.postgresql.org/message-id/Z601RQxTmIUohdkV%40paquier.xyz>
Precedence: bulk


--qjhby8jojjfBvt5H
Content-Type: multipart/mixed; boundary="Yxgq/EINaf3jsJOi"
Content-Disposition: inline


--Yxgq/EINaf3jsJOi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 12, 2025 at 12:00:03PM -0500, Andres Freund wrote:
> Particularly for something like libpq it's not quitetrivial to add
> attributes like this, of course. We can't even depend on pg_config.h.
>=20
> One way would be to define them in libpq-fe.h, guarded by an #ifdef, that=
's
> "armed" by a commandline -D flag, if the compiler is supported?

Interesting set of tricks.

I have looked at bit at the uses of PQescapeLiteral() and
PQescapeIdentifier() in the tree.  On top of the one in pg_amcheck you
are just pointing to, there is an inconsistency in pg_upgrade.c for
set_locale_and_encoding() where datlocale_literal may be allocated
with a pg_strdup() or a PQescapeLiteral() depending on the path.  The
code has been using PQfreemem() for the pg_strdup() allocation, which
is logically incorrect.

The pg_upgrade path is fancy as designed this way, for sure, but
that's less invasive than hardcoding three times "NULL".  Any thoughts
about backpatching something like that for both of them?
--
Michael

--Yxgq/EINaf3jsJOi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="libpq-escape-fixes.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amchec=
k.c
index c5ec25be22b..dda8b1b8ade 100644
--- a/src/bin/pg_amcheck/pg_amcheck.c
+++ b/src/bin/pg_amcheck/pg_amcheck.c
@@ -560,7 +560,7 @@ main(int argc, char *argv[])
=20
 			executeCommand(conn, install_sql, opts.echo);
 			pfree(install_sql);
-			pfree(schema);
+			PQfreemem(schema);
 		}
=20
 		/*
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrad=
e.c
index 36c7f3879d5..d854ad110ee 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -465,7 +465,10 @@ set_locale_and_encoding(void)
=20
 	PQfreemem(datcollate_literal);
 	PQfreemem(datctype_literal);
-	PQfreemem(datlocale_literal);
+	if (locale->db_locale)
+		PQfreemem(datlocale_literal);
+	else
+		pg_free(datlocale_literal);
=20
 	PQfinish(conn_new_template1);
=20

--Yxgq/EINaf3jsJOi--

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmetNUUACgkQnvQgOdby
QH0VsBAAn7WfQHrFIiR+XCHFoXYhAKNMxKIbEmRrOZHUiFxAFSS96ilhK6l9rYzW
BDOaYOxYMi+D+MDveehywl7igNHViH8fkWRefLFuqRkgFz6Lzr4RTtSFKmqx4qw1
ZoTkwOOmtpBMEf9ZrBp9wHUJ71+YRuOCQoc3Xw0p3XizOv50Br+cIlQWgd8L1+un
QLCEb1/hKAxJx7S7BJFZV+JhqDs08fWzow3Jyazxt6J0IrgDV8mN/EWbuqmTy4Zl
Mh71ERa+9kW9GtEky/3GtNrWGKVLpwpbKAjVezdGVEtUe05Kcv/SgL7/Mzmk2h2u
yHOSBW3MqdkJIrtwq+CO9yjeBYKIChUmnCqL6kF1zIoniafQg1cnAYctHCM+5YY/
hn4OPBIciSLIlWsw8RBjtkI7LHd3WTyo8xS4tbqOm7IpT5Zj7dWKokGAtWN5LzKv
2pVRAxf9OWJL7bqeZiVKYBXuMGdYh+C2q2v22X06XeHxwbYInhifgZEgEop/GdYw
mZT2bFWHvNLfaTQhE2Mtzf+FLZcLV/O085Oc644df5OVgB9iqjYXY7Vqa7pfaBbY
F7nHMi47EuPpslWz3T8ZrMXIkeUA84igJ7qXPya9E1NyOTjUIl6hc9h8e3XKpce0
WQvDhTFVqhM2l4UOnUANFkS4HPGTHltFMdlV2DQ/bLq/3nwfCSo=
=/T85
-----END PGP SIGNATURE-----

--qjhby8jojjfBvt5H--