From 5c7038d70bb9c4d28a80b0a2051f73fafab5af3f Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Fri, 9 Feb 2024 15:03:16 +0100 Subject: Refactor pipe_read_line to return the full line Commit 5b2f4afffe6 refactored find_other_exec() and in the process created pipe_read_line() into a static routine for reading a single line of output, aimed at reading version numbers. Commit a7e8ece41 later exposed it externally in order to read a postgresql.conf GUC using "postgres -C ..". Further, f06b1c598 also made use of it for reading a version string much like find_other_exec(). The internal variable remained "pgver", even when used for other purposes. Since the function requires passing a buffer and its size, and at most size - 1 bytes will be read via fgets(), there is a truncation risk when using this for reading GUCs (like how pg_rewind does, though the risk in this case is marginal). To keep this as generic functionality for reading a line from a pipe, this refactors pipe_read_line() into returning an allocated buffer containing all of the line to remove the risk of silent truncation. Reviewed-by: Heikki Linnakangas Reviewed-by: Álvaro Herrera Reviewed-by: John Naylor Discussion: https://postgr.es/m/DEDF73CE-D528-49A3-9089-B3592FD671A9@yesql.se --- src/common/exec.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) (limited to 'src/common/exec.c') diff --git a/src/common/exec.c b/src/common/exec.c index 8cfc721b12a..da929f15b95 100644 --- a/src/common/exec.c +++ b/src/common/exec.c @@ -42,6 +42,8 @@ #endif #endif +#include "common/string.h" + /* Inhibit mingw CRT's auto-globbing of command line arguments */ #if defined(WIN32) && !defined(_MSC_VER) extern int _CRT_glob = 0; /* 0 turns off globbing; 1 turns it on */ @@ -328,7 +330,7 @@ find_other_exec(const char *argv0, const char *target, const char *versionstr, char *retpath) { char cmd[MAXPGPATH]; - char line[MAXPGPATH]; + char *line; if (find_my_exec(argv0, retpath) < 0) return -1; @@ -346,46 +348,55 @@ find_other_exec(const char *argv0, const char *target, snprintf(cmd, sizeof(cmd), "\"%s\" -V", retpath); - if (!pipe_read_line(cmd, line, sizeof(line))) + if ((line = pipe_read_line(cmd)) == NULL) return -1; if (strcmp(line, versionstr) != 0) + { + pfree(line); return -2; + } + pfree(line); return 0; } /* - * Execute a command in a pipe and read the first line from it. + * Execute a command in a pipe and read the first line from it. The returned + * string is palloc'd (malloc'd in frontend code), the caller is responsible + * for freeing. */ char * -pipe_read_line(char *cmd, char *line, int maxsize) +pipe_read_line(char *cmd) { - FILE *pgver; + FILE *pipe_cmd; + char *line; fflush(NULL); errno = 0; - if ((pgver = popen(cmd, "r")) == NULL) + if ((pipe_cmd = popen(cmd, "r")) == NULL) { perror("popen failure"); return NULL; } + /* Make sure popen() didn't change errno */ errno = 0; - if (fgets(line, maxsize, pgver) == NULL) + line = pg_get_line(pipe_cmd, NULL); + + if (line == NULL) { - if (feof(pgver)) - fprintf(stderr, "no data was returned by command \"%s\"\n", cmd); + if (ferror(pipe_cmd)) + log_error(errcode_for_file_access(), + _("could not read from command \"%s\": %m"), cmd); else - perror("fgets failure"); - pclose(pgver); /* no error checking */ - return NULL; + log_error(errcode_for_file_access(), + _("no data was returned by command \"%s\": %m"), cmd); } - if (pclose_check(pgver)) - return NULL; + (void) pclose_check(pipe_cmd); return line; } -- cgit v1.2.3