Code and docs review for multiple -c and -f options in psql.
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 13 Dec 2015 19:52:07 +0000 (14:52 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 13 Dec 2015 19:52:07 +0000 (14:52 -0500)
Commit d5563d7df94488bf drew complaints from Coverity, which quite
correctly complained that one copy of each -c or -f string was being
leaked.  What's more, simple_action_list_append was allocating enough space
for still a third copy of each string as part of the SimpleActionListCell,
even though that coding method had been superseded by a separate strdup
operation.  There were some other minor coding infelicities too.  The
documentation needed more work as well, eg it forgot to explain that -c
causes psql not to accept any interactive input.

doc/src/sgml/ref/psql-ref.sgml
src/bin/psql/startup.c

index 47e9da2c8a5778a9af752a83a7f3fd8692396808..a416c1de9f9c64a1d6cc63cc1d40dbda6283c59f 100644 (file)
@@ -39,9 +39,9 @@ PostgreSQL documentation
      queries interactively, issue them to
      <productname>PostgreSQL</productname>, and see the query results.
      Alternatively, input can be from a file or from command line
-     arguments. In addition, it provides a number of meta-commands and various
-     shell-like features to facilitate writing scripts and automating a wide
-     variety of tasks.
+     arguments. In addition, <application>psql</application> provides a
+     number of meta-commands and various shell-like features to
+     facilitate writing scripts and automating a wide variety of tasks.
     </para>
  </refsect1>
 
@@ -90,42 +90,50 @@ PostgreSQL documentation
       <term><option>--command=<replaceable class="parameter">command</replaceable></></term>
       <listitem>
       <para>
-      Specifies that <application>psql</application> is to execute the given
-      command string, <replaceable class="parameter">command</replaceable>.
-      This option can be repeated and combined in any order with
-      the <option>-f</option> option.
+       Specifies that <application>psql</application> is to execute the given
+       command string, <replaceable class="parameter">command</replaceable>.
+       This option can be repeated and combined in any order with
+       the <option>-f</option> option.  When either <option>-c</option>
+       or <option>-f</option> is specified, <application>psql</application>
+       does not read commands from standard input; instead it terminates
+       after processing all the <option>-c</option> and <option>-f</option>
+       options in sequence.
       </para>
       <para>
-      <replaceable class="parameter">command</replaceable> must be either
-      a command string that is completely parsable by the server (i.e.,
-      it contains no <application>psql</application>-specific features),
-      or a single backslash command. Thus you cannot mix
-      <acronym>SQL</acronym> and <application>psql</application>
-      meta-commands with this option. To achieve that, you could
-      use repeated <option>-c</option> options or pipe the string
-      into <application>psql</application>, for example:
-      <literal>psql -c '\x' -c 'SELECT * FROM foo;'</literal> or
-      <literal>echo '\x \\ SELECT * FROM foo;' | psql</literal>.
-      (<literal>\\</> is the separator meta-command.)
+       <replaceable class="parameter">command</replaceable> must be either
+       a command string that is completely parsable by the server (i.e.,
+       it contains no <application>psql</application>-specific features),
+       or a single backslash command. Thus you cannot mix
+       <acronym>SQL</acronym> and <application>psql</application>
+       meta-commands within a <option>-c</option> option. To achieve that,
+       you could use repeated <option>-c</option> options or pipe the string
+       into <application>psql</application>, for example:
+<programlisting>
+psql -c '\x' -c 'SELECT * FROM foo;'
+</programlisting>
+       or
+<programlisting>
+echo '\x \\ SELECT * FROM foo;' | psql
+</programlisting>
+       (<literal>\\</> is the separator meta-command.)
       </para>
       <para>
-       Each command string passed to <option>-c</option> is sent to the server
-       as a single query. Because of this, the server executes it as a single
-       transaction, even if a command string contains
-       multiple <acronym>SQL</acronym> commands, unless there are
-       explicit <command>BEGIN</>/<command>COMMIT</> commands included in the
-       string to divide it into multiple transactions.  Also, the server only
-       returns the result of the last <acronym>SQL</acronym> command to the
-       client.  This is different from the behavior when the same string with
-       multiple <acronym>SQL</acronym> commands is fed
-       to <application>psql</application>'s standard input because
-       then <application>psql</application> sends each <acronym>SQL</acronym>
-       command separately.
+       Each <acronym>SQL</acronym> command string passed
+       to <option>-c</option> is sent to the server as a single query.
+       Because of this, the server executes it as a single transaction even
+       if the string contains multiple <acronym>SQL</acronym> commands,
+       unless there are explicit <command>BEGIN</>/<command>COMMIT</>
+       commands included in the string to divide it into multiple
+       transactions.  Also, <application>psql</application> only prints the
+       result of the last <acronym>SQL</acronym> command in the string.
+       This is different from the behavior when the same string is read from
+       a file or fed to <application>psql</application>'s standard input,
+       because then <application>psql</application> sends
+       each <acronym>SQL</acronym> command separately.
       </para>
       <para>
-       Putting more than one command in the <option>-c</option> string often
-       has unexpected results.  This is a result of the fact that the whole
-       string is sent to the server as a single query.
+       Because of this behavior, putting more than one command in a
+       single <option>-c</option> string often has unexpected results.
        It's better to use repeated <option>-c</option> commands or feed
        multiple commands to <application>psql</application>'s standard input,
        either using <application>echo</application> as illustrated above, or
@@ -192,18 +200,26 @@ EOF
       <term><option>--file=<replaceable class="parameter">filename</replaceable></></term>
       <listitem>
       <para>
-      Use the file <replaceable class="parameter">filename</replaceable>
-      as the source of commands instead of reading commands interactively.
-      This option can be repeated and combined in any order with
-      the <option>-c</option> option.  After the commands in
-      every <option>-c</option> command string and <option>-f</option> file
-      are processed, <application>psql</application> terminates.  This option
-      is in many ways equivalent to the meta-command <command>\i</command>.
+       Read commands from the
+       file <replaceable class="parameter">filename</replaceable>,
+       rather than standard input.
+       This option can be repeated and combined in any order with
+       the <option>-c</option> option.  When either <option>-c</option>
+       or <option>-f</option> is specified, <application>psql</application>
+       does not read commands from standard input; instead it terminates
+       after processing all the <option>-c</option> and <option>-f</option>
+       options in sequence.
+       Except for that, this option is largely equivalent to the
+       meta-command <command>\i</command>.
       </para>
 
       <para>
        If <replaceable>filename</replaceable> is <literal>-</literal>
-       (hyphen), then standard input is read.
+       (hyphen), then standard input is read until an EOF indication
+       or <command>\q</> meta-command.  This can be used to intersperse
+       interactive input with input from files.  Note however that Readline
+       is not used in this case (much as if <option>-n</option> had been
+       specified).
       </para>
 
       <para>
@@ -550,12 +566,13 @@ EOF
       <term><option>--single-transaction</option></term>
       <listitem>
        <para>
-        When <application>psql</application> executes commands from a script
-        and/or a <option>-c</option> option, adding this option
-        wraps <command>BEGIN</>/<command>COMMIT</> around all of those
-        commands as a whole to execute them as a single transaction.  This
-        ensures that either all the commands complete successfully, or no
-        changes are applied.
+        This option can only be used in combination with one or more
+        <option>-c</option> and/or <option>-f</option> options.  It causes
+        <application>psql</application> to issue a <command>BEGIN</> command
+        before the first such option and a <command>COMMIT</> command after
+        the last one, thereby wrapping all the commands into a single
+        transaction.  This ensures that either all the commands complete
+        successfully, or no changes are applied.
        </para>
 
        <para>
@@ -3799,16 +3816,6 @@ PSQL_EDITOR_LINENUMBER_ARG='--line '
   <title>Notes</title>
 
     <itemizedlist>
-      <listitem>
-      <para>
-       In an earlier life <application>psql</application> allowed the
-       first argument of a single-letter backslash command to start
-       directly after the command, without intervening whitespace.
-       As of <productname>PostgreSQL</productname> 8.4 this is no
-       longer allowed.
-      </para>
-      </listitem>
-
       <listitem>
       <para><application>psql</application> works best with servers of the same
        or an older major version.  Backslash commands are particularly likely
@@ -3824,8 +3831,8 @@ PSQL_EDITOR_LINENUMBER_ARG='--line '
        If you want to use <application>psql</application> to connect to several
        servers of different major versions, it is recommended that you use the
        newest version of <application>psql</application>.  Alternatively, you
-       can keep a copy of <application>psql</application> from each major
-       version around and be sure to use the version that matches the
+       can keep around a copy of <application>psql</application> from each
+       major version and be sure to use the version that matches the
        respective server.  But in practice, this additional complication should
        not be necessary.
       </para>
@@ -3833,8 +3840,19 @@ PSQL_EDITOR_LINENUMBER_ARG='--line '
 
       <listitem>
       <para>
-       Before <productname>PostgreSQL</productname> 9.6, <option>-c</option>
-       implied <option>-X</option>; this is no longer the case.
+       Before <productname>PostgreSQL</productname> 9.6,
+       the <option>-c</option> option implied <option>-X</option>
+       (<option>--no-psqlrc</>); this is no longer the case.
+      </para>
+      </listitem>
+
+      <listitem>
+      <para>
+       Before <productname>PostgreSQL</productname> 8.4,
+       <application>psql</application> allowed the
+       first argument of a single-letter backslash command to start
+       directly after the command, without intervening whitespace.
+       Now, some whitespace is required.
       </para>
       </listitem>
     </itemizedlist>
index 17fb943dfab7bd9133b5f28fd7ec9cfc11fb4d6b..004747810966ef163a00cbd0d2549050315fef4c 100644 (file)
@@ -58,8 +58,8 @@ enum _actions
 typedef struct SimpleActionListCell
 {
    struct SimpleActionListCell *next;
-   int         action;
-   char       *val;
+   enum _actions action;
+   char       *val;
 } SimpleActionListCell;
 
 typedef struct SimpleActionList
@@ -84,11 +84,11 @@ struct adhoc_opts
 
 static void parse_psql_options(int argc, char *argv[],
                   struct adhoc_opts * options);
+static void simple_action_list_append(SimpleActionList *list,
+                         enum _actions action, const char *val);
 static void process_psqlrc(char *argv0);
 static void process_psqlrc_file(char *filename);
 static void showVersion(void);
-static void simple_action_list_append(SimpleActionList *list,
-                                     int action, const char *val);
 static void EstablishVariableSpace(void);
 
 #define NOPAGER        0
@@ -172,9 +172,6 @@ main(int argc, char *argv[])
    SetVariable(pset.vars, "PROMPT2", DEFAULT_PROMPT2);
    SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3);
 
