From e2e2a9db4c6581b3839fc8139f7d33c6770316b9 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 11 Jun 2006 23:06:00 +0000
Subject: [PATCH] Code review for psql multiline history patch(es). Fix memory
leak, failure to enter commands in history if canceled by control-C, other
infelicities.
---
src/bin/psql/command.c | 12 +---
src/bin/psql/help.c | 3 +-
src/bin/psql/input.c | 127 ++++++++++++++++------------------
src/bin/psql/input.h | 10 +--
src/bin/psql/mainloop.c | 134 +++++++++++++++++-------------------
src/bin/psql/prompt.c | 3 +-
src/bin/psql/tab-complete.c | 4 +-
7 files changed, 132 insertions(+), 161 deletions(-)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 3c5bd6740f1..a9d8a07f835 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3,7 +3,7 @@
*
* Copyright (c) 2000-2006, PostgreSQL Global Development Group
*
- * $PostgreSQL: pgsql/src/bin/psql/command.c,v 1.169 2006/06/07 22:24:45 momjian Exp $
+ * $PostgreSQL: pgsql/src/bin/psql/command.c,v 1.170 2006/06/11 23:06:00 tgl Exp $
*/
#include "postgres_fe.h"
#include "command.h"
@@ -1361,7 +1361,7 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf)
}
else
{
- /* read file back in */
+ /* read file back into query_buf */
char line[1024];
resetPQExpBuffer(query_buf);
@@ -1374,14 +1374,6 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf)
error = true;
}
-#ifdef USE_READLINE
-#ifdef HAVE_REPLACE_HISTORY_ENTRY
-
- replace_history_entry(where_history(), query_buf->data, NULL);
-#else
- add_history(query_buf->data);
-#endif
-#endif
fclose(stream);
}
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index dd3bdef0434..6bc0b73d08e 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -3,11 +3,10 @@
*
* Copyright (c) 2000-2006, PostgreSQL Global Development Group
*
- * $PostgreSQL: pgsql/src/bin/psql/help.c,v 1.110 2006/03/05 15:58:51 momjian Exp $
+ * $PostgreSQL: pgsql/src/bin/psql/help.c,v 1.111 2006/06/11 23:06:00 tgl Exp $
*/
#include "postgres_fe.h"
#include "common.h"
-#include "pqexpbuffer.h"
#include "input.h"
#include "print.h"
#include "help.h"
diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c
index 1dc36d9dbc5..b970175b805 100644
--- a/src/bin/psql/input.c
+++ b/src/bin/psql/input.c
@@ -3,12 +3,12 @@
*
* Copyright (c) 2000-2006, PostgreSQL Global Development Group
*
- * $PostgreSQL: pgsql/src/bin/psql/input.c,v 1.53 2006/03/21 13:38:12 momjian Exp $
+ * $PostgreSQL: pgsql/src/bin/psql/input.c,v 1.54 2006/06/11 23:06:00 tgl Exp $
*/
#include "postgres_fe.h"
-#include "pqexpbuffer.h"
#include "input.h"
+#include "pqexpbuffer.h"
#include "settings.h"
#include "tab-complete.h"
#include "common.h"
@@ -78,110 +78,89 @@ GetHistControlConfig(void)
#endif
-static char *
-gets_basic(const char prompt[])
-{
- fputs(prompt, stdout);
- fflush(stdout);
- return gets_fromFile(stdin);
-}
-
-
/*
* gets_interactive()
*
- * Gets a line of interactive input, using readline of desired.
+ * Gets a line of interactive input, using readline if desired.
* The result is malloc'ed.
*/
char *
gets_interactive(const char *prompt)
{
#ifdef USE_READLINE
- char *s;
-
if (useReadline)
+ {
/* On some platforms, readline is declared as readline(char *) */
- s = readline((char *) prompt);
- else
- s = gets_basic(prompt);
-
- return s;
-#else
- return gets_basic(prompt);
+ return readline((char *) prompt);
+ }
#endif
+
+ fputs(prompt, stdout);
+ fflush(stdout);
+ return gets_fromFile(stdin);
}
-/* Put the line in the history buffer and also add the trailing \n */
+/*
+ * Append the line to the history buffer, making sure there is a trailing '\n'
+ */
void
-pg_append_history(char *s, PQExpBuffer history_buf)
+pg_append_history(const char *s, PQExpBuffer history_buf)
{
#ifdef USE_READLINE
-
- int slen;
- if (useReadline && useHistory && s && s[0])
+ if (useHistory && s && s[0])
{
- slen = strlen(s);
- if (s[slen-1] == '\n')
- appendPQExpBufferStr(history_buf, s);
- else
- {
- appendPQExpBufferStr(history_buf, s);
+ appendPQExpBufferStr(history_buf, s);
+ if (s[strlen(s) - 1] != '\n')
appendPQExpBufferChar(history_buf, '\n');
- }
}
#endif
}
/*
- * Feed the string to readline
+ * Emit accumulated history entry to readline's history mechanism,
+ * then reset the buffer to empty.
+ *
+ * Note: we write nothing if history_buf is empty, so extra calls to this
+ * function don't hurt. There must have been at least one line added by
+ * pg_append_history before we'll do anything.
*/
void
-pg_write_history(char *s)
+pg_send_history(PQExpBuffer history_buf)
{
#ifdef USE_READLINE
- static char *prev_hist;
- int slen, i;
-
- if (useReadline && useHistory )
+ static char *prev_hist = NULL;
+
+ char *s = history_buf->data;
+
+ if (useHistory && s[0])
{
- enum histcontrol HC;
-
- /* Flushing of empty buffer should do nothing */
- if (*s == 0)
- return;
-
- prev_hist = NULL;
-
- HC = GetHistControlConfig();
+ enum histcontrol HC = GetHistControlConfig();
if (((HC & hctl_ignorespace) && s[0] == ' ') ||
- ((HC & hctl_ignoredups) && prev_hist && strcmp(s, prev_hist) == 0))
+ ((HC & hctl_ignoredups) && prev_hist && strcmp(s, prev_hist) == 0))
{
/* Ignore this line as far as history is concerned */
}
else
{
- free(prev_hist);
- slen = strlen(s);
- /* Trim the trailing \n's */
- for (i = slen-1; i >= 0 && s[i] == '\n'; i--)
+ int i;
+
+ /* Trim any trailing \n's (OK to scribble on history_buf) */
+ for (i = strlen(s)-1; i >= 0 && s[i] == '\n'; i--)
;
s[i + 1] = '\0';
+ /* Save each previous line for ignoredups processing */
+ if (prev_hist)
+ free(prev_hist);
prev_hist = pg_strdup(s);
+ /* And send it to readline */
add_history(s);
}
}
-#endif
-}
-void
-pg_clear_history(PQExpBuffer history_buf)
-{
-#ifdef USE_READLINE
- if (useReadline && useHistory)
- resetPQExpBuffer(history_buf);
+ resetPQExpBuffer(history_buf);
#endif
}
@@ -219,6 +198,9 @@ gets_fromFile(FILE *source)
#ifdef USE_READLINE
+/*
+ * Convert newlines to NL_IN_HISTORY for safe saving in readline history file
+ */
static void
encode_history(void)
{
@@ -232,6 +214,9 @@ encode_history(void)
*cur_ptr = NL_IN_HISTORY;
}
+/*
+ * Reverse the above encoding
+ */
static void
decode_history(void)
{
@@ -285,9 +270,10 @@ initializeInput(int flags)
}
if (psql_history)
+ {
read_history(psql_history);
-
- decode_history();
+ decode_history();
+ }
}
#endif
@@ -299,11 +285,13 @@ initializeInput(int flags)
}
-/* This function is designed for saving the readline history when user
- * run \s command or when psql finishes.
- * We have an argument named encodeFlag to handle those cases differently
- * In that case of call via \s we don't really need to encode \n as \x01,
- * but when we save history for Readline we must do that conversion
+/*
+ * This function is for saving the readline history when user
+ * runs \s command or when psql finishes.
+ *
+ * We have an argument named encodeFlag to handle the cases differently.
+ * In case of call via \s we don't really need to encode \n as \x01,
+ * but when we save history for Readline we must do that conversion.
*/
bool
saveHistory(char *fname, bool encodeFlag)
@@ -316,7 +304,8 @@ saveHistory(char *fname, bool encodeFlag)
if (write_history(fname) == 0)
return true;
- psql_error("could not save history to file \"%s\": %s\n", fname, strerror(errno));
+ psql_error("could not save history to file \"%s\": %s\n",
+ fname, strerror(errno));
}
#else
psql_error("history is not supported by this installation\n");
diff --git a/src/bin/psql/input.h b/src/bin/psql/input.h
index 4fbc86079b4..0c6ede0cd5a 100644
--- a/src/bin/psql/input.h
+++ b/src/bin/psql/input.h
@@ -3,7 +3,7 @@
*
* Copyright (c) 2000-2006, PostgreSQL Global Development Group
*
- * $PostgreSQL: pgsql/src/bin/psql/input.h,v 1.27 2006/03/21 13:38:12 momjian Exp $
+ * $PostgreSQL: pgsql/src/bin/psql/input.h,v 1.28 2006/06/11 23:06:00 tgl Exp $
*/
#ifndef INPUT_H
#define INPUT_H
@@ -32,6 +32,8 @@
#endif
#endif
+#include "pqexpbuffer.h"
+
char *gets_interactive(const char *prompt);
char *gets_fromFile(FILE *source);
@@ -39,9 +41,7 @@ char *gets_fromFile(FILE *source);
void initializeInput(int flags);
bool saveHistory(char *fname, bool encodeFlag);
-void pg_append_history(char *s, PQExpBuffer history_buf);
-void pg_clear_history(PQExpBuffer history_buf);
-void pg_write_history(char *s);
-
+void pg_append_history(const char *s, PQExpBuffer history_buf);
+void pg_send_history(PQExpBuffer history_buf);
#endif /* INPUT_H */
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index 6c02ba1ccae..af297ae2a91 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -3,7 +3,7 @@
*
* Copyright (c) 2000-2006, PostgreSQL Global Development Group
*
- * $PostgreSQL: pgsql/src/bin/psql/mainloop.c,v 1.78 2006/06/07 13:18:37 momjian Exp $
+ * $PostgreSQL: pgsql/src/bin/psql/mainloop.c,v 1.79 2006/06/11 23:06:00 tgl Exp $
*/
#include "postgres_fe.h"
#include "mainloop.h"
@@ -37,12 +37,12 @@ MainLoop(FILE *source)
PQExpBuffer query_buf; /* buffer for query being accumulated */
PQExpBuffer previous_buf; /* if there isn't anything in the new buffer
* yet, use this one for \e, etc. */
- PQExpBuffer history_buf;
+ PQExpBuffer history_buf; /* earlier lines of a multi-line command,
+ * not yet saved to readline history */
char *line; /* current line of input */
int added_nl_pos;
bool success;
-
- volatile bool line_saved_in_history;
+ bool line_saved_in_history;
volatile int successResult = EXIT_SUCCESS;
volatile backslashResult slashCmdStatus = PSQL_CMD_UNKNOWN;
volatile promptStatus_t prompt_status = PROMPT_READY;
@@ -70,7 +70,6 @@ MainLoop(FILE *source)
query_buf = createPQExpBuffer();
previous_buf = createPQExpBuffer();
history_buf = createPQExpBuffer();
-
if (!query_buf || !previous_buf || !history_buf)
{
psql_error("out of memory\n");
@@ -80,8 +79,6 @@ MainLoop(FILE *source)
/* main loop to get queries and execute them */
while (successResult == EXIT_SUCCESS)
{
- line_saved_in_history = false;
-
/*
* Welcome code for Control-C
*/
@@ -97,7 +94,7 @@ MainLoop(FILE *source)
successResult = EXIT_USER;
break;
}
- pg_clear_history(history_buf);
+
cancel_pressed = false;
}
@@ -113,8 +110,6 @@ MainLoop(FILE *source)
count_eof = 0;
slashCmdStatus = PSQL_CMD_UNKNOWN;
prompt_status = PROMPT_READY;
- if (pset.cur_cmd_interactive)
- pg_write_history(history_buf->data);
if (pset.cur_cmd_interactive)
putc('\n', stdout);
@@ -135,33 +130,10 @@ MainLoop(FILE *source)
fflush(stdout);
- if (slashCmdStatus == PSQL_CMD_NEWEDIT)
- {
- /*
- * just returned from editing the line? then just copy to the
- * input buffer
- */
- line = pg_strdup(query_buf->data);
- /* reset parsing state since we are rescanning whole line */
- resetPQExpBuffer(query_buf);
- psql_scan_reset(scan_state);
- slashCmdStatus = PSQL_CMD_UNKNOWN;
- prompt_status = PROMPT_READY;
-
- if (pset.cur_cmd_interactive)
- {
- /*
- * Pass all the contents of history_buf to readline
- * and free the history buffer.
- */
- pg_write_history(history_buf->data);
- pg_clear_history(history_buf);
- pg_write_history(line);
- line_saved_in_history = true;
- }
- }
- /* otherwise, get another line */
- else if (pset.cur_cmd_interactive)
+ /*
+ * get another line
+ */
+ if (pset.cur_cmd_interactive)
{
/* May need to reset prompt, eg after \r command */
if (query_buf->len == 0)
@@ -230,7 +202,8 @@ MainLoop(FILE *source)
*/
psql_scan_setup(scan_state, line, strlen(line));
success = true;
-
+ line_saved_in_history = false;
+
while (success || !die_on_error)
{
PsqlScanResult scan_result;
@@ -247,6 +220,17 @@ MainLoop(FILE *source)
(scan_result == PSCAN_EOL &&
GetVariableBool(pset.vars, "SINGLELINE")))
{
+ /*
+ * Save query in history. We use history_buf to accumulate
+ * multi-line queries into a single history entry.
+ */
+ if (pset.cur_cmd_interactive && !line_saved_in_history)
+ {
+ pg_append_history(line, history_buf);
+ pg_send_history(history_buf);
+ line_saved_in_history = true;
+ }
+
/* execute query */
success = SendQuery(query_buf->data);
slashCmdStatus = success ? PSQL_CMD_SEND : PSQL_CMD_ERROR;
@@ -265,12 +249,27 @@ MainLoop(FILE *source)
* If we added a newline to query_buf, and nothing else has
* been inserted in query_buf by the lexer, then strip off the
* newline again. This avoids any change to query_buf when a
- * line contains only a backslash command.
+ * line contains only a backslash command. Also, in this
+ * situation we force out any previous lines as a separate
+ * history entry; we don't want SQL and backslash commands
+ * intermixed in history if at all possible.
*/
if (query_buf->len == added_nl_pos)
+ {
query_buf->data[--query_buf->len] = '\0';
+ pg_send_history(history_buf);
+ }
added_nl_pos = -1;
+ /* save backslash command in history */
+ if (pset.cur_cmd_interactive && !line_saved_in_history)
+ {
+ pg_append_history(line, history_buf);
+ pg_send_history(history_buf);
+ line_saved_in_history = true;
+ }
+
+ /* execute backslash command */
slashCmdStatus = HandleSlashCmds(scan_state,
query_buf->len > 0 ?
query_buf : previous_buf);
@@ -295,44 +294,32 @@ MainLoop(FILE *source)
/* flush any paren nesting info after forced send */
psql_scan_reset(scan_state);
}
+ else if (slashCmdStatus == PSQL_CMD_NEWEDIT)
+ {
+ /* rescan query_buf as new input */
+ psql_scan_finish(scan_state);
+ free(line);
+ line = pg_strdup(query_buf->data);
+ resetPQExpBuffer(query_buf);
+ /* reset parsing state since we are rescanning whole line */
+ psql_scan_reset(scan_state);
+ psql_scan_setup(scan_state, line, strlen(line));
+ line_saved_in_history = false;
+ prompt_status = PROMPT_READY;
+ }
+ else if (slashCmdStatus == PSQL_CMD_TERMINATE)
+ break;
}
- /*
- * If we append to history a backslash command that is inside
- * a multi-line query, then when we recall the history, the
- * backslash command will make the query invalid, so we write
- * backslash commands immediately rather than keeping them
- * as part of the current multi-line query. We do the test
- * down here so we can check for \g and other 'execute'
- * backslash commands, which should be appended.
- */
- if (!line_saved_in_history && pset.cur_cmd_interactive)
- {
- /* Sending a command (PSQL_CMD_SEND) zeros the length */
- if (scan_result == PSCAN_BACKSLASH)
- pg_write_history(line);
- else
- pg_append_history(line, history_buf);
- line_saved_in_history = true;
- }
-
- /* fall out of loop on \q or if lexer reached EOL */
- if (slashCmdStatus == PSQL_CMD_TERMINATE ||
- scan_result == PSCAN_INCOMPLETE ||
+ /* fall out of loop if lexer reached EOL */
+ if (scan_result == PSCAN_INCOMPLETE ||
scan_result == PSCAN_EOL)
break;
}
- if ((pset.cur_cmd_interactive && prompt_status == PROMPT_READY) ||
- (GetVariableBool(pset.vars, "SINGLELINE") && prompt_status == PROMPT_CONTINUE))
- {
- /*
- * Pass all the contents of history_buf to readline
- * and free the history buffer.
- */
- pg_write_history(history_buf->data);
- pg_clear_history(history_buf);
- }
+ /* Add line to pending history if we didn't execute anything yet */
+ if (pset.cur_cmd_interactive && !line_saved_in_history)
+ pg_append_history(line, history_buf);
psql_scan_finish(scan_state);
free(line);
@@ -359,6 +346,11 @@ MainLoop(FILE *source)
if (query_buf->len > 0 && !pset.cur_cmd_interactive &&
successResult == EXIT_SUCCESS)
{
+ /* save query in history */
+ if (pset.cur_cmd_interactive)
+ pg_send_history(history_buf);
+
+ /* execute query */
success = SendQuery(query_buf->data);
if (!success && die_on_error)
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index f7d591b0545..07917c53450 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -3,7 +3,7 @@
*
* Copyright (c) 2000-2006, PostgreSQL Global Development Group
*
- * $PostgreSQL: pgsql/src/bin/psql/prompt.c,v 1.44 2006/04/19 16:02:17 tgl Exp $
+ * $PostgreSQL: pgsql/src/bin/psql/prompt.c,v 1.45 2006/06/11 23:06:00 tgl Exp $
*/
#include "postgres_fe.h"
#include "prompt.h"
@@ -12,7 +12,6 @@
#include "settings.h"
#include "common.h"
-#include "pqexpbuffer.h"
#include "input.h"
#include "variables.h"
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 653e57023d8..36d2ff22020 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3,7 +3,7 @@
*
* Copyright (c) 2000-2006, PostgreSQL Global Development Group
*
- * $PostgreSQL: pgsql/src/bin/psql/tab-complete.c,v 1.152 2006/05/28 02:27:08 alvherre Exp $
+ * $PostgreSQL: pgsql/src/bin/psql/tab-complete.c,v 1.153 2006/06/11 23:06:00 tgl Exp $
*/
/*----------------------------------------------------------------------
@@ -43,7 +43,6 @@
#include "postgres_fe.h"
#include "tab-complete.h"
-#include "pqexpbuffer.h"
#include "input.h"
/* If we don't have this, we might as well forget about the whole thing: */
@@ -51,6 +50,7 @@
#include <ctype.h>
#include "libpq-fe.h"
+#include "pqexpbuffer.h"
#include "common.h"
#include "settings.h"
--
GitLab