From fcbbf82d2b6caf7b156f2ec35b322e23caf1e99e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 13 Dec 2015 14:52:07 -0500 Subject: [PATCH] Code and docs review for multiple -c and -f options in psql. 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 | 140 +++++++++++++++++++-------------- src/bin/psql/startup.c | 93 ++++++++++------------ 2 files changed, 121 insertions(+), 112 deletions(-) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 47e9da2c8a..a416c1de9f 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -39,9 +39,9 @@ PostgreSQL documentation queries interactively, issue them to PostgreSQL, 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, psql provides a + number of meta-commands and various shell-like features to + facilitate writing scripts and automating a wide variety of tasks. @@ -90,42 +90,50 @@ PostgreSQL documentation - Specifies that psql is to execute the given - command string, command. - This option can be repeated and combined in any order with - the option. + Specifies that psql is to execute the given + command string, command. + This option can be repeated and combined in any order with + the option. When either + or is specified, psql + does not read commands from standard input; instead it terminates + after processing all the and + options in sequence. - command must be either - a command string that is completely parsable by the server (i.e., - it contains no psql-specific features), - or a single backslash command. Thus you cannot mix - SQL and psql - meta-commands with this option. To achieve that, you could - use repeated options or pipe the string - into psql, for example: - psql -c '\x' -c 'SELECT * FROM foo;' or - echo '\x \\ SELECT * FROM foo;' | psql. - (\\ is the separator meta-command.) + command must be either + a command string that is completely parsable by the server (i.e., + it contains no psql-specific features), + or a single backslash command. Thus you cannot mix + SQL and psql + meta-commands within a option. To achieve that, + you could use repeated options or pipe the string + into psql, for example: + +psql -c '\x' -c 'SELECT * FROM foo;' + + or + +echo '\x \\ SELECT * FROM foo;' | psql + + (\\ is the separator meta-command.) - Each command string passed to 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 SQL commands, unless there are - explicit BEGIN/COMMIT commands included in the - string to divide it into multiple transactions. Also, the server only - returns the result of the last SQL command to the - client. This is different from the behavior when the same string with - multiple SQL commands is fed - to psql's standard input because - then psql sends each SQL - command separately. + Each SQL command string passed + to 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 SQL commands, + unless there are explicit BEGIN/COMMIT + commands included in the string to divide it into multiple + transactions. Also, psql only prints the + result of the last SQL command in the string. + This is different from the behavior when the same string is read from + a file or fed to psql's standard input, + because then psql sends + each SQL command separately. - Putting more than one command in the 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 string often has unexpected results. It's better to use repeated commands or feed multiple commands to psql's standard input, either using echo as illustrated above, or @@ -192,18 +200,26 @@ EOF - Use the file filename - as the source of commands instead of reading commands interactively. - This option can be repeated and combined in any order with - the option. After the commands in - every command string and file - are processed, psql terminates. This option - is in many ways equivalent to the meta-command \i. + Read commands from the + file filename, + rather than standard input. + This option can be repeated and combined in any order with + the option. When either + or is specified, psql + does not read commands from standard input; instead it terminates + after processing all the and + options in sequence. + Except for that, this option is largely equivalent to the + meta-command \i. If filename is - - (hyphen), then standard input is read. + (hyphen), then standard input is read until an EOF indication + or \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 had been + specified). @@ -550,12 +566,13 @@ EOF - When psql executes commands from a script - and/or a option, adding this option - wraps BEGIN/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 + and/or options. It causes + psql to issue a BEGIN command + before the first such option and a 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. @@ -3799,16 +3816,6 @@ PSQL_EDITOR_LINENUMBER_ARG='--line ' Notes - - - In an earlier life psql allowed the - first argument of a single-letter backslash command to start - directly after the command, without intervening whitespace. - As of PostgreSQL 8.4 this is no - longer allowed. - - - psql 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 psql to connect to several servers of different major versions, it is recommended that you use the newest version of psql. Alternatively, you - can keep a copy of psql from each major - version around and be sure to use the version that matches the + can keep around a copy of psql 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. @@ -3833,8 +3840,19 @@ PSQL_EDITOR_LINENUMBER_ARG='--line ' - Before PostgreSQL 9.6, - implied ; this is no longer the case. + Before PostgreSQL 9.6, + the option implied + ( + + + + + Before PostgreSQL 8.4, + psql allowed the + first argument of a single-letter backslash command to start + directly after the command, without intervening whitespace. + Now, some whitespace is required. diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 17fb943dfa..0047478109 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -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) {