On 2026-Mar-09, Michael Paquier wrote:
> On Mon, Mar 09, 2026 at 11:21:35AM +0800, yangyz wrote:
> > I think it should be modified.
> >
> > Move createPQExpBuffer inside the conditional block to match its destroy
> > counterpart.
> > This improves code clarity and satisfies static analyzers, even though the
> > actual memory
> > leak is minimal in practice.
>
> destroyPQExpBuffer() is called for each tuple from pg_database except
> if dealing with "template{0,1}" or "postgres". It means that we would
> just leak a few bytes for these three cases. I agree that the
> variable declaration can be placed better, but it's really not worth
> bothering in this context.
True, but at the same time it looks as if this routine is wastefully
written -- I mean, why spend time with a stringinfo here at all? We
could write this in much simpler form, as in the attached, which is even
three lines shorter. In fact, before 763aaa06f034, this is exactly how
this routine was written, and I don't see why it was changed this way.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Just treat us the way you want to be treated + some extra allowance
for ignorance." (Michael Brusser)
>From f2cb8d62fea60a9a5834830f67b6e945665a9a4f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email protected]>
Date: Mon, 9 Mar 2026 13:39:45 +0100
Subject: [PATCH v2] simplify coding
---
src/bin/pg_dump/pg_dumpall.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 4ded9020952..3084c05e236 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1833,7 +1833,6 @@ dropDBs(PGconn *conn)
for (i = 0; i < PQntuples(res); i++)
{
char *dbname = PQgetvalue(res, i, 0);
- PQExpBuffer delQry = createPQExpBuffer();
/*
* Skip "postgres" and "template1"; dumpDatabases() will deal with
@@ -1846,15 +1845,14 @@ dropDBs(PGconn *conn)
{
if (archDumpFormat == archNull)
{
- appendPQExpBuffer(delQry, "DROP DATABASE %s%s;\n",
- if_exists ? "IF EXISTS " : "",
- fmtId(dbname));
- fprintf(OPF, "%s", delQry->data);
+ fprintf(OPF, "DROP DATABASE %s%s;\n",
+ if_exists ? "IF EXISTS " : "",
+ fmtId(dbname));
}
else
{
- appendPQExpBuffer(delQry, "DROP DATABASE IF EXISTS %s;\n",
- fmtId(dbname));
+ char *stmt = psprintf("DROP DATABASE IF EXISTS %s;\n",
+ fmtId(dbname));
ArchiveEntry(fout,
nilCatalogId, /* catalog ID */
@@ -1862,10 +1860,9 @@ dropDBs(PGconn *conn)
ARCHIVE_OPTS(.tag = psprintf("DATABASE %s", fmtId(dbname)),
.description = "DROP_GLOBAL",
.section = SECTION_PRE_DATA,
- .createStmt = delQry->data));
+ .createStmt = stmt));
+ pg_free(stmt);
}
-
- destroyPQExpBuffer(delQry);
}
}
--
2.47.3
