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? * Should we document this? * Should we change this to just throw a warning? 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. -- Bruce Momjian <br...@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers