Commit fcbbf82d authored by Tom Lane's avatar Tom Lane

Code and docs review for multiple -c and -f options in psql.

Commit d5563d7d 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.
parent a91bdf67
This diff is collapsed.
...@@ -58,8 +58,8 @@ enum _actions ...@@ -58,8 +58,8 @@ enum _actions
typedef struct SimpleActionListCell typedef struct SimpleActionListCell
{ {
struct SimpleActionListCell *next; struct SimpleActionListCell *next;
int action; enum _actions action;
char *val; char *val;
} SimpleActionListCell; } SimpleActionListCell;
typedef struct SimpleActionList typedef struct SimpleActionList
...@@ -84,11 +84,11 @@ struct adhoc_opts ...@@ -84,11 +84,11 @@ struct adhoc_opts
static void parse_psql_options(int argc, char *argv[], static void parse_psql_options(int argc, char *argv[],
struct adhoc_opts * options); 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(char *argv0);
static void process_psqlrc_file(char *filename); static void process_psqlrc_file(char *filename);
static void showVersion(void); static void showVersion(void);
static void simple_action_list_append(SimpleActionList *list,
int action, const char *val);
static void EstablishVariableSpace(void); static void EstablishVariableSpace(void);
#define NOPAGER 0 #define NOPAGER 0
...@@ -172,9 +172,6 @@ main(int argc, char *argv[]) ...@@ -172,9 +172,6 @@ main(int argc, char *argv[])
SetVariable(pset.vars, "PROMPT2", DEFAULT_PROMPT2); SetVariable(pset.vars, "PROMPT2", DEFAULT_PROMPT2);
SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3); SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3);
options.actions.head = NULL;
options.actions.tail = NULL;
parse_psql_options(argc, argv, &options); parse_psql_options(argc, argv, &options);
/* /*
...@@ -298,13 +295,13 @@ main(int argc, char *argv[]) ...@@ -298,13 +295,13 @@ main(int argc, char *argv[])
process_psqlrc(argv[0]); process_psqlrc(argv[0]);
/* /*
* If any actions were given by caller, process them in the order in * If any actions were given by user, process them in the order in which
* which they were specified. * they were specified. Note single_txn is only effective in this mode.
*/ */
if (options.actions.head != NULL) if (options.actions.head != NULL)
{ {
PGresult *res; PGresult *res;
SimpleActionListCell *cell; SimpleActionListCell *cell;
successResult = EXIT_SUCCESS; /* silence compiler */ successResult = EXIT_SUCCESS; /* silence compiler */
...@@ -341,8 +338,8 @@ main(int argc, char *argv[]) ...@@ -341,8 +338,8 @@ main(int argc, char *argv[])
scan_state = psql_scan_create(); scan_state = psql_scan_create();
psql_scan_setup(scan_state, psql_scan_setup(scan_state,
cell->val, cell->val,
strlen(cell->val)); strlen(cell->val));
successResult = HandleSlashCmds(scan_state, NULL) != PSQL_CMD_ERROR successResult = HandleSlashCmds(scan_state, NULL) != PSQL_CMD_ERROR
? EXIT_SUCCESS : EXIT_FAILURE; ? EXIT_SUCCESS : EXIT_FAILURE;
...@@ -356,7 +353,7 @@ main(int argc, char *argv[]) ...@@ -356,7 +353,7 @@ main(int argc, char *argv[])
else else
{ {
/* should never come here */ /* should never come here */
Assert(0); Assert(false);
} }
if (successResult != EXIT_SUCCESS && pset.on_error_stop) if (successResult != EXIT_SUCCESS && pset.on_error_stop)
...@@ -473,11 +470,11 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options) ...@@ -473,11 +470,11 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options)
if (optarg[0] == '\\') if (optarg[0] == '\\')
simple_action_list_append(&options->actions, simple_action_list_append(&options->actions,
ACT_SINGLE_SLASH, ACT_SINGLE_SLASH,
pstrdup(optarg + 1)); optarg + 1);
else else
simple_action_list_append(&options->actions, simple_action_list_append(&options->actions,
ACT_SINGLE_QUERY, ACT_SINGLE_QUERY,
pstrdup(optarg)); optarg);
break; break;
case 'd': case 'd':
options->dbname = pg_strdup(optarg); options->dbname = pg_strdup(optarg);
...@@ -490,8 +487,8 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options) ...@@ -490,8 +487,8 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options)
break; break;
case 'f': case 'f':
simple_action_list_append(&options->actions, simple_action_list_append(&options->actions,
ACT_FILE, ACT_FILE,
pg_strdup(optarg)); optarg);
break; break;
case 'F': case 'F':
pset.popt.topt.fieldSep.separator = pg_strdup(optarg); pset.popt.topt.fieldSep.separator = pg_strdup(optarg);
...@@ -672,6 +669,33 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options) ...@@ -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. * Load .psqlrc file, if found.
*/ */
...@@ -945,39 +969,6 @@ show_context_hook(const char *newval) ...@@ -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 static void
EstablishVariableSpace(void) EstablishVariableSpace(void)
{ {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment