Hi list, Here's the second patch from my coccicheck run. Originally it flagged the fact that the opened file in psql's process_file() wasn't being closed in the ON_ERROR_STOP path, but there seem to be two more unintended behaviors here.
(1) In the error path, the value of pset.inputfile wasn't being properly restored. The caller does free(fname) on line 786, so psql.inputfile would point to unallocated memory. (2) The more significant issue is that stdin *was closed in the success return path. So when you run a script with two "\i -" lines, the first "\q" would close stdin and the next one would fail with: psql:-:0: could not read from input file: Bad file descriptor In fact, this means that stdin was being accessed after being fclose()d, which is undefined behavior per ANSI C, though it seems to work just fine on Linux. The new behavior requires the same amount of "\q"s as the number of executions of '-' because stdin is never closed. Regards, Marti
From 43b7595fdcc69cc9db0d066a53f53c5e71c965aa Mon Sep 17 00:00:00 2001 From: Marti Raudsepp <ma...@juffo.org> Date: Wed, 20 Oct 2010 23:44:36 +0300 Subject: [PATCH] psql: Don't close stdin, don't leak file descriptor with ON_ERROR_STOP Changes three things: 1. Don't close stdin when returning success if input was stdin 2. Don't leak file descriptor when exiting due to ON_ERROR_STOP 3. pset.inputfile wasn't being restored with ON_ERROR_STOP, yet the memory was freed by the caller (1) changes the behavior of "\q" on stdin. Previously multiple inclusions of stdin would be terminated with a single quit, now a separate quit is needed for each invocation. Previous behavior also accessed stdin after it was fclose()d, which is undefined behavior per ANSI C. (2) and (3) should have no practical impact, because the process would quit immediately afterwards anyway. --- src/bin/psql/command.c | 15 ++++++++++++--- 1 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index e6d703a..45a145c 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -1987,7 +1987,10 @@ process_file(char *filename, bool single_txn) if ((res = PSQLexec("BEGIN", false)) == NULL) { if (pset.on_error_stop) - return EXIT_USER; + { + result = EXIT_USER; + goto error; + } } else PQclear(res); @@ -2000,13 +2003,19 @@ process_file(char *filename, bool single_txn) if ((res = PSQLexec("COMMIT", false)) == NULL) { if (pset.on_error_stop) - return EXIT_USER; + { + result = EXIT_USER; + goto error; + } } else PQclear(res); } - fclose(fd); +error: + if(fd != stdin) + fclose(fd); + pset.inputfile = oldfilename; return result; } -- 1.7.3.1
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers