[Michael Paquier <michael@paquier.xyz>]

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.
> 
> 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

Attachments:

libpq-escape-fixes.patch (text/x-diff, 846B) download
diff --git a/src/bin/pg_amcheck/pg_amcheck.c b/src/bin/pg_amcheck/pg_amcheck.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[])
 
 			executeCommand(conn, install_sql, opts.echo);
 			pfree(install_sql);
-			pfree(schema);
+			PQfreemem(schema);
 		}
 
 		/*
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.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)
 
 	PQfreemem(datcollate_literal);
 	PQfreemem(datctype_literal);
-	PQfreemem(datlocale_literal);
+	if (locale->db_locale)
+		PQfreemem(datlocale_literal);
+	else
+		pg_free(datlocale_literal);
 
 	PQfinish(conn_new_template1);
 

  signature.asc (application/pgp-signature, 832B) download