public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Euler Taveira <[email protected]>
Cc: [email protected] <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Small memory fixes for pg_createsubcriber
Date: Thu, 13 Feb 2025 08:56:53 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <xseczrku4nm3p4rujapxiiuttpqrpagkbwrr2nlbnmwemh525y@3he54emhpqha>
References: <CAEudQAp=AW5dJXrGLbC_aZg_9nOo=42W7uLDRONFQE-gcgnkgQ@mail.gmail.com>
	<[email protected]>
	<obqvpv4yx57lbnfiqvy4pmnfi2l5wn6xzoq2ftytxa426pa226@2sc46frt5xtb>
	<[email protected]>
	<xseczrku4nm3p4rujapxiiuttpqrpagkbwrr2nlbnmwemh525y@3he54emhpqha>

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:

  [text/x-diff] libpq-escape-fixes.patch (847B, ../[email protected]/2-libpq-escape-fixes.patch)
  download | inline diff:
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);
 


  [application/pgp-signature] signature.asc (833B, ../[email protected]/3-signature.asc)
  download

view thread (17+ 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], [email protected], [email protected], [email protected]
  Subject: Re: Small memory fixes for pg_createsubcriber
  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