From 22707f3f0c5a160d9483136a198b64dc1fbb9d82 Mon Sep 17 00:00:00 2001 From: "Jonathan Gonzalez V." Date: Mon, 3 Aug 2026 15:21:45 -0400 Subject: [PATCH 1/1] replace calls to system() with PostgreSQL own implementation 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 #include #include +#include #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 +#include +#include + +#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