On Thu, Mar 28, 2013 at 03:06:30PM -0400, Steve Singer wrote:
> So to summarise:
>
> Plan A: The first patch I attached for pg_upgrade + documentation
> changes, and changing the other places that call PQconndefaults() to
> accept failures on either out of memory, or an invalid PGSERVICE
>
> Plan B: Create a new function PQconndefaults2(char * errorBuffer) or
> something similar that returned error information to the caller.
>
> Plan C: PQconndefaults() just ignores an invalid service but
> connection attempts fail because other callers of
> conninfo_add_defaults still pay attention to connection failures.
> This is the second patch I sent.
>
> Plan D: Service lookup failures are always ignored by
> conninfo_add_defaults. If you attempt to connect with a bad
> PGSERVICE set it will behave as if no PGSERVICE value was set. I
> don't think anyone explicitly proposed this yet.
>
> Plan 'D' is the only option that I'm opposed to, it will effect a
> lot more applications then ones that call PQconndefaults() and I
> feel it will confuse users.
>
> I'm not convinced plan B is worth the effort of having to maintain
> two versions of PQconndefaults() for a while to fix a corner case.
OK, I am back to looking at this issue from March. I went with option
C, patch attached, which seems to be the most popular. It is similar to
Steve's patch, but structured differently and includes a doc patch.
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c
new file mode 100644
index b75f553..7d2aa35
*** a/contrib/pg_upgrade/server.c
--- b/contrib/pg_upgrade/server.c
*************** check_pghost_envvar(void)
*** 325,330 ****
--- 325,333 ----
start = PQconndefaults();
+ if (!start)
+ pg_fatal("out of memory\n");
+
for (option = start; option->keyword != NULL; option++)
{
if (option->envvar && (strcmp(option->envvar, "PGHOST") == 0 ||
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
new file mode 100644
index 955f248..503a63a
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
*************** typedef struct
*** 483,489 ****
with an entry having a null <structfield>keyword</> pointer. The
null pointer is returned if memory could not be allocated. Note that
the current default values (<structfield>val</structfield> fields)
! will depend on environment variables and other context. Callers
must treat the connection options data as read-only.
</para>
--- 483,490 ----
with an entry having a null <structfield>keyword</> pointer. The
null pointer is returned if memory could not be allocated. Note that
the current default values (<structfield>val</structfield> fields)
! will depend on environment variables and other context. A
! missing or invalid service file will be silently ignored. Callers
must treat the connection options data as read-only.
</para>
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
new file mode 100644
index 975f795..9797140
*** a/src/interfaces/libpq/fe-auth.c
--- b/src/interfaces/libpq/fe-auth.c
*************** pg_fe_sendauth(AuthRequest areq, PGconn
*** 982,988 ****
* if there is an error, return NULL with an error message in errorMessage
*/
char *
! pg_fe_getauthname(PQExpBuffer errorMessage)
{
const char *name = NULL;
char *authn;
--- 982,988 ----
* if there is an error, return NULL with an error message in errorMessage
*/
char *
! pg_fe_getauthname(void)
{
const char *name = NULL;
char *authn;
diff --git a/src/interfaces/libpq/fe-auth.h b/src/interfaces/libpq/fe-auth.h
new file mode 100644
index bfddc68..25440b0
*** a/src/interfaces/libpq/fe-auth.h
--- b/src/interfaces/libpq/fe-auth.h
***************
*** 19,24 ****
extern int pg_fe_sendauth(AuthRequest areq, PGconn *conn);
! extern char *pg_fe_getauthname(PQExpBuffer errorMessage);
#endif /* FE_AUTH_H */
--- 19,24 ----
extern int pg_fe_sendauth(AuthRequest areq, PGconn *conn);
! extern char *pg_fe_getauthname(void);
#endif /* FE_AUTH_H */
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
new file mode 100644
index 8dd1a59..7ab4a9a
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
*************** PQconndefaults(void)
*** 875,881 ****
connOptions = conninfo_init(&errorBuf);
if (connOptions != NULL)
{
! if (!conninfo_add_defaults(connOptions, &errorBuf))
{
PQconninfoFree(connOptions);
connOptions = NULL;
--- 875,882 ----
connOptions = conninfo_init(&errorBuf);
if (connOptions != NULL)
{
! /* pass NULL errorBuf to ignore errors */
! if (!conninfo_add_defaults(connOptions, NULL))
{
PQconninfoFree(connOptions);
connOptions = NULL;
*************** conninfo_array_parse(const char *const *
*** 4412,4420 ****
*
* Defaults are obtained from a service file, environment variables, etc.
*
! * Returns TRUE if successful, otherwise FALSE; errorMessage is filled in
! * upon failure. Note that failure to locate a default value is not an
! * error condition here --- we just leave the option's value as NULL.
*/
static bool
conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
--- 4413,4422 ----
*
* Defaults are obtained from a service file, environment variables, etc.
*
! * Returns TRUE if successful, otherwise FALSE; errorMessage, if supplied,
! * is filled in upon failure. Note that failure to locate a default value
! * is not an error condition here --- we just leave the option's value as
! * NULL.
*/
static bool
conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
*************** conninfo_add_defaults(PQconninfoOption *
*** 4424,4432 ****
/*
* If there's a service spec, use it to obtain any not-explicitly-given
! * parameters.
*/
! if (parseServiceInfo(options, errorMessage) != 0)
return false;
/*
--- 4426,4435 ----
/*
* If there's a service spec, use it to obtain any not-explicitly-given
! * parameters. Ignore error if no error message buffer is passed
! * because there is no way to pass back the failure message.
*/
! if (parseServiceInfo(options, errorMessage) != 0 && errorMessage)
return false;
/*
*************** conninfo_add_defaults(PQconninfoOption *
*** 4448,4455 ****
option->val = strdup(tmp);
if (!option->val)
{
! printfPQExpBuffer(errorMessage,
! libpq_gettext("out of memory\n"));
return false;
}
continue;
--- 4451,4459 ----
option->val = strdup(tmp);
if (!option->val)
{
! if (errorMessage)
! printfPQExpBuffer(errorMessage,
! libpq_gettext("out of memory\n"));
return false;
}
continue;
*************** conninfo_add_defaults(PQconninfoOption *
*** 4465,4472 ****
option->val = strdup(option->compiled);
if (!option->val)
{
! printfPQExpBuffer(errorMessage,
! libpq_gettext("out of memory\n"));
return false;
}
continue;
--- 4469,4477 ----
option->val = strdup(option->compiled);
if (!option->val)
{
! if (errorMessage)
! printfPQExpBuffer(errorMessage,
! libpq_gettext("out of memory\n"));
return false;
}
continue;
*************** conninfo_add_defaults(PQconninfoOption *
*** 4477,4483 ****
*/
if (strcmp(option->keyword, "user") == 0)
{
! option->val = pg_fe_getauthname(errorMessage);
continue;
}
}
--- 4482,4488 ----
*/
if (strcmp(option->keyword, "user") == 0)
{
! option->val = pg_fe_getauthname();
continue;
}
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
