Commit 7a5f8b5c authored by Tom Lane's avatar Tom Lane

Improve coding of column-name parsing in psql's new crosstabview.c.

Coverity complained about this code, not without reason because it was
rather messy.  Adjust it to not scribble on the passed string; that adds
one malloc/free cycle per column name, which is going to be insignificant
in context.  We can actually const-ify both the string argument and the
PGresult.

Daniel Verité, with some further cleanup by me
parent 2201d801
...@@ -82,7 +82,8 @@ static bool printCrosstab(const PGresult *results, ...@@ -82,7 +82,8 @@ static bool printCrosstab(const PGresult *results,
int num_columns, pivot_field *piv_columns, int field_for_columns, int num_columns, pivot_field *piv_columns, int field_for_columns,
int num_rows, pivot_field *piv_rows, int field_for_rows, int num_rows, pivot_field *piv_rows, int field_for_rows,
int field_for_data); int field_for_data);
static int parseColumnRefs(char *arg, PGresult *res, int **col_numbers, static int parseColumnRefs(const char *arg, const PGresult *res,
int **col_numbers,
int max_columns, char separator); int max_columns, char separator);
static void avlInit(avl_tree *tree); static void avlInit(avl_tree *tree);
static void avlMergeValue(avl_tree *tree, char *name, char *sort_value); static void avlMergeValue(avl_tree *tree, char *name, char *sort_value);
...@@ -90,7 +91,7 @@ static int avlCollectFields(avl_tree *tree, avl_node *node, ...@@ -90,7 +91,7 @@ static int avlCollectFields(avl_tree *tree, avl_node *node,
pivot_field *fields, int idx); pivot_field *fields, int idx);
static void avlFree(avl_tree *tree, avl_node *node); static void avlFree(avl_tree *tree, avl_node *node);
static void rankSort(int num_columns, pivot_field *piv_columns); static void rankSort(int num_columns, pivot_field *piv_columns);
static int indexOfColumn(const char *arg, PGresult *res); static int indexOfColumn(const char *arg, const PGresult *res);
static int pivotFieldCompare(const void *a, const void *b); static int pivotFieldCompare(const void *a, const void *b);
static int rankCompare(const void *a, const void *b); static int rankCompare(const void *a, const void *b);
...@@ -103,7 +104,7 @@ static int rankCompare(const void *a, const void *b); ...@@ -103,7 +104,7 @@ static int rankCompare(const void *a, const void *b);
* then call printCrosstab() for the actual output. * then call printCrosstab() for the actual output.
*/ */
bool bool
PrintResultsInCrosstab(PGresult *res) PrintResultsInCrosstab(const PGresult *res)
{ {
char *opt_field_for_rows = pset.ctv_col_V; char *opt_field_for_rows = pset.ctv_col_V;
char *opt_field_for_columns = pset.ctv_col_H; char *opt_field_for_columns = pset.ctv_col_H;
...@@ -475,33 +476,37 @@ error: ...@@ -475,33 +476,37 @@ error:
} }
/* /*
* Parse col1[<sep>col2][<sep>col3]... * Parse "arg", which is a string of column IDs separated by "separator".
* where colN can be: *
* Each column ID can be:
* - a number from 1 to PQnfields(res) * - a number from 1 to PQnfields(res)
* - an unquoted column name matching (case insensitively) one of PQfname(res,...) * - an unquoted column name matching (case insensitively) one of PQfname(res,...)
* - a quoted column name matching (case sensitively) one of PQfname(res,...) * - a quoted column name matching (case sensitively) one of PQfname(res,...)
* max_columns: 0 if no maximum *
* If max_columns > 0, it is the max number of column IDs allowed.
*
* On success, return number of column IDs found (possibly 0), and return a
* malloc'd array of the matching column numbers of "res" into *col_numbers.
*
* On failure, return -1 and set *col_numbers to NULL.
*/ */
static int static int
parseColumnRefs(char *arg, parseColumnRefs(const char *arg,
PGresult *res, const PGresult *res,
int **col_numbers, int **col_numbers,
int max_columns, int max_columns,
char separator) char separator)
{ {
char *p = arg; const char *p = arg;
char c; char c;
int col_num = -1; int num_cols = 0;
int nb_cols = 0;
char *field_start = NULL;
*col_numbers = NULL; *col_numbers = NULL;
while ((c = *p) != '\0') while ((c = *p) != '\0')
{ {
const char *field_start = p;
bool quoted_field = false; bool quoted_field = false;
field_start = p;
/* first char */ /* first char */
if (c == '"') if (c == '"')
{ {
...@@ -533,20 +538,27 @@ parseColumnRefs(char *arg, ...@@ -533,20 +538,27 @@ parseColumnRefs(char *arg,
if (p != field_start) if (p != field_start)
{ {
/* look up the column and add its index into *col_numbers */ char *col_name;
if (max_columns != 0 && nb_cols == max_columns) int col_num;
/* enforce max_columns limit */
if (max_columns > 0 && num_cols == max_columns)
{ {
psql_error(_("No more than %d column references expected\n"), max_columns); psql_error(_("No more than %d column references expected\n"),
max_columns);
goto errfail; goto errfail;
} }
c = *p; /* look up the column and add its index into *col_numbers */
*p = '\0'; col_name = pg_malloc(p - field_start + 1);
col_num = indexOfColumn(field_start, res); memcpy(col_name, field_start, p - field_start);
col_name[p - field_start] = '\0';
col_num = indexOfColumn(col_name, res);
pg_free(col_name);
if (col_num < 0) if (col_num < 0)
goto errfail; goto errfail;
*p = c; *col_numbers = (int *) pg_realloc(*col_numbers,
*col_numbers = (int *) pg_realloc(*col_numbers, (1 + nb_cols) * sizeof(int)); (num_cols + 1) * sizeof(int));
(*col_numbers)[nb_cols++] = col_num; (*col_numbers)[num_cols++] = col_num;
} }
else else
{ {
...@@ -557,7 +569,7 @@ parseColumnRefs(char *arg, ...@@ -557,7 +569,7 @@ parseColumnRefs(char *arg,
if (*p) if (*p)
p += PQmblen(p, pset.encoding); p += PQmblen(p, pset.encoding);
} }
return nb_cols; return num_cols;
errfail: errfail:
pg_free(*col_numbers); pg_free(*col_numbers);
...@@ -776,7 +788,7 @@ fieldNameEquals(const char *arg, const char *fieldname) ...@@ -776,7 +788,7 @@ fieldNameEquals(const char *arg, const char *fieldname)
char c; char c;
if (*p++ != '"') if (*p++ != '"')
return !pg_strcasecmp(arg, fieldname); return (pg_strcasecmp(arg, fieldname) == 0);
while ((c = *p++)) while ((c = *p++))
{ {
...@@ -805,7 +817,7 @@ fieldNameEquals(const char *arg, const char *fieldname) ...@@ -805,7 +817,7 @@ fieldNameEquals(const char *arg, const char *fieldname)
* or if it's ambiguous (arg corresponding to several columns) * or if it's ambiguous (arg corresponding to several columns)
*/ */
static int static int
indexOfColumn(const char *arg, PGresult *res) indexOfColumn(const char *arg, const PGresult *res)
{ {
int idx; int idx;
......
...@@ -22,5 +22,6 @@ ...@@ -22,5 +22,6 @@
#define CROSSTABVIEW_MAX_COLUMNS 1600 #define CROSSTABVIEW_MAX_COLUMNS 1600
/* prototypes */ /* prototypes */
extern bool PrintResultsInCrosstab(PGresult *res); extern bool PrintResultsInCrosstab(const PGresult *res);
#endif /* CROSSTABVIEW_H */ #endif /* CROSSTABVIEW_H */
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