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 accept

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 B:

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 br...@momjian.us 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

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 B:

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 ssin...@ca.afilias.info 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

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 ssin...@ca.afilias.info 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

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

2013-03-25 Thread Tom Lane
Bruce Momjian br...@momjian.us 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

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 br...@momjian.us 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

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

2013-03-25 Thread Tom Lane
Bruce Momjian br...@momjian.us 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

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 if

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

2013-03-20 Thread Tom Lane
Bruce Momjian br...@momjian.us 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

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 br...@momjian.us 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

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

2013-03-20 Thread Tom Lane
Bruce Momjian br...@momjian.us 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

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 br...@momjian.us 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,

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

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

2013-03-20 Thread Tom Lane
Steve Singer ssin...@ca.afilias.info 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.

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.

[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;

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_envvar ()