Clean up command argument assembly
This is a small code cleanup patch.
Several commands internally assemble command lines to call other
commands. This includes initdb, pg_dumpall, and pg_regress. (Also
pg_ctl, but that is different enough that I didn't consider it here.)
This has all evolved a bit organically, with fixed-size buffers, and
various optional command-line arguments being injected with
confusing-looking code, and the spacing between options handled in
inconsistent ways. This patch cleans all this up a bit to look clearer
and be more easily extensible with new arguments and options. We start
each command with printfPQExpBuffer(), and then append arguments as
necessary with appendPQExpBuffer(). Also standardize on using
initPQExpBuffer() over createPQExpBuffer() where possible. pg_regress
uses StringInfo instead of PQExpBuffer, but many of the same ideas apply.
Attachments:
0001-Clean-up-command-argument-assembly.patchtext/plain; charset=UTF-8; name=0001-Clean-up-command-argument-assembly.patchDownload
From dbd9d7b7ff2c069a131c9efa8bff2597c0d9e1c8 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Mon, 26 Jun 2023 11:30:24 +0200
Subject: [PATCH] Clean up command argument assembly
Several commands internally assemble command lines to call other
commands. This includes initdb, pg_dumpall, and pg_regress. (Also
pg_ctl, but that is different enough that I didn't consider it here.)
This has all evolved a bit organically, with fixed-size buffers, and
various optional command-line arguments being injected with
confusing-looking code, and the spacing between options handled in
inconsistent ways. Clean all this up a bit to look clearer and be
more easily extensible with new arguments and options. We start each
command with printfPQExpBuffer(), and then append arguments as
necessary with appendPQExpBuffer(). Also standardize on using
initPQExpBuffer() over createPQExpBuffer() where possible. pg_regress
uses StringInfo instead of PQExpBuffer, but many of the same ideas
apply.
---
src/bin/initdb/initdb.c | 56 +++++++++++++++++++----------------
src/bin/pg_dump/pg_dumpall.c | 27 +++++++++--------
src/test/regress/pg_regress.c | 26 +++++++++-------
3 files changed, 62 insertions(+), 47 deletions(-)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index fc1fb363e7..3f4167682a 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -309,16 +309,16 @@ void initialize_data_directory(void);
/*
* macros for running pipes to postgres
*/
-#define PG_CMD_DECL char cmd[MAXPGPATH]; FILE *cmdfd
+#define PG_CMD_DECL FILE *cmdfd
-#define PG_CMD_OPEN \
+#define PG_CMD_OPEN(cmd) \
do { \
cmdfd = popen_check(cmd, "w"); \
if (cmdfd == NULL) \
exit(1); /* message already printed by popen_check */ \
} while (0)
-#define PG_CMD_CLOSE \
+#define PG_CMD_CLOSE() \
do { \
if (pclose_check(cmdfd)) \
exit(1); /* message already printed by pclose_check */ \
@@ -1138,13 +1138,15 @@ test_config_settings(void)
static bool
test_specific_config_settings(int test_conns, int test_buffs)
{
- PQExpBuffer cmd = createPQExpBuffer();
+ PQExpBufferData cmd;
_stringlist *gnames,
*gvalues;
int status;
+ initPQExpBuffer(&cmd);
+
/* Set up the test postmaster invocation */
- printfPQExpBuffer(cmd,
+ printfPQExpBuffer(&cmd,
"\"%s\" --check %s %s "
"-c max_connections=%d "
"-c shared_buffers=%d "
@@ -1158,18 +1160,18 @@ test_specific_config_settings(int test_conns, int test_buffs)
gnames != NULL; /* assume lists have the same length */
gnames = gnames->next, gvalues = gvalues->next)
{
- appendPQExpBuffer(cmd, " -c %s=", gnames->str);
- appendShellString(cmd, gvalues->str);
+ appendPQExpBuffer(&cmd, " -c %s=", gnames->str);
+ appendShellString(&cmd, gvalues->str);
}
- appendPQExpBuffer(cmd,
+ appendPQExpBuffer(&cmd,
" < \"%s\" > \"%s\" 2>&1",
DEVNULL, DEVNULL);
fflush(NULL);
- status = system(cmd->data);
+ status = system(cmd.data);
- destroyPQExpBuffer(cmd);
+ termPQExpBuffer(&cmd);
return (status == 0);
}
@@ -1469,6 +1471,7 @@ static void
bootstrap_template1(void)
{
PG_CMD_DECL;
+ PQExpBufferData cmd;
char **line;
char **bki_lines;
char headerline[MAXPGPATH];
@@ -1530,16 +1533,17 @@ bootstrap_template1(void)
/* Also ensure backend isn't confused by this environment var: */
unsetenv("PGCLIENTENCODING");
- snprintf(cmd, sizeof(cmd),
- "\"%s\" --boot -X %d %s %s %s %s",
- backend_exec,
- wal_segment_size_mb * (1024 * 1024),
- data_checksums ? "-k" : "",
- boot_options, extra_options,
- debug ? "-d 5" : "");
+ initPQExpBuffer(&cmd);
+
+ printfPQExpBuffer(&cmd, "\"%s\" --boot %s %s", backend_exec, boot_options, extra_options);
+ appendPQExpBuffer(&cmd, " -X %d", wal_segment_size_mb * (1024 * 1024));
+ if (data_checksums)
+ appendPQExpBuffer(&cmd, " -k");
+ if (debug)
+ appendPQExpBuffer(&cmd, " -d 5");
- PG_CMD_OPEN;
+ PG_CMD_OPEN(cmd.data);
for (line = bki_lines; *line != NULL; line++)
{
@@ -1547,8 +1551,9 @@ bootstrap_template1(void)
free(*line);
}
- PG_CMD_CLOSE;
+ PG_CMD_CLOSE();
+ termPQExpBuffer(&cmd);
free(bki_lines);
check_ok();
@@ -2951,6 +2956,7 @@ void
initialize_data_directory(void)
{
PG_CMD_DECL;
+ PQExpBufferData cmd;
int i;
setup_signals();
@@ -3014,12 +3020,11 @@ initialize_data_directory(void)
fputs(_("performing post-bootstrap initialization ... "), stdout);
fflush(stdout);
- snprintf(cmd, sizeof(cmd),
- "\"%s\" %s %s template1 >%s",
- backend_exec, backend_options, extra_options,
- DEVNULL);
+ initPQExpBuffer(&cmd);
+ printfPQExpBuffer(&cmd, "\"%s\" %s %s template1 >%s",
+ backend_exec, backend_options, extra_options, DEVNULL);
- PG_CMD_OPEN;
+ PG_CMD_OPEN(cmd.data);
setup_auth(cmdfd);
@@ -3054,7 +3059,8 @@ initialize_data_directory(void)
make_postgres(cmdfd);
- PG_CMD_CLOSE;
+ PG_CMD_CLOSE();
+ termPQExpBuffer(&cmd);
check_ok();
}
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 3627b69e2a..36f134137f 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1564,11 +1564,14 @@ dumpDatabases(PGconn *conn)
static int
runPgDump(const char *dbname, const char *create_opts)
{
- PQExpBuffer connstrbuf = createPQExpBuffer();
- PQExpBuffer cmd = createPQExpBuffer();
+ PQExpBufferData connstrbuf;
+ PQExpBufferData cmd;
int ret;
- appendPQExpBuffer(cmd, "\"%s\" %s %s", pg_dump_bin,
+ initPQExpBuffer(&connstrbuf);
+ initPQExpBuffer(&cmd);
+
+ printfPQExpBuffer(&cmd, "\"%s\" %s %s", pg_dump_bin,
pgdumpopts->data, create_opts);
/*
@@ -1576,27 +1579,27 @@ runPgDump(const char *dbname, const char *create_opts)
* format.
*/
if (filename)
- appendPQExpBufferStr(cmd, " -Fa ");
+ appendPQExpBufferStr(&cmd, " -Fa ");
else
- appendPQExpBufferStr(cmd, " -Fp ");
+ appendPQExpBufferStr(&cmd, " -Fp ");
/*
* Append the database name to the already-constructed stem of connection
* string.
*/
- appendPQExpBuffer(connstrbuf, "%s dbname=", connstr);
- appendConnStrVal(connstrbuf, dbname);
+ appendPQExpBuffer(&connstrbuf, "%s dbname=", connstr);
+ appendConnStrVal(&connstrbuf, dbname);
- appendShellString(cmd, connstrbuf->data);
+ appendShellString(&cmd, connstrbuf.data);
- pg_log_info("running \"%s\"", cmd->data);
+ pg_log_info("running \"%s\"", cmd.data);
fflush(NULL);
- ret = system(cmd->data);
+ ret = system(cmd.data);
- destroyPQExpBuffer(cmd);
- destroyPQExpBuffer(connstrbuf);
+ termPQExpBuffer(&cmd);
+ termPQExpBuffer(&connstrbuf);
return ret;
}
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 60d34a40b2..3e4fd918dd 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -2293,6 +2293,7 @@ regression_main(int argc, char *argv[],
if (temp_instance)
{
+ StringInfoData cmd;
FILE *pg_conf;
const char *env_wait;
int wait_seconds;
@@ -2318,23 +2319,28 @@ regression_main(int argc, char *argv[],
make_directory(buf);
/* initdb */
- snprintf(buf, sizeof(buf),
- "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync%s%s > \"%s/log/initdb.log\" 2>&1",
- bindir ? bindir : "",
- bindir ? "/" : "",
- temp_instance,
- debug ? " --debug" : "",
- nolocale ? " --no-locale" : "",
- outputdir);
+ initStringInfo(&cmd);
+ appendStringInfo(&cmd,
+ "\"%s%sinitdb\" -D \"%s/data\" --no-clean --no-sync",
+ bindir ? bindir : "",
+ bindir ? "/" : "",
+ temp_instance);
+ if (debug)
+ appendStringInfo(&cmd, " --debug");
+ if (nolocale)
+ appendStringInfo(&cmd, " --no-locale");
+ appendStringInfo(&cmd, " > \"%s/log/initdb.log\" 2>&1", outputdir);
fflush(NULL);
- if (system(buf))
+ if (system(cmd.data))
{
bail("initdb failed\n"
"# Examine \"%s/log/initdb.log\" for the reason.\n"
"# Command was: %s",
- outputdir, buf);
+ outputdir, cmd.data);
}
+ pfree(cmd.data);
+
/*
* Adjust the default postgresql.conf for regression testing. The user
* can specify a file to be appended; in any case we expand logging
--
2.41.0
On 26/06/2023 12:33, Peter Eisentraut wrote:
This is a small code cleanup patch.
Several commands internally assemble command lines to call other
commands. This includes initdb, pg_dumpall, and pg_regress. (Also
pg_ctl, but that is different enough that I didn't consider it here.)
This has all evolved a bit organically, with fixed-size buffers, and
various optional command-line arguments being injected with
confusing-looking code, and the spacing between options handled in
inconsistent ways. This patch cleans all this up a bit to look clearer
and be more easily extensible with new arguments and options.
+1
We start each command with printfPQExpBuffer(), and then append
arguments as necessary with appendPQExpBuffer(). Also standardize on
using initPQExpBuffer() over createPQExpBuffer() where possible.
pg_regress uses StringInfo instead of PQExpBuffer, but many of the
same ideas apply.
It's a bit bogus to use PQExpBuffer for these. If you run out of memory,
you silently get an empty string instead. StringInfo, which exits the
process on OOM, would be more appropriate. We have tons of such
inappropriate uses of PQExpBuffer in all our client programs, though, so
I don't insist on fixing this particular case right now.
--
Heikki Linnakangas
Neon (https://neon.tech)
On 04.07.23 14:14, Heikki Linnakangas wrote:
On 26/06/2023 12:33, Peter Eisentraut wrote:
This is a small code cleanup patch.
Several commands internally assemble command lines to call other
commands. This includes initdb, pg_dumpall, and pg_regress. (Also
pg_ctl, but that is different enough that I didn't consider it here.)
This has all evolved a bit organically, with fixed-size buffers, and
various optional command-line arguments being injected with
confusing-looking code, and the spacing between options handled in
inconsistent ways. This patch cleans all this up a bit to look clearer
and be more easily extensible with new arguments and options.+1
committed
We start each command with printfPQExpBuffer(), and then append
arguments as necessary with appendPQExpBuffer(). Also standardize on
using initPQExpBuffer() over createPQExpBuffer() where possible.
pg_regress uses StringInfo instead of PQExpBuffer, but many of the
same ideas apply.It's a bit bogus to use PQExpBuffer for these. If you run out of memory,
you silently get an empty string instead. StringInfo, which exits the
process on OOM, would be more appropriate. We have tons of such
inappropriate uses of PQExpBuffer in all our client programs, though, so
I don't insist on fixing this particular case right now.
Interesting point. But as you say better dealt with as a separate problem.
On 05.07.23 07:22, Peter Eisentraut wrote:
It's a bit bogus to use PQExpBuffer for these. If you run out of
memory, you silently get an empty string instead. StringInfo, which
exits the process on OOM, would be more appropriate. We have tons of
such inappropriate uses of PQExpBuffer in all our client programs,
though, so I don't insist on fixing this particular case right now.Interesting point. But as you say better dealt with as a separate problem.
I was inspired by a33e17f210 (for pg_rewind) to clean up some more
fixed-buffer command assembly and replace it with extensible buffers and
some more elegant code. And then I remembered this thread, and it's
really a continuation of this.
The first patch deals with pg_regress and pg_isolation_regress. It is
pretty straightforward.
The second patch deals with pg_upgrade. It would require exporting
appendPQExpBufferVA() from libpq, which might be overkill. But this
gets to your point earlier: Should pg_upgrade rather be using
StringInfo instead of PQExpBuffer? (Then we'd use appendStringInfoVA(),
which already exists, but even if not we wouldn't need to change libpq
to get it.) Should anything outside of libpq be using PQExpBuffer?
Attachments:
0001-Use-extensible-buffers-to-assemble-command-lines.patchtext/plain; charset=UTF-8; name=0001-Use-extensible-buffers-to-assemble-command-lines.patchDownload
From dc3da0dce85a69314fff0a03998fd89b4ca19d8c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Sun, 4 Feb 2024 18:32:16 +0100
Subject: [PATCH 1/2] Use extensible buffers to assemble command lines
This makes use of StringInfo to assemble command lines, instead of
using fixed-size buffers and the (remote) possibility of "command too
long" errors. Also makes the code a bit simpler.
This covers the test driver programs pg_regress and
pg_isolation_regress.
Similar to the changes done for pg_rewind in a33e17f210.
---
src/test/isolation/isolation_main.c | 37 ++++++++++----------------
src/test/regress/pg_regress_main.c | 41 +++++++++++------------------
2 files changed, 30 insertions(+), 48 deletions(-)
diff --git a/src/test/isolation/isolation_main.c b/src/test/isolation/isolation_main.c
index 05e81035c1f..2a3e41d2101 100644
--- a/src/test/isolation/isolation_main.c
+++ b/src/test/isolation/isolation_main.c
@@ -12,6 +12,7 @@
#include "postgres_fe.h"
+#include "lib/stringinfo.h"
#include "pg_regress.h"
char saved_argv0[MAXPGPATH];
@@ -34,8 +35,7 @@ isolation_start_test(const char *testname,
char infile[MAXPGPATH];
char outfile[MAXPGPATH];
char expectfile[MAXPGPATH];
- char psql_cmd[MAXPGPATH * 3];
- size_t offset = 0;
+ StringInfoData psql_cmd;
char *appnameenv;
/* need to do the path lookup here, check isolation_init() for details */
@@ -75,34 +75,23 @@ isolation_start_test(const char *testname,
add_stringlist_item(resultfiles, outfile);
add_stringlist_item(expectfiles, expectfile);
+ initStringInfo(&psql_cmd);
+
if (launcher)
- {
- offset += snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset,
- "%s ", launcher);
- if (offset >= sizeof(psql_cmd))
- {
- fprintf(stderr, _("command too long\n"));
- exit(2);
- }
- }
+ appendStringInfo(&psql_cmd, "%s ", launcher);
- offset += snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset,
- "\"%s\" \"dbname=%s\" < \"%s\" > \"%s\" 2>&1",
- isolation_exec,
- dblist->str,
- infile,
- outfile);
- if (offset >= sizeof(psql_cmd))
- {
- fprintf(stderr, _("command too long\n"));
- exit(2);
- }
+ appendStringInfo(&psql_cmd,
+ "\"%s\" \"dbname=%s\" < \"%s\" > \"%s\" 2>&1",
+ isolation_exec,
+ dblist->str,
+ infile,
+ outfile);
appnameenv = psprintf("isolation/%s", testname);
setenv("PGAPPNAME", appnameenv, 1);
free(appnameenv);
- pid = spawn_process(psql_cmd);
+ pid = spawn_process(psql_cmd.data);
if (pid == INVALID_PID)
{
@@ -113,6 +102,8 @@ isolation_start_test(const char *testname,
unsetenv("PGAPPNAME");
+ pfree(psql_cmd.data);
+
return pid;
}
diff --git a/src/test/regress/pg_regress_main.c b/src/test/regress/pg_regress_main.c
index 18155ef97e2..d607a3023b2 100644
--- a/src/test/regress/pg_regress_main.c
+++ b/src/test/regress/pg_regress_main.c
@@ -18,6 +18,7 @@
#include "postgres_fe.h"
+#include "lib/stringinfo.h"
#include "pg_regress.h"
/*
@@ -34,8 +35,7 @@ psql_start_test(const char *testname,
char infile[MAXPGPATH];
char outfile[MAXPGPATH];
char expectfile[MAXPGPATH];
- char psql_cmd[MAXPGPATH * 3];
- size_t offset = 0;
+ StringInfoData psql_cmd;
char *appnameenv;
/*
@@ -62,40 +62,29 @@ psql_start_test(const char *testname,
add_stringlist_item(resultfiles, outfile);
add_stringlist_item(expectfiles, expectfile);
+ initStringInfo(&psql_cmd);
+
if (launcher)
- {
- offset += snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset,
- "%s ", launcher);
- if (offset >= sizeof(psql_cmd))
- {
- fprintf(stderr, _("command too long\n"));
- exit(2);
- }
- }
+ appendStringInfo(&psql_cmd, "%s ", launcher);
/*
* Use HIDE_TABLEAM to hide different AMs to allow to use regression tests
* against different AMs without unnecessary differences.
*/
- offset += snprintf(psql_cmd + offset, sizeof(psql_cmd) - offset,
- "\"%s%spsql\" -X -a -q -d \"%s\" %s < \"%s\" > \"%s\" 2>&1",
- bindir ? bindir : "",
- bindir ? "/" : "",
- dblist->str,
- "-v HIDE_TABLEAM=on -v HIDE_TOAST_COMPRESSION=on",
- infile,
- outfile);
- if (offset >= sizeof(psql_cmd))
- {
- fprintf(stderr, _("command too long\n"));
- exit(2);
- }
+ appendStringInfo(&psql_cmd,
+ "\"%s%spsql\" -X -a -q -d \"%s\" %s < \"%s\" > \"%s\" 2>&1",
+ bindir ? bindir : "",
+ bindir ? "/" : "",
+ dblist->str,
+ "-v HIDE_TABLEAM=on -v HIDE_TOAST_COMPRESSION=on",
+ infile,
+ outfile);
appnameenv = psprintf("pg_regress/%s", testname);
setenv("PGAPPNAME", appnameenv, 1);
free(appnameenv);
- pid = spawn_process(psql_cmd);
+ pid = spawn_process(psql_cmd.data);
if (pid == INVALID_PID)
{
@@ -106,6 +95,8 @@ psql_start_test(const char *testname,
unsetenv("PGAPPNAME");
+ pfree(psql_cmd.data);
+
return pid;
}
base-commit: 774bcffe4a9853a24e61d758637c0aad2871f1fb
--
2.43.0
0002-WIP-pg_upgrade-No-more-command-too-long.patchtext/plain; charset=UTF-8; name=0002-WIP-pg_upgrade-No-more-command-too-long.patchDownload
From 5c47c810568020830f9a44fa701d261bb91d1c3b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Sun, 4 Feb 2024 18:32:16 +0100
Subject: [PATCH 2/2] WIP: pg_upgrade: No more "command too long"
Use extensive buffers to assemble command lines instead of fixed size.
Similar to the changes done for pg_rewind in a33e17f210.
XXX requires exporting appendPQExpBufferVA() from libpq
XXX should pg_upgrade be using StringInfo instead?
---
src/bin/pg_upgrade/exec.c | 37 ++++++++++++++++----------------
src/interfaces/libpq/exports.txt | 1 +
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c
index fec8dc4c2f7..2ea231d6f62 100644
--- a/src/bin/pg_upgrade/exec.c
+++ b/src/bin/pg_upgrade/exec.c
@@ -13,6 +13,7 @@
#include "common/string.h"
#include "pg_upgrade.h"
+#include "pqexpbuffer.h"
static void check_data_dir(ClusterInfo *cluster);
static void check_bin_dir(ClusterInfo *cluster, bool check_versions);
@@ -87,13 +88,11 @@ exec_prog(const char *log_filename, const char *opt_log_file,
bool report_error, bool exit_on_error, const char *fmt,...)
{
int result = 0;
- int written;
char log_file[MAXPGPATH];
-
-#define MAXCMDLEN (2 * MAXPGPATH)
- char cmd[MAXCMDLEN];
+ PQExpBufferData cmd;
FILE *log;
va_list ap;
+ bool done;
#ifdef WIN32
static DWORD mainThreadId = 0;
@@ -105,18 +104,16 @@ exec_prog(const char *log_filename, const char *opt_log_file,
snprintf(log_file, MAXPGPATH, "%s/%s", log_opts.logdir, log_filename);
- written = 0;
- va_start(ap, fmt);
- written += vsnprintf(cmd + written, MAXCMDLEN - written, fmt, ap);
- va_end(ap);
- if (written >= MAXCMDLEN)
- pg_fatal("command too long");
- written += snprintf(cmd + written, MAXCMDLEN - written,
- " >> \"%s\" 2>&1", log_file);
- if (written >= MAXCMDLEN)
- pg_fatal("command too long");
+ initPQExpBuffer(&cmd);
+ do
+ {
+ va_start(ap, fmt);
+ done = appendPQExpBufferVA(&cmd, fmt, ap);
+ va_end(ap);
+ } while (!done);
+ appendPQExpBuffer(&cmd, " >> \"%s\" 2>&1", log_file);
- pg_log(PG_VERBOSE, "%s", cmd);
+ pg_log(PG_VERBOSE, "%s", cmd.data);
#ifdef WIN32
@@ -132,7 +129,7 @@ exec_prog(const char *log_filename, const char *opt_log_file,
if (mainThreadId != GetCurrentThreadId())
{
fflush(NULL);
- result = system(cmd);
+ result = system(cmd.data);
}
#endif
@@ -165,7 +162,7 @@ exec_prog(const char *log_filename, const char *opt_log_file,
if (mainThreadId == GetCurrentThreadId())
fprintf(log, "\n\n");
#endif
- fprintf(log, "command: %s\n", cmd);
+ fprintf(log, "command: %s\n", cmd.data);
#ifdef WIN32
/* Are we printing "command:" after its output? */
if (mainThreadId != GetCurrentThreadId())
@@ -184,7 +181,7 @@ exec_prog(const char *log_filename, const char *opt_log_file,
#endif
{
fflush(NULL);
- result = system(cmd);
+ result = system(cmd.data);
}
if (result != 0 && report_error)
@@ -193,7 +190,7 @@ exec_prog(const char *log_filename, const char *opt_log_file,
report_status(PG_REPORT, "\n*failure*");
fflush(stdout);
- pg_log(PG_VERBOSE, "There were problems executing \"%s\"", cmd);
+ pg_log(PG_VERBOSE, "There were problems executing \"%s\"", cmd.data);
if (opt_log_file)
pg_log(exit_on_error ? PG_FATAL : PG_REPORT,
"Consult the last few lines of \"%s\" or \"%s\" for\n"
@@ -221,6 +218,8 @@ exec_prog(const char *log_filename, const char *opt_log_file,
fclose(log);
#endif
+ termPQExpBuffer(&cmd);
+
return result == 0;
}
diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt
index 088592deb16..2fc5130e7bf 100644
--- a/src/interfaces/libpq/exports.txt
+++ b/src/interfaces/libpq/exports.txt
@@ -193,3 +193,4 @@ PQsendClosePrepared 190
PQsendClosePortal 191
PQchangePassword 192
PQsendPipelineSync 193
+appendPQExpBufferVA 194
--
2.43.0
Peter Eisentraut <peter@eisentraut.org> writes:
Should anything outside of libpq be using PQExpBuffer?
Perhaps not. PQExpBuffer's behavior for OOM cases is designed
specifically for libpq, where exit-on-OOM is not okay and we
can hope to include failure checks wherever needed. For most
of our application code, we'd much rather just exit-on-OOM
and not have to think about failure checks at the call sites.
Having said that, converting stuff like pg_dump would be quite awful
in terms of code churn and creating a back-patching nightmare.
Would it make any sense to think about having two sets of
routines with identical call APIs, but different failure
behavior, so that we don't have to touch the callers?
regards, tom lane