-   options.actions.head = NULL;
-   options.actions.tail = NULL;
-
    parse_psql_options(argc, argv, &options);
 
    /*
@@ -298,13 +295,13 @@ main(int argc, char *argv[])
        process_psqlrc(argv[0]);
 
    /*
-    * If any actions were given by caller, process them in the order in
-    * which they were specified.
+    * If any actions were given by user, process them in the order in which
+    * they were specified.  Note single_txn is only effective in this mode.
     */
    if (options.actions.head != NULL)
    {
-       PGresult        *res;
-       SimpleActionListCell    *cell;
+       PGresult   *res;
+       SimpleActionListCell *cell;
 
        successResult = EXIT_SUCCESS;   /* silence compiler */
 
@@ -341,8 +338,8 @@ main(int argc, char *argv[])
 
                scan_state = psql_scan_create();
                psql_scan_setup(scan_state,
-                           cell->val,
-                           strlen(cell->val));
+                               cell->val,
+                               strlen(cell->val));
 
                successResult = HandleSlashCmds(scan_state, NULL) != PSQL_CMD_ERROR
                    ? EXIT_SUCCESS : EXIT_FAILURE;
@@ -356,7 +353,7 @@ main(int argc, char *argv[])
            else
            {
                /* should never come here */
-               Assert(0);
+               Assert(false);
            }
 
            if (successResult != EXIT_SUCCESS && pset.on_error_stop)
