On 02/04/2010 08:31 AM, Joe Conway wrote: > On 02/04/2010 01:23 AM, Fujii Masao wrote: >> On Thu, Feb 4, 2010 at 1:26 PM, Joe Conway <m...@joeconway.com> wrote: >>> OK, this one includes pg_dump(all)/pg_restore and common.c from >>> bin/scripts (createdb, vacuumdb, etc). I still need to adjust the docs, >>> but other than that any remaining complaints? > >> * expand_dbname is defined as a "bool" value in PQconnectdbParams() >> and PQconnectStartParams(). But we should hide such a "bool" from >> an user-visible API, and use an "int" instead? > > Yes, I suppose there is precedence for that. > >> * conninfo_array_parse() calls PQconninfoFree(str_options) as soon >> as one "dbname" keyword is found. So if more than one "dbname" >> keywords are unexpectedly specified in PQconnectdbParams(), the >> str_options would be free()-ed doubly. > > Great catch -- thank you! > > Thanks for the review. I'll do a documentation update, make these > changes, and commit later today if I don't hear any other objections.
Attached has both items fixed and documentation changes. Joe
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /opt/src/cvs/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.296
diff -c -r1.296 libpq.sgml
*** doc/src/sgml/libpq.sgml 28 Jan 2010 06:28:26 -0000 1.296
--- doc/src/sgml/libpq.sgml 4 Feb 2010 17:28:55 -0000
***************
*** 98,104 ****
Makes a new connection to the database server.
<synopsis>
! PGconn *PQconnectdbParams(const char **keywords, const char **values);
</synopsis>
</para>
--- 98,104 ----
Makes a new connection to the database server.
<synopsis>
! PGconn *PQconnectdbParams(const char **keywords, const char **values, int expand_dbname);
</synopsis>
</para>
***************
*** 115,120 ****
--- 115,126 ----
</para>
<para>
+ When <literal>expand_dbname</literal> is non-zero, the
+ <parameter>dbname</parameter> key word value is allowed to be recognized
+ as a <parameter>conninfo</parameter> string. See below for details.
+ </para>
+
+ <para>
The passed arrays can be empty to use all default parameters, or can
contain one or more parameter settings. They should be matched in length.
Processing will stop with the last non-<symbol>NULL</symbol> element
***************
*** 473,478 ****
--- 479,502 ----
is checked. If the environment variable is not set either,
then the indicated built-in defaults are used.
</para>
+
+ <para>
+ If <literal>expand_dbname</literal> is non-zero and
+ <parameter>dbname</parameter> contains an <symbol>=</symbol> sign, it
+ is taken as a <parameter>conninfo</parameter> string in exactly the same way as
+ if it had been passed to <function>PQconnectdb</function>(see below). Previously
+ processed key words will be overridden by key words in the
+ <parameter>conninfo</parameter> string.
+ </para>
+
+ <para>
+ In general key words are processed from the beginning of these arrays in index
+ order. The effect of this is that when key words are repeated, the last processed
+ value is retained. Therefore, through careful placement of the
+ <parameter>dbname</parameter> key word, it is possible to determine what may
+ be overridden by a <parameter>conninfo</parameter> string, and what may not.
+ </para>
+
</listitem>
</varlistentry>
***************
*** 573,579 ****
Make a connection to the database server in a nonblocking manner.
<synopsis>
! PGconn *PQconnectStartParams(const char **keywords, const char **values);
</synopsis>
<synopsis>
--- 597,603 ----
Make a connection to the database server in a nonblocking manner.
<synopsis>
! PGconn *PQconnectStartParams(const char **keywords, const char **values, int expand_dbname);
</synopsis>
<synopsis>
***************
*** 597,604 ****
<para>
With <function>PQconnectStartParams</function>, the database connection is made
using the parameters taken from the <literal>keywords</literal> and
! <literal>values</literal> arrays, as described above for
! <function>PQconnectdbParams</function>.
</para>
<para>
--- 621,628 ----
<para>
With <function>PQconnectStartParams</function>, the database connection is made
using the parameters taken from the <literal>keywords</literal> and
! <literal>values</literal> arrays, and controlled by <literal>expand_dbname</literal>,
! as described above for <function>PQconnectdbParams</function>.
</para>
<para>
Index: src/bin/pg_dump/pg_backup_db.c
===================================================================
RCS file: /opt/src/cvs/pgsql/src/bin/pg_dump/pg_backup_db.c,v
retrieving revision 1.85
diff -c -r1.85 pg_backup_db.c
*** src/bin/pg_dump/pg_backup_db.c 14 Dec 2009 00:39:11 -0000 1.85
--- src/bin/pg_dump/pg_backup_db.c 4 Feb 2010 03:57:42 -0000
***************
*** 154,163 ****
do
{
new_pass = false;
! newConn = PQsetdbLogin(PQhost(AH->connection), PQport(AH->connection),
! NULL, NULL, newdb,
! newuser, password);
if (!newConn)
die_horribly(AH, modulename, "failed to reconnect to database\n");
--- 154,187 ----
do
{
+ #define PARAMS_ARRAY_SIZE 7
+ const char **keywords = malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
+ const char **values = malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
+
+ if (!keywords || !values)
+ die_horribly(AH, modulename, "out of memory\n");
+
+ keywords[0] = "host";
+ values[0] = PQhost(AH->connection);
+ keywords[1] = "port";
+ values[1] = PQport(AH->connection);
+ keywords[2] = "user";
+ values[2] = newuser;
+ keywords[3] = "password";
+ values[3] = password;
+ keywords[4] = "dbname";
+ values[4] = newdb;
+ keywords[5] = "fallback_application_name";
+ values[5] = progname;
+ keywords[6] = NULL;
+ values[6] = NULL;
+
new_pass = false;
! newConn = PQconnectdbParams(keywords, values, true);
!
! free(keywords);
! free(values);
!
if (!newConn)
die_horribly(AH, modulename, "failed to reconnect to database\n");
***************
*** 237,245 ****
*/
do
{
new_pass = false;
! AH->connection = PQsetdbLogin(pghost, pgport, NULL, NULL,
! dbname, username, password);
if (!AH->connection)
die_horribly(AH, modulename, "failed to connect to database\n");
--- 261,293 ----
*/
do
{
+ #define PARAMS_ARRAY_SIZE 7
+ const char **keywords = malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
+ const char **values = malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
+
+ if (!keywords || !values)
+ die_horribly(AH, modulename, "out of memory\n");
+
+ keywords[0] = "host";
+ values[0] = pghost;
+ keywords[1] = "port";
+ values[1] = pgport;
+ keywords[2] = "user";
+ values[2] = username;
+ keywords[3] = "password";
+ values[3] = password;
+ keywords[4] = "dbname";
+ values[4] = dbname;
+ keywords[5] = "fallback_application_name";
+ values[5] = progname;
+ keywords[6] = NULL;
+ values[6] = NULL;
+
new_pass = false;
! AH->connection = PQconnectdbParams(keywords, values, true);
!
! free(keywords);
! free(values);
if (!AH->connection)
die_horribly(AH, modulename, "failed to connect to database\n");
***************
*** 697,699 ****
--- 745,748 ----
else
return false;
}
+
Index: src/bin/pg_dump/pg_dumpall.c
===================================================================
RCS file: /opt/src/cvs/pgsql/src/bin/pg_dump/pg_dumpall.c,v
retrieving revision 1.131
diff -c -r1.131 pg_dumpall.c
*** src/bin/pg_dump/pg_dumpall.c 6 Jan 2010 03:34:41 -0000 1.131
--- src/bin/pg_dump/pg_dumpall.c 4 Feb 2010 03:55:45 -0000
***************
*** 1618,1625 ****
*/
do
{
new_pass = false;
! conn = PQsetdbLogin(pghost, pgport, NULL, NULL, dbname, pguser, password);
if (!conn)
{
--- 1618,1653 ----
*/
do
{
+ #define PARAMS_ARRAY_SIZE 7
+ const char **keywords = malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
+ const char **values = malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
+
+ if (!keywords || !values)
+ {
+ fprintf(stderr, _("%s: out of memory\n"), progname);
+ exit(1);
+ }
+
+ keywords[0] = "host";
+ values[0] = pghost;
+ keywords[1] = "port";
+ values[1] = pgport;
+ keywords[2] = "user";
+ values[2] = pguser;
+ keywords[3] = "password";
+ values[3] = password;
+ keywords[4] = "dbname";
+ values[4] = dbname;
+ keywords[5] = "fallback_application_name";
+ values[5] = progname;
+ keywords[6] = NULL;
+ values[6] = NULL;
+
new_pass = false;
! conn = PQconnectdbParams(keywords, values, true);
!
! free(keywords);
! free(values);
if (!conn)
{
Index: src/bin/psql/command.c
===================================================================
RCS file: /opt/src/cvs/pgsql/src/bin/psql/command.c,v
retrieving revision 1.213
diff -c -r1.213 command.c
*** src/bin/psql/command.c 2 Jan 2010 16:57:59 -0000 1.213
--- src/bin/psql/command.c 4 Feb 2010 01:18:34 -0000
***************
*** 1213,1219 ****
* Connects to a database with given parameters. If there exists an
* established connection, NULL values will be replaced with the ones
* in the current connection. Otherwise NULL will be passed for that
! * parameter to PQsetdbLogin(), so the libpq defaults will be used.
*
* In interactive mode, if connection fails with the given parameters,
* the old connection will be kept.
--- 1213,1219 ----
* Connects to a database with given parameters. If there exists an
* established connection, NULL values will be replaced with the ones
* in the current connection. Otherwise NULL will be passed for that
! * parameter to PQconnectdbParams(), so the libpq defaults will be used.
*
* In interactive mode, if connection fails with the given parameters,
* the old connection will be kept.
***************
*** 1255,1262 ****
while (true)
{
! n_conn = PQsetdbLogin(host, port, NULL, NULL,
! dbname, user, password);
/* We can immediately discard the password -- no longer needed */
if (password)
--- 1255,1283 ----
while (true)
{
! #define PARAMS_ARRAY_SIZE 7
! const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
! const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
!
! keywords[0] = "host";
! values[0] = host;
! keywords[1] = "port";
! values[1] = port;
! keywords[2] = "user";
! values[2] = user;
! keywords[3] = "password";
! values[3] = password;
! keywords[4] = "dbname";
! values[4] = dbname;
! keywords[5] = "fallback_application_name";
! values[5] = pset.progname;
! keywords[6] = NULL;
! values[6] = NULL;
!
! n_conn = PQconnectdbParams(keywords, values, true);
!
! free(keywords);
! free(values);
/* We can immediately discard the password -- no longer needed */
if (password)
Index: src/bin/psql/startup.c
===================================================================
RCS file: /opt/src/cvs/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.159
diff -c -r1.159 startup.c
*** src/bin/psql/startup.c 28 Jan 2010 06:28:26 -0000 1.159
--- src/bin/psql/startup.c 4 Feb 2010 02:31:32 -0000
***************
*** 90,97 ****
char *password = NULL;
char *password_prompt = NULL;
bool new_pass;
- const char *keywords[] = {"host","port","dbname","user",
- "password","application_name",NULL};
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("psql"));
--- 90,95 ----
***************
*** 173,192 ****
/* loop until we have a password if requested by backend */
do
{
! const char *values[] = {
! options.host,
! options.port,
! (options.action == ACT_LIST_DB &&
! options.dbname == NULL) ? "postgres" : options.dbname,
! options.username,
! password,
! pset.progname,
! NULL
! };
!
! new_pass = false;
!
! pset.db = PQconnectdbParams(keywords, values);
if (PQstatus(pset.db) == CONNECTION_BAD &&
PQconnectionNeedsPassword(pset.db) &&
--- 171,201 ----
/* loop until we have a password if requested by backend */
do
{
! #define PARAMS_ARRAY_SIZE 7
! const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
! const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
!
! keywords[0] = "host";
! values[0] = options.host;
! keywords[1] = "port";
! values[1] = options.port;
! keywords[2] = "user";
! values[2] = options.username;
! keywords[3] = "password";
! values[3] = password;
! keywords[4] = "dbname";
! values[4] = (options.action == ACT_LIST_DB &&
! options.dbname == NULL) ?
! "postgres" : options.dbname;
! keywords[5] = "fallback_application_name";
! values[5] = pset.progname;
! keywords[6] = NULL;
! values[6] = NULL;
!
! new_pass = false;
! pset.db = PQconnectdbParams(keywords, values, true);
! free(keywords);
! free(values);
if (PQstatus(pset.db) == CONNECTION_BAD &&
PQconnectionNeedsPassword(pset.db) &&
Index: src/bin/scripts/common.c
===================================================================
RCS file: /opt/src/cvs/pgsql/src/bin/scripts/common.c,v
retrieving revision 1.38
diff -c -r1.38 common.c
*** src/bin/scripts/common.c 2 Jan 2010 16:58:00 -0000 1.38
--- src/bin/scripts/common.c 4 Feb 2010 03:54:14 -0000
***************
*** 108,115 ****
*/
do
{
new_pass = false;
! conn = PQsetdbLogin(pghost, pgport, NULL, NULL, dbname, pguser, password);
if (!conn)
{
--- 108,143 ----
*/
do
{
+ #define PARAMS_ARRAY_SIZE 7
+ const char **keywords = malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
+ const char **values = malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
+
+ if (!keywords || !values)
+ {
+ fprintf(stderr, _("%s: out of memory\n"), progname);
+ exit(1);
+ }
+
+ keywords[0] = "host";
+ values[0] = pghost;
+ keywords[1] = "port";
+ values[1] = pgport;
+ keywords[2] = "user";
+ values[2] = pguser;
+ keywords[3] = "password";
+ values[3] = password;
+ keywords[4] = "dbname";
+ values[4] = dbname;
+ keywords[5] = "fallback_application_name";
+ values[5] = progname;
+ keywords[6] = NULL;
+ values[6] = NULL;
+
new_pass = false;
! conn = PQconnectdbParams(keywords, values, true);
!
! free(keywords);
! free(values);
if (!conn)
{
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /opt/src/cvs/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.385
diff -c -r1.385 fe-connect.c
*** src/interfaces/libpq/fe-connect.c 28 Jan 2010 06:28:26 -0000 1.385
--- src/interfaces/libpq/fe-connect.c 4 Feb 2010 16:51:34 -0000
***************
*** 269,275 ****
PQExpBuffer errorMessage, bool use_defaults);
static PQconninfoOption *conninfo_array_parse(const char **keywords,
const char **values, PQExpBuffer errorMessage,
! bool use_defaults);
static char *conninfo_getval(PQconninfoOption *connOptions,
const char *keyword);
static void defaultNoticeReceiver(void *arg, const PGresult *res);
--- 269,275 ----
PQExpBuffer errorMessage, bool use_defaults);
static PQconninfoOption *conninfo_array_parse(const char **keywords,
const char **values, PQExpBuffer errorMessage,
! bool use_defaults, int expand_dbname);
static char *conninfo_getval(PQconninfoOption *connOptions,
const char *keyword);
static void defaultNoticeReceiver(void *arg, const PGresult *res);
***************
*** 336,344 ****
* call succeeded.
*/
PGconn *
! PQconnectdbParams(const char **keywords, const char **values)
{
! PGconn *conn = PQconnectStartParams(keywords, values);
if (conn && conn->status != CONNECTION_BAD)
(void) connectDBComplete(conn);
--- 336,346 ----
* call succeeded.
*/
PGconn *
! PQconnectdbParams(const char **keywords,
! const char **values,
! int expand_dbname)
{
! PGconn *conn = PQconnectStartParams(keywords, values, expand_dbname);
if (conn && conn->status != CONNECTION_BAD)
(void) connectDBComplete(conn);
***************
*** 400,406 ****
* See PQconnectPoll for more info.
*/
PGconn *
! PQconnectStartParams(const char **keywords, const char **values)
{
PGconn *conn;
PQconninfoOption *connOptions;
--- 402,410 ----
* See PQconnectPoll for more info.
*/
PGconn *
! PQconnectStartParams(const char **keywords,
! const char **values,
! int expand_dbname)
{
PGconn *conn;
PQconninfoOption *connOptions;
***************
*** 416,422 ****
* Parse the conninfo arrays
*/
connOptions = conninfo_array_parse(keywords, values,
! &conn->errorMessage, true);
if (connOptions == NULL)
{
conn->status = CONNECTION_BAD;
--- 420,427 ----
* Parse the conninfo arrays
*/
connOptions = conninfo_array_parse(keywords, values,
! &conn->errorMessage,
! true, expand_dbname);
if (connOptions == NULL)
{
conn->status = CONNECTION_BAD;
***************
*** 3729,3744 ****
* left in errorMessage.
* Defaults are supplied (from a service file, environment variables, etc)
* for unspecified options, but only if use_defaults is TRUE.
*/
static PQconninfoOption *
conninfo_array_parse(const char **keywords, const char **values,
! PQExpBuffer errorMessage, bool use_defaults)
{
char *tmp;
PQconninfoOption *options;
PQconninfoOption *option;
int i = 0;
/* Make a working copy of PQconninfoOptions */
options = malloc(sizeof(PQconninfoOptions));
if (options == NULL)
--- 3734,3786 ----
* left in errorMessage.
* Defaults are supplied (from a service file, environment variables, etc)
* for unspecified options, but only if use_defaults is TRUE.
+ *
+ * If expand_dbname is non-zero, and the value passed for keyword "dbname"
+ * contains an "=", assume it is a conninfo string and process it,
+ * overriding any previously processed conflicting keywords. Subsequent
+ * keywords will take precedence, however.
*/
static PQconninfoOption *
conninfo_array_parse(const char **keywords, const char **values,
! PQExpBuffer errorMessage, bool use_defaults,
! int expand_dbname)
{
char *tmp;
PQconninfoOption *options;
+ PQconninfoOption *str_options = NULL;
PQconninfoOption *option;
int i = 0;
+ /*
+ * If expand_dbname is non-zero, check keyword "dbname"
+ * to see if val is actually a conninfo string
+ */
+ while(expand_dbname && keywords[i])
+ {
+ const char *pname = keywords[i];
+ const char *pvalue = values[i];
+
+ /* first find "dbname" if any */
+ if (strcmp(pname, "dbname") == 0)
+ {
+ /* next look for "=" in the value */
+ if (pvalue && strchr(pvalue, '='))
+ {
+ /*
+ * Must be a conninfo string, so parse it, but do not
+ * use defaults here -- those get picked up later.
+ * We only want to override for those parameters actually
+ * passed.
+ */
+ str_options = conninfo_parse(pvalue, errorMessage, false);
+ if (str_options == NULL)
+ return NULL;
+ }
+ break;
+ }
+ ++i;
+ }
+
/* Make a working copy of PQconninfoOptions */
options = malloc(sizeof(PQconninfoOptions));
if (options == NULL)
***************
*** 3749,3754 ****
--- 3791,3797 ----
}
memcpy(options, PQconninfoOptions, sizeof(PQconninfoOptions));
+ i = 0;
/* Parse the keywords/values arrays */
while(keywords[i])
{
***************
*** 3774,3795 ****
return NULL;
}
! /*
! * Store the value
! */
! if (option->val)
! free(option->val);
! option->val = strdup(pvalue);
! if (!option->val)
! {
! printfPQExpBuffer(errorMessage,
! libpq_gettext("out of memory\n"));
! PQconninfoFree(options);
! return NULL;
! }
}
++i;
}
/*
* Stop here if caller doesn't want defaults filled in.
--- 3817,3870 ----
return NULL;
}
! /*
! * If we are on the dbname parameter, and we have a parsed
! * conninfo string, copy those parameters across, overriding
! * any existing previous settings
! */
! if (strcmp(pname, "dbname") == 0 && str_options)
! {
! PQconninfoOption *str_option;
!
! for (str_option = str_options; str_option->keyword != NULL; str_option++)
! {
! if (str_option->val != NULL)
! {
! int k;
!
! for (k = 0; options[k].keyword; k++)
! {
! if (strcmp(options[k].keyword, str_option->keyword) == 0)
! {
! if (options[k].val)
! free(options[k].val);
! options[k].val = strdup(str_option->val);
! break;
! }
! }
! }
! }
! }
! else
! {
! /*
! * Store the value, overriding previous settings
! */
! if (option->val)
! free(option->val);
! option->val = strdup(pvalue);
! if (!option->val)
! {
! printfPQExpBuffer(errorMessage,
! libpq_gettext("out of memory\n"));
! PQconninfoFree(options);
! return NULL;
! }
! }
}
++i;
}
+ PQconninfoFree(str_options);
/*
* Stop here if caller doesn't want defaults filled in.
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /opt/src/cvs/pgsql/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.150
diff -c -r1.150 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h 28 Jan 2010 06:28:26 -0000 1.150
--- src/interfaces/libpq/libpq-fe.h 4 Feb 2010 16:48:17 -0000
***************
*** 226,237 ****
/* make a new client connection to the backend */
/* Asynchronous (non-blocking) */
extern PGconn *PQconnectStart(const char *conninfo);
! extern PGconn *PQconnectStartParams(const char **keywords, const char **values);
extern PostgresPollingStatusType PQconnectPoll(PGconn *conn);
/* Synchronous (blocking) */
extern PGconn *PQconnectdb(const char *conninfo);
! extern PGconn *PQconnectdbParams(const char **keywords, const char **values);
extern PGconn *PQsetdbLogin(const char *pghost, const char *pgport,
const char *pgoptions, const char *pgtty,
const char *dbName,
--- 226,239 ----
/* make a new client connection to the backend */
/* Asynchronous (non-blocking) */
extern PGconn *PQconnectStart(const char *conninfo);
! extern PGconn *PQconnectStartParams(const char **keywords,
! const char **values, int expand_dbname);
extern PostgresPollingStatusType PQconnectPoll(PGconn *conn);
/* Synchronous (blocking) */
extern PGconn *PQconnectdb(const char *conninfo);
! extern PGconn *PQconnectdbParams(const char **keywords,
! const char **values, int expand_dbname);
extern PGconn *PQsetdbLogin(const char *pghost, const char *pgport,
const char *pgoptions, const char *pgtty,
const char *dbName,
signature.asc
Description: OpenPGP digital signature
