Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-12-03 Thread Bruce Momjian
On Fri, Nov 29, 2013 at 04:14:00PM -0500, Bruce Momjian wrote: > 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 > >

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-11-29 Thread Bruce Momjian
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

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-28 Thread Bruce Momjian
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

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-28 Thread Steve Singer
On 13-03-26 12:40 AM, Tom Lane wrote: Bruce Momjian writes: On Mon, Mar 25, 2013 at 07:07:42PM -0400, Tom Lane wrote: Well, plan B would be to invent a replacement function that does have the ability to return an error message, but that seems like a lot of work for a problem that's so marginal

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-25 Thread Tom Lane
Bruce Momjian writes: > On Mon, Mar 25, 2013 at 07:07:42PM -0400, Tom Lane wrote: >> Well, plan B would be to invent a replacement function that does have >> the ability to return an error message, but that seems like a lot of >> work for a problem that's so marginal that it wasn't noticed till no

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-25 Thread Bruce Momjian
On Mon, Mar 25, 2013 at 07:07:42PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > We are taking this approach because PQconndefaults() doesn't have an API > > to return the error cause, while other API calls do. Returning true so > > we can later report the right error from a later API call j

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-25 Thread Tom Lane
Bruce Momjian writes: > We are taking this approach because PQconndefaults() doesn't have an API > to return the error cause, while other API calls do. Returning true so > we can later report the right error from a later API call just feels > wrong. Well, plan B would be to invent a replacement

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-25 Thread Bruce Momjian
On Mon, Mar 25, 2013 at 10:59:21AM -0400, Steve Singer wrote: > On 13-03-20 05:54 PM, Tom Lane wrote: > >Steve Singer writes: > > > > >> From a end-user expectations point of view I am okay with somehow > >>marking the structure returned by PQconndefaults in a way that the > >>connect calls will

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-25 Thread Steve Singer
On 13-03-20 05:54 PM, Tom Lane wrote: Steve Singer writes: From a end-user expectations point of view I am okay with somehow marking the structure returned by PQconndefaults in a way that the connect calls will later fail. Unless the program changes the value of PGSERVICE, surely all su

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-20 Thread Tom Lane
Steve Singer writes: > On 13-03-20 02:17 PM, Bruce Momjian wrote: >> I think the concern is that the services file could easily change the >> defaults that are used for connecting, though you could argue that the >> real defaults for a bad service entry are properly returned. > Yes, my concern is

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-20 Thread Steve Singer
On 13-03-20 02:17 PM, Bruce Momjian wrote: On Wed, Mar 20, 2013 at 01:30:20PM -0400, Tom Lane wrote: While this surely isn't the nicest answer, it doesn't seem totally unreasonable to me. A bad service name indeed does not contribute anything to the set of defaults available. I think the c

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-20 Thread Bruce Momjian
On Wed, Mar 20, 2013 at 01:30:20PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > On Wed, Mar 20, 2013 at 12:30:32PM -0400, Tom Lane wrote: > >> I think we should either change PQconndefaults to *not* fail in this > >> circumstance, or find a way to return an error message. > > > Well, Steve

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-20 Thread Tom Lane
Bruce Momjian writes: > On Wed, Mar 20, 2013 at 12:30:32PM -0400, Tom Lane wrote: >> I think we should either change PQconndefaults to *not* fail in this >> circumstance, or find a way to return an error message. > Well, Steve Singer didn't like the idea of ignoring a service lookup > failure. W

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-20 Thread Bruce Momjian
On Wed, Mar 20, 2013 at 12:30:32PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > On Tue, Mar 19, 2013 at 10:12:21AM -0400, Steve Singer wrote: > >>> * Should we document this? > > >> Yes the documentation should indicate that PQconndefaults() can > >> return NULL for more than just memory f

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-20 Thread Tom Lane
Bruce Momjian writes: > On Tue, Mar 19, 2013 at 10:12:21AM -0400, Steve Singer wrote: >>> * Should we document this? >> Yes the documentation should indicate that PQconndefaults() can >> return NULL for more than just memory failures. > The attached patch fixes this. I am unclear about backpat

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-20 Thread Bruce Momjian
On Tue, Mar 19, 2013 at 10:12:21AM -0400, Steve Singer wrote: > >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

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-19 Thread Steve Singer
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. 0x000

Re: [HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-18 Thread Bruce Momjian
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. > 0x0040bdd1 in check_pghost_envv

[HACKERS] pg_upgrade segfaults when given an invalid PGSERVICE value

2013-03-18 Thread Steve Singer
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. 0x0040bdd1 in check_pghost_envvar () at server.c:304 304 for (option = start; option->keywo