@@ -473,11 +470,11 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options)
                if (optarg[0] == '\\')
                    simple_action_list_append(&options->actions,
                                              ACT_SINGLE_SLASH,
-                                             pstrdup(optarg + 1));
+                                             optarg + 1);
                else
                    simple_action_list_append(&options->actions,
                                              ACT_SINGLE_QUERY,
-                                             pstrdup(optarg));
+                                             optarg);
                break;
            case 'd':
                options->dbname = pg_strdup(optarg);
@@ -490,8 +487,8 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options)
                break;
            case 'f':
                simple_action_list_append(&options->actions,
-                                   ACT_FILE,
-                                   pg_strdup(optarg));
+                                         ACT_FILE,
+                                         optarg);
                break;
            case 'F':
                pset.popt.topt.fieldSep.separator = pg_strdup(optarg);
@@ -672,6 +669,33 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options)
 }
 
 
+/*
+ * Append a new item to the end of the SimpleActionList.
+ * Note that "val" is copied if it's not NULL.
+ */
+static void
+simple_action_list_append(SimpleActionList *list,
+                         enum _actions action, const char *val)
+{
+   SimpleActionListCell *cell;
+
+   cell = (SimpleActionListCell *) pg_malloc(sizeof(SimpleActionListCell));
+
+   cell->next = NULL;
+   cell->action = action;
+   if (val)
+       cell->val = pg_strdup(val);
+   else
+       cell->val = NULL;
+
+   if (list->tail)
+       list->tail->next = cell;
+   else
+       list->head = cell;
+   list->tail = cell;
+}
+
+
 /*
  * Load .psqlrc file, if found.
  */
@@ -945,39 +969,6 @@ show_context_hook(const char *newval)
 }
 
 
-/*
- * Support for list of actions. SimpleStringList cannot be used due possible
- * combination different actions with the requirement to save the order.
- */
-static void
-simple_action_list_append(SimpleActionList *list, int action, const char *val)
-{
-   SimpleActionListCell   *cell;
-   size_t                  vallen = 0;
-
-   if (val)
-       vallen = strlen(val);
-
-   cell = (SimpleActionListCell *)
-       pg_malloc(offsetof(SimpleActionListCell, val) + vallen + 1);
-
-   cell->next = NULL;
-   cell->action = action;
-   if (val)
-   {
-       cell->val = pg_malloc(vallen + 1);
-       strcpy(cell->val, val);
-   }
-   else
-       cell->val = NULL;
-
-   if (list->tail)
-       list->tail->next = cell;
-   else
-       list->head = cell;
-   list->tail = cell;
-}
-
 static void
 EstablishVariableSpace(void)
 {