agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Jonathan Gonzalez V. <jonathan@abdiel.eu>
Subject: [PATCH 1/1] replace calls to system() with PostgreSQL own implementation
Date: Mon, 3 Aug 2026 15:21:45 -0400
Introduce PCommand and psystem() to execute commands from argument
arrays, avoiding the shell on Unix.
Windows implementation still on a work in progress
---
src/backend/utils/init/postinit.c | 58 ---------
src/bin/pg_ctl/pg_ctl.c | 30 ++---
src/common/Makefile | 1 +
src/common/exec.c | 59 +++++++++
src/common/meson.build | 2 +
src/common/pg_exec.c | 206 ++++++++++++++++++++++++++++++
src/include/common/pg_exec.h | 39 ++++++
src/include/miscadmin.h | 1 -
src/include/port.h | 2 +
9 files changed, 324 insertions(+), 74 deletions(-)
create mode 100644 src/common/pg_exec.c
create mode 100644 src/include/common/pg_exec.h
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 3d8c9bdebd5..78ea7d56022 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -492,64 +492,6 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
ReleaseSysCache(tup);
}
-
-/*
- * pg_split_opts -- split a string of options and append it to an argv array
- *
- * The caller is responsible for ensuring the argv array is large enough. The
- * maximum possible number of arguments added by this routine is
- * (strlen(optstr) + 1) / 2.
- *
- * Because some option values can contain spaces we allow escaping using
- * backslashes, with \\ representing a literal backslash.
- */
-void
-pg_split_opts(char **argv, int *argcp, const char *optstr)
-{
- StringInfoData s;
-
- initStringInfo(&s);
-
- while (*optstr)
- {
- bool last_was_escape = false;
-
- resetStringInfo(&s);
-
- /* skip over leading space */
- while (isspace((unsigned char) *optstr))
- optstr++;
-
- if (*optstr == '\0')
- break;
-
- /*
- * Parse a single option, stopping at the first space, unless it's
- * escaped.
- */
- while (*optstr)
- {
- if (isspace((unsigned char) *optstr) && !last_was_escape)
- break;
-
- if (!last_was_escape && *optstr == '\\')
- last_was_escape = true;
- else
- {
- last_was_escape = false;
- appendStringInfoChar(&s, *optstr);
- }
-
- optstr++;
- }
-
- /* now store the option in the next argv[] position */
- argv[(*argcp)++] = pstrdup(s.data);
- }
-
- pfree(s.data);
-}
-
/*
* Initialize MaxBackends value from config options.
*
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 6c604e2d962..19e884bac1f 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -25,6 +25,7 @@
#include "common/controldata_utils.h"
#include "common/file_perm.h"
#include "common/logging.h"
+#include "common/pg_exec.h"
#include "common/string.h"
#include "datatype/timestamp.h"
#include "getopt_long.h"
@@ -42,6 +43,7 @@ typedef enum
IMMEDIATE_MODE,
} ShutdownMode;
+
typedef enum
{
POSTMASTER_READY,
@@ -904,26 +906,18 @@ find_other_exec_or_die(const char *argv0, const char *target, const char *versio
static void
do_init(void)
{
- char *cmd;
+ PCommand *cmd;
if (exec_path == NULL)
exec_path = find_other_exec_or_die(argv0, "initdb", "initdb (PostgreSQL) " PG_VERSION "\n");
- if (pgdata_opt == NULL)
- pgdata_opt = "";
-
- if (post_opts == NULL)
- post_opts = "";
-
- if (!silent_mode)
- cmd = psprintf("\"%s\" %s%s",
- exec_path, pgdata_opt, post_opts);
- else
- cmd = psprintf("\"%s\" %s%s > \"%s\"",
- exec_path, pgdata_opt, post_opts, DEVNULL);
+ cmd = pcommand_init(exec_path, "initdb");
+ pcommand_append_arg(cmd, pgdata_opt);
+ pcommand_append_arg(cmd, post_opts);
+ cmd->silent = silent_mode;
fflush(NULL);
- if (system(cmd) != 0)
+ if (psystem(cmd) != 0)
{
write_stderr(_("%s: database system initialization failed\n"), progname);
exit(1);
@@ -2155,6 +2149,12 @@ adjust_data_dir(void)
else
my_exec_path = pg_strdup(exec_path);
+/*
+ * command = pcommand_init(my_exec_path, "postgres");
+ * pcommand_append_arg(command, "-C data_directory");
+ * pcommand_append_arg(command, pgdata_opt);
+ * pcommand_append_arg(command, post_opts);
+ */
/* it's important for -C to be the first option, see main.c */
cmd = psprintf("\"%s\" -C data_directory %s%s",
my_exec_path,
@@ -2287,7 +2287,7 @@ main(int argc, char **argv)
* We could pass PGDATA just in an environment variable
* but we do -D too for clearer postmaster 'ps' display
*/
- pgdata_opt = psprintf("-D \"%s\" ", pgdata_D);
+ pgdata_opt = psprintf("-D %s ", pgdata_D);
pg_free(pgdata_D);
break;
}
diff --git a/src/common/Makefile b/src/common/Makefile
index 1a2fbbe887f..753399f023a 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -68,6 +68,7 @@ OBJS_COMMON = \
md5_common.o \
parse_manifest.o \
percentrepl.o \
+ pg_exec.o \
pg_get_line.o \
pg_lzcompress.o \
pg_prng.o \
diff --git a/src/common/exec.c b/src/common/exec.c
index 2881aa92ca6..c5a31c17a95 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -33,6 +33,7 @@
#include <sys/stat.h>
#include <sys/wait.h>
#include <unistd.h>
+#include <fcntl.h>
#ifdef EXEC_BACKEND
#if defined(HAVE_SYS_PERSONALITY_H)
@@ -43,6 +44,7 @@
#endif
#include "common/string.h"
+#include "lib/stringinfo.h"
/* Inhibit mingw CRT's auto-globbing of command line arguments */
#if defined(WIN32) && !defined(_MSC_VER)
@@ -711,3 +713,60 @@ GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser)
}
#endif
+
+/*
+ * pg_split_opts -- split a string of options and append it to an argv array
+ *
+ * The caller is responsible for ensuring the argv array is large enough. The
+ * maximum possible number of arguments added by this routine is
+ * (strlen(optstr) + 1) / 2.
+ *
+ * Because some option values can contain spaces we allow escaping using
+ * backslashes, with \\ representing a literal backslash.
+ */
+void
+pg_split_opts(char **argv, int *argcp, const char *optstr)
+{
+ StringInfoData s;
+
+ initStringInfo(&s);
+
+ while (*optstr)
+ {
+ bool last_was_escape = false;
+
+ resetStringInfo(&s);
+
+ /* skip over leading space */
+ while (isspace((unsigned char) *optstr))
+ optstr++;
+
+ if (*optstr == '\0')
+ break;
+
+ /*
+ * Parse a single option, stopping at the first space, unless it's
+ * escaped.
+ */
+ while (*optstr)
+ {
+ if (isspace((unsigned char) *optstr) && !last_was_escape)
+ break;
+
+ if (!last_was_escape && *optstr == '\\')
+ last_was_escape = true;
+ else
+ {
+ last_was_escape = false;
+ appendStringInfoChar(&s, *optstr);
+ }
+
+ optstr++;
+ }
+
+ /* now store the option in the next argv[] position */
+ argv[(*argcp)++] = pstrdup(s.data);
+ }
+
+ pfree(s.data);
+}
diff --git a/src/common/meson.build b/src/common/meson.build
index 9bd55cda95b..06ebb4dd874 100644
--- a/src/common/meson.build
+++ b/src/common/meson.build
@@ -22,6 +22,7 @@ common_sources = files(
'md5_common.c',
'parse_manifest.c',
'percentrepl.c',
+ 'pg_exec.c',
'pg_get_line.c',
'pg_lzcompress.c',
'pg_prng.c',
@@ -190,6 +191,7 @@ foreach name, opts : pgcommon_variants
kwargs: opts + {
'include_directories': [
include_directories('.'),
+ include_directories('../interfaces/libpq'),
opts.get('include_directories', []),
],
'dependencies': opts['dependencies'] + [ssl],
diff --git a/src/common/pg_exec.c b/src/common/pg_exec.c
new file mode 100644
index 00000000000..bb5613216f2
--- /dev/null
+++ b/src/common/pg_exec.c
@@ -0,0 +1,206 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_exec.c
+ * Functions fo execute and manage the input/output of commands
+ *
+ *
+ * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ * src/common/pg_exec.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include <unistd.h>
+#include <sys/wait.h>
+#include <fcntl.h>
+
+#include "c.h"
+
+#include "postgres.h"
+#include "common/pg_exec.h"
+#include "utils/palloc.h"
+
+#ifdef WIN32
+#include "fe_utils/string_utils.h"
+#endif
+
+static int
+pcommand_count_args(const char *arg)
+{
+ int count = 0;
+ bool in_word = false;
+
+ Assert(arg != NULL);
+
+ while (*arg)
+ {
+ if (isspace((unsigned char) *arg))
+ in_word = false;
+ else if (!in_word)
+ {
+ count++;
+ in_word = true;
+ }
+
+ arg++;
+ }
+
+ return count;
+}
+
+void
+pcommand_append_arg(PCommand * cmd, const char *arg)
+{
+ int args_count;
+
+ if (arg == NULL)
+ return;
+
+ args_count = pcommand_count_args(arg);
+
+ if (args_count == 0)
+ return;
+
+ if (cmd->argc + args_count >= cmd->nalloc)
+ {
+ cmd->nalloc += args_count + 1;
+ cmd->argv = repalloc_array(cmd->argv, char *, cmd->nalloc);
+ }
+
+ if (args_count > 1)
+ {
+ pg_split_opts(cmd->argv, &cmd->argc, arg);
+ cmd->argv[cmd->argc] = NULL;
+ }
+ else
+ {
+
+ cmd->argv[cmd->argc++] = pstrdup(arg);
+ cmd->argv[cmd->argc] = NULL;
+ }
+}
+
+
+#ifdef WIN32
+int
+pcommand_win(PCommand * cmd)
+{
+ PQExpBufferData cmd_str;
+ int i;
+
+ initPQExpBuffer(&cmd_str);
+
+ appendShellString(&cmd_str, cmd->path);
+
+ for (i = 1; i < cmd->argc; i++)
+ {
+ appendPQExpBufferChar(&cmd_str, ' ');
+ appendShellString(&cmd_str, cmd->argv[i]);
+ }
+
+ if (cmd->silent)
+ {
+ appendPQExpBufferStr(&cmd_str, " > ");
+ appendShellString(&cmd_str, DEVNULL);
+ }
+
+ return system(cmd_str.data);
+}
+#endif
+
+PCommand *
+pcommand_init(char *path, char *command)
+{
+ PCommand *cmd = palloc(sizeof(PCommand));
+
+ cmd->path = path;
+ cmd->command = command;
+
+ cmd->argc = 0;
+ cmd->nalloc = 2;
+ cmd->argv = palloc_array(char *, cmd->nalloc);
+
+ pcommand_append_arg(cmd, command);
+
+ cmd->stdin_fd = -1;
+ cmd->stdout_fd = -1;
+ cmd->stderr_fd = -1;
+
+ return cmd;
+}
+
+int
+pcommand_exec(PCommand * cmd)
+{
+ if (cmd->stdin_fd >= 0 && dup2(cmd->stdin_fd, STDIN_FILENO) < 0)
+ return errno;
+ if (cmd->stdout_fd >= 0 && dup2(cmd->stdout_fd, STDOUT_FILENO) < 0)
+ return errno;
+ if (cmd->stderr_fd >= 0 && dup2(cmd->stderr_fd, STDERR_FILENO) < 0)
+ return errno;
+
+ execv(cmd->path, cmd->argv);
+ return errno;
+}
+
+
+#ifndef WIN32
+int
+pcommand_wait(PCommand * cmd)
+{
+ int status = -1;
+
+ /* This should have some kind of timeout... do we have that somewhere else */
+ while (true)
+ {
+ if (waitpid(cmd->pid, &status, 0) < 0)
+ {
+ if (errno == EINTR)
+ continue;
+ return status;
+ }
+ else
+ {
+ return status;
+ }
+ }
+}
+#endif
+
+/*
+ * psystem() is a replacement for system(3) that avoids the use of a shell
+ * It's not a full replacement yet since it doesn't support pipes. For now it
+ * should be used only in places where is known that no pipe is requried and the
+ * commands being call are known commands.
+ * This funciton shouldn't be used as a replacement for system(3) call in places
+ * like RestoreArchivedFile() or shell_archive_file()
+ *
+ * The return value is always the return of the executed command
+ */
+int
+psystem(PCommand * cmd)
+{
+#ifdef WIN32
+ return pcommand_win(cmd);
+#else
+
+ cmd->pid = fork();
+ if (cmd->pid < 0)
+ return -1;
+
+ if (cmd->pid == 0)
+ {
+ if (cmd->silent)
+ cmd->stdout_fd = open(DEVNULL, O_WRONLY);
+
+ errno = pcommand_exec(cmd);
+ _exit(127);
+ }
+
+ return pcommand_wait(cmd);
+#endif
+}
diff --git a/src/include/common/pg_exec.h b/src/include/common/pg_exec.h
new file mode 100644
index 00000000000..d987449c615
--- /dev/null
+++ b/src/include/common/pg_exec.h
@@ -0,0 +1,39 @@
+/*-------------------------------------------------------------------------
+ * Exec commands for backend/frontend programs
+ *
+ * Copyright (c) 2018-2026, PostgreSQL Global Development Group
+ *
+ * src/include/common/pg_exec.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef COMMON_PG_EXEC_H
+#define COMMON_PG_EXEC_H
+
+typedef struct PCommand
+{
+ pid_t pid;
+ char *path;
+ char *command;
+ char *command_win;
+ char **argv;
+ int argc;
+ int nalloc;
+ bool silent;
+ int stdin_fd;
+ int stdout_fd;
+ int stderr_fd;
+} PCommand;
+
+extern PCommand * pcommand_init(char *path, char *command);
+extern void pcommand_append_arg(PCommand * cmd, const char *arg);
+#ifdef WIN32
+extern int pcommand_win(PCommand *cmd);
+#endif
+#ifndef WIN32
+extern int pcommand_wait(PCommand *cmd);
+#endif
+extern int pcommand_exec(PCommand * cmd);
+extern int psystem(PCommand * command);
+
+#endif
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 0fc59af02b9..41d4c62c531 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -512,7 +512,6 @@ extern PGDLLIMPORT ProcessingMode Mode;
#define INIT_PG_LOAD_SESSION_LIBS 0x0001
#define INIT_PG_OVERRIDE_ALLOW_CONNS 0x0002
#define INIT_PG_OVERRIDE_ROLE_LOGIN 0x0004
-extern void pg_split_opts(char **argv, int *argcp, const char *optstr);
extern void InitializeMaxBackends(void);
extern void InitializeFastPathLocks(void);
extern void InitPostgres(const char *in_dbname, Oid dboid,
diff --git a/src/include/port.h b/src/include/port.h
index 172acf7d02f..82d900fcbc4 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -140,6 +140,8 @@ extern int find_my_exec(const char *argv0, char *retpath);
extern int find_other_exec(const char *argv0, const char *target,
const char *versionstr, char *retpath);
extern char *pipe_read_line(char *cmd);
+extern void pg_split_opts(char **argv, int *argcp, const char *optstr);
+
/* Doesn't belong here, but this is used with find_other_exec(), so... */
#define PG_BACKEND_VERSIONSTR "postgres (PostgreSQL) " PG_VERSION "\n"
--
2.53.0
--=-=-=--
Message-ID: <no-message-id-1305627@localhost>
Permalink: ../no-message-id-1305627@localhost/
Also on: postgresql.org/message-id/no-message-id-1305627@localhost
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: pgsql-hackers@postgresql.org
Cc: jonathan@abdiel.eu
Subject: Re: [PATCH 1/1] replace calls to system() with PostgreSQL own implementation
In-Reply-To: <no-message-id-1305627@localhost>
* 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