Re: pg_upgrade segfaults when given an invalid PGSERVICE value

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.

Yep, that's pretty much it. I agree B is overkill, and D seems
dangerous. I think Tom likes C --- my only question is how many people
will be confused that C returns inaccurate information, because though
it returns connection options, it doesn't process the service file,
while forced processing of the service file for real connections throws
an error.

We might be fine with C, but it must be documented in the C code and
SGML docs.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +