diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2015-12-03 14:28:58 -0500 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2015-12-03 14:29:28 -0500 |
commit | 344cdff2c1541e7a1249299a33723aabeafa0b0c (patch) | |
tree | fe16426718eb6f1abd388ba0db94c31aa8e80f38 /src/bin/psql/print.c | |
parent | f15b820a5c60b10f3ac1b2fdb37d534ecb0a4bf8 (diff) |
Clean up some psql issues around handling of the query output file.
Formerly, if "psql -o foo" failed to open the output file "foo", it would
print an error message but then carry on as though -o had not been
specified at all. This seems contrary to expectation: a program that
cannot open its output file normally fails altogether. Make psql do
exit(1) after reporting the error.
If "\o foo" failed to open "foo", it would print an error message but then
reset the output file to stdout, as if the argument had been omitted.
This is likewise pretty surprising behavior. Make it keep the previous
output state, instead.
psql keeps SIGPIPE interrupts disabled when it is writing to a pipe, either
a pipe specified by -o/\o or a transient pipe opened for purposes such as
using a pager on query output. The logic for this was too simple and could
sometimes re-enable SIGPIPE when a -o pipe was still active, thus possibly
leading to an unexpected psql crash later.
Fixing the last point required getting rid of the kluge in PrintQueryTuples
and ExecQueryUsingCursor whereby they'd transiently change the global
queryFout state, but that seems like good cleanup anyway.
Back-patch to 9.5 but not further; these are minor-enough issues that
changing the behavior in stable branches doesn't seem appropriate.
Diffstat (limited to 'src/bin/psql/print.c')
-rw-r--r-- | src/bin/psql/print.c | 78 |
1 files changed, 65 insertions, 13 deletions
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c index 190f2bc5d85..05d4b3162c3 100644 --- a/src/bin/psql/print.c +++ b/src/bin/psql/print.c @@ -39,6 +39,13 @@ */ volatile bool cancel_pressed = false; +/* + * Likewise, the sigpipe_trap and pager open/close functions are here rather + * than in common.c so that this file can be used by non-psql programs. + */ +static bool always_ignore_sigpipe = false; + + /* info for locale-aware numeric formatting; set up by setDecimalLocale() */ static char *decimal_point; static int groupdigits; @@ -2775,11 +2782,62 @@ print_troff_ms_vertical(const printTableContent *cont, FILE *fout) /********************************/ -/* Public functions */ +/* Public functions */ /********************************/ /* + * disable_sigpipe_trap + * + * Turn off SIGPIPE interrupt --- call this before writing to a temporary + * query output file that is a pipe. + * + * No-op on Windows, where there's no SIGPIPE interrupts. + */ +void +disable_sigpipe_trap(void) +{ +#ifndef WIN32 + pqsignal(SIGPIPE, SIG_IGN); +#endif +} + +/* + * restore_sigpipe_trap + * + * Restore normal SIGPIPE interrupt --- call this when done writing to a + * temporary query output file that was (or might have been) a pipe. + * + * Note: within psql, we enable SIGPIPE interrupts unless the permanent query + * output file is a pipe, in which case they should be kept off. This + * approach works only because psql is not currently complicated enough to + * have nested usages of short-lived output files. Otherwise we'd probably + * need a genuine save-and-restore-state approach; but for now, that would be + * useless complication. In non-psql programs, this always enables SIGPIPE. + * + * No-op on Windows, where there's no SIGPIPE interrupts. + */ +void +restore_sigpipe_trap(void) +{ +#ifndef WIN32 + pqsignal(SIGPIPE, always_ignore_sigpipe ? SIG_IGN : SIG_DFL); +#endif +} + +/* + * set_sigpipe_trap_state + * + * Set the trap state that restore_sigpipe_trap should restore to. + */ +void +set_sigpipe_trap_state(bool ignore) +{ + always_ignore_sigpipe = ignore; +} + + +/* * PageOutput * * Tests if pager is needed and returns appropriate FILE pointer. @@ -2792,9 +2850,6 @@ PageOutput(int lines, const printTableOpt *topt) /* check whether we need / can / are supposed to use pager */ if (topt && topt->pager && isatty(fileno(stdin)) && isatty(fileno(stdout))) { - const char *pagerprog; - FILE *pagerpipe; - #ifdef TIOCGWINSZ unsigned short int pager = topt->pager; int min_lines = topt->pager_min_lines; @@ -2807,20 +2862,19 @@ PageOutput(int lines, const printTableOpt *topt) if (result == -1 || (lines >= screen_size.ws_row && lines >= min_lines) || pager > 1) - { #endif + { + const char *pagerprog; + FILE *pagerpipe; + pagerprog = getenv("PAGER"); if (!pagerprog) pagerprog = DEFAULT_PAGER; -#ifndef WIN32 - pqsignal(SIGPIPE, SIG_IGN); -#endif + disable_sigpipe_trap(); pagerpipe = popen(pagerprog, "w"); if (pagerpipe) return pagerpipe; -#ifdef TIOCGWINSZ } -#endif } return stdout; @@ -2848,9 +2902,7 @@ ClosePager(FILE *pagerpipe) fprintf(pagerpipe, _("Interrupted\n")); pclose(pagerpipe); -#ifndef WIN32 - pqsignal(SIGPIPE, SIG_DFL); -#endif + restore_sigpipe_trap(); } } |