OSDN Git Service

Code review for psql multiline history patch(es). Fix memory leak,
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 11 Jun 2006 23:06:00 +0000 (23:06 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 11 Jun 2006 23:06:00 +0000 (23:06 +0000)
failure to enter commands in history if canceled by control-C, other
infelicities.

src/bin/psql/command.c
src/bin/psql/help.c
src/bin/psql/input.c
src/bin/psql/input.h
src/bin/psql/mainloop.c
src/bin/psql/prompt.c
src/bin/psql/tab-complete.c

index 3c5bd67..a9d8a07 100644 (file)
@@ -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);
                }
 
index dd3bdef..6bc0b73 100644 (file)
@@ -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"
index 1dc36d9..b970175 100644 (file)
@@ -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");
index 4fbc860..0c6ede0 100644 (file)
@@ -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 */
index 6c02ba1..af297ae 100644 (file)
@@ -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)
index f7d591b..07917c5 100644 (file)
@@ -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"
 
index 653e570..36d2ff2 100644 (file)
@@ -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"