Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 5c7038d

Browse filesBrowse files
danielgustafssondanielgustafsson
committed
Refactor pipe_read_line to return the full line
Commit 5b2f4af 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 a7e8ece later exposed it externally in order to read a postgresql.conf GUC using "postgres -C ..". Further, f06b1c5 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 <hlinnaka@iki.fi> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: John Naylor <johncnaylorls@gmail.com> Discussion: https://postgr.es/m/DEDF73CE-D528-49A3-9089-B3592FD671A9@yesql.se
1 parent c01f6ef commit 5c7038d
Copy full SHA for 5c7038d

File tree

Expand file treeCollapse file tree

4 files changed

+36
-25
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+36
-25
lines changed

‎src/bin/pg_rewind/pg_rewind.c

Copy file name to clipboardExpand all lines: src/bin/pg_rewind/pg_rewind.c
+6-8Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,8 +1055,7 @@ static void
10551055
getRestoreCommand(const char *argv0)
10561056
{
10571057
int rc;
1058-
char postgres_exec_path[MAXPGPATH],
1059-
cmd_output[MAXPGPATH];
1058+
char postgres_exec_path[MAXPGPATH];
10601059
PQExpBuffer postgres_cmd;
10611060

10621061
if (!restore_wal)
@@ -1105,16 +1104,15 @@ getRestoreCommand(const char *argv0)
11051104
/* add -C switch, for restore_command */
11061105
appendPQExpBufferStr(postgres_cmd, " -C restore_command");
11071106

1108-
if (!pipe_read_line(postgres_cmd->data, cmd_output, sizeof(cmd_output)))
1109-
exit(1);
1107+
restore_command = pipe_read_line(postgres_cmd->data);
1108+
if (restore_command == NULL)
1109+
pg_fatal("unable to read restore_command from target cluster");
11101110

1111-
(void) pg_strip_crlf(cmd_output);
1111+
(void) pg_strip_crlf(restore_command);
11121112

1113-
if (strcmp(cmd_output, "") == 0)
1113+
if (strcmp(restore_command, "") == 0)
11141114
pg_fatal("restore_command is not set in the target cluster");
11151115

1116-
restore_command = pg_strdup(cmd_output);
1117-
11181116
pg_log_debug("using for rewind restore_command = \'%s\'",
11191117
restore_command);
11201118

‎src/bin/pg_upgrade/exec.c

Copy file name to clipboardExpand all lines: src/bin/pg_upgrade/exec.c
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ static void
431431
check_exec(const char *dir, const char *program, bool check_version)
432432
{
433433
char path[MAXPGPATH];
434-
char line[MAXPGPATH];
434+
char *line;
435435
char cmd[MAXPGPATH];
436436
char versionstr[128];
437437

@@ -442,7 +442,7 @@ check_exec(const char *dir, const char *program, bool check_version)
442442

443443
snprintf(cmd, sizeof(cmd), "\"%s\" -V", path);
444444

445-
if (!pipe_read_line(cmd, line, sizeof(line)))
445+
if ((line = pipe_read_line(cmd)) == NULL)
446446
pg_fatal("check for \"%s\" failed: cannot execute",
447447
path);
448448

@@ -456,4 +456,6 @@ check_exec(const char *dir, const char *program, bool check_version)
456456
pg_fatal("check for \"%s\" failed: incorrect version: found \"%s\", expected \"%s\"",
457457
path, line, versionstr);
458458
}
459+
460+
pg_free(line);
459461
}

‎src/common/exec.c

Copy file name to clipboardExpand all lines: src/common/exec.c
+25-14Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
#endif
4343
#endif
4444

45+
#include "common/string.h"
46+
4547
/* Inhibit mingw CRT's auto-globbing of command line arguments */
4648
#if defined(WIN32) && !defined(_MSC_VER)
4749
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,
328330
const char *versionstr, char *retpath)
329331
{
330332
char cmd[MAXPGPATH];
331-
char line[MAXPGPATH];
333+
char *line;
332334

333335
if (find_my_exec(argv0, retpath) < 0)
334336
return -1;
@@ -346,46 +348,55 @@ find_other_exec(const char *argv0, const char *target,
346348

347349
snprintf(cmd, sizeof(cmd), "\"%s\" -V", retpath);
348350

349-
if (!pipe_read_line(cmd, line, sizeof(line)))
351+
if ((line = pipe_read_line(cmd)) == NULL)
350352
return -1;
351353

352354
if (strcmp(line, versionstr) != 0)
355+
{
356+
pfree(line);
353357
return -2;
358+
}
354359

360+
pfree(line);
355361
return 0;
356362
}
357363

358364

359365
/*
360-
* Execute a command in a pipe and read the first line from it.
366+
* Execute a command in a pipe and read the first line from it. The returned
367+
* string is palloc'd (malloc'd in frontend code), the caller is responsible
368+
* for freeing.
361369
*/
362370
char *
363-
pipe_read_line(char *cmd, char *line, int maxsize)
371+
pipe_read_line(char *cmd)
364372
{
365-
FILE *pgver;
373+
FILE *pipe_cmd;
374+
char *line;
366375

367376
fflush(NULL);
368377

369378
errno = 0;
370-
if ((pgver = popen(cmd, "r")) == NULL)
379+
if ((pipe_cmd = popen(cmd, "r")) == NULL)
371380
{
372381
perror("popen failure");
373382
return NULL;
374383
}
375384

385+
/* Make sure popen() didn't change errno */
376386
errno = 0;
377-
if (fgets(line, maxsize, pgver) == NULL)
387+
line = pg_get_line(pipe_cmd, NULL);
388+
389+
if (line == NULL)
378390
{
379-
if (feof(pgver))
380-
fprintf(stderr, "no data was returned by command \"%s\"\n", cmd);
391+
if (ferror(pipe_cmd))
392+
log_error(errcode_for_file_access(),
393+
_("could not read from command \"%s\": %m"), cmd);
381394
else
382-
perror("fgets failure");
383-
pclose(pgver); /* no error checking */
384-
return NULL;
395+
log_error(errcode_for_file_access(),
396+
_("no data was returned by command \"%s\": %m"), cmd);
385397
}
386398

387-
if (pclose_check(pgver))
388-
return NULL;
399+
(void) pclose_check(pipe_cmd);
389400

390401
return line;
391402
}

‎src/include/port.h

Copy file name to clipboardExpand all lines: src/include/port.h
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ extern int validate_exec(const char *path);
137137
extern int find_my_exec(const char *argv0, char *retpath);
138138
extern int find_other_exec(const char *argv0, const char *target,
139139
const char *versionstr, char *retpath);
140-
extern char *pipe_read_line(char *cmd, char *line, int maxsize);
140+
extern char *pipe_read_line(char *cmd);
141141

142142
/* Doesn't belong here, but this is used with find_other_exec(), so... */
143143
#define PG_BACKEND_VERSIONSTR "postgres (PostgreSQL) " PG_VERSION "\n"

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.