On 13-03-18 09:17 PM, Bruce Momjian wrote:
On Mon, Mar 18, 2013 at 12:08:09PM -0400, Steve Singer wrote:
If you try running pg_upgrade with the PGSERVICE environment
variable set to some invalid/non-existent service pg_upgrade
segfaults
Program received signal SIGSEGV, Segmentation fault.
0x000000000040bdd1 in check_pghost_envvar () at server.c:304
304 for (option = start; option->keyword != NULL; option++)
(gdb) p start
$5 = (PQconninfoOption *) 0x0
PQconndefaults can return NULL if it has issues.
The attached patch prints a minimally useful error message. I don't
a good way of getting a more useful error message out of
PQconndefaults()
I checked this against master but it was reported to me as a issue in 9.2
Well, that's interesting. There is no mention of PQconndefaults()
returning NULL except for out of memory:
Returns a connection options array. This can be used to determine
all possible <function>PQconnectdb</function> options and their
current default values. The return value points to an array of
<structname>PQconninfoOption</structname> structures, which ends
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.
Looking at libpq/fe-connect.c::PQconndefaults(), it calls
conninfo_add_defaults(), which has this:
/*
* If there's a service spec, use it to obtain any not-explicitly-given
* parameters.
*/
if (parseServiceInfo(options, errorMessage) != 0)
return false;
so it is clearly possible for PQconndefaults() to return NULL for
service file failures. The questions are:
* Is this what we want?
What other choices do we have? I don't think PQconndefaults() should
continue on as if PGSERVICE wasn't set in the environment after a
failure from parseServiceInfo.
* Should we document this?
Yes the documentation should indicate that PQconndefaults() can return
NULL for more than just memory failures.
* Should we change this to just throw a warning?
How would we communicate warnings from PQconndefaults() back to the caller?
Also, it seems pg_upgrade isn't the only utility that is confused:
contrib/postgres_fdw/option.c and contrib/dblink/dblink.c think
PQconndefaults() returning NULL means out of memory and report that
as the error string.
bin/scripts/pg_isready.c and contrib/pg_upgrade/server.c have no
check for NULL return.
libpq/test/uri-regress.c knows to throw a generic error message.
So, we have some decisions and work to do.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
