Re: [HACKERS] innocuous: pgbench does FD_ISSET on invalid socket

2016-02-15 Thread Michael Paquier
On Tue, Feb 16, 2016 at 8:47 AM, Alvaro Herrera wrote: > Michael Paquier wrote: > >> Different issues in the same area: >> 1) In vacuumdb.c, init_slot() does not check for the return value of >> PQsocket(): >> slot->sock = PQsocket(conn); >> 2) In isolationtester.c,

Re: [HACKERS] innocuous: pgbench does FD_ISSET on invalid socket

2016-02-15 Thread Alvaro Herrera
I noticed that strerror(errno) wasn't the most helpful error context ever, so I changed it to PQerrorMessage(). There may be room for additional improvement there. I also noticed that if one connection dies, we terminate the whole thread, and if the thread is serving multiple connections, the

Re: [HACKERS] innocuous: pgbench does FD_ISSET on invalid socket

2016-02-15 Thread Fabien COELHO
Hello Alvaro, Any objections to changing it like this? I'd probably backpatch to 9.5, but no further (even though this pattern actually appears all the way back.) My 0.02€: if the pattern is repeated, maybe a function which incorporates the check would save lines and improve overall

Re: [HACKERS] innocuous: pgbench does FD_ISSET on invalid socket

2016-02-14 Thread Michael Paquier
On Mon, Feb 15, 2016 at 3:20 PM, Michael Paquier wrote: > On Sat, Feb 13, 2016 at 6:25 AM, Alvaro Herrera > wrote: >> I noticed that pgbench calls FD_ISSET on a socket returned by >> PQsocket() without first checking that it's not invalid. I

Re: [HACKERS] innocuous: pgbench does FD_ISSET on invalid socket

2016-02-14 Thread Michael Paquier
On Sat, Feb 13, 2016 at 6:25 AM, Alvaro Herrera wrote: > I noticed that pgbench calls FD_ISSET on a socket returned by > PQsocket() without first checking that it's not invalid. I don't think > there's a real problem here because the socket was already checked a few >

[HACKERS] innocuous: pgbench does FD_ISSET on invalid socket

2016-02-12 Thread Alvaro Herrera
I noticed that pgbench calls FD_ISSET on a socket returned by PQsocket() without first checking that it's not invalid. I don't think there's a real problem here because the socket was already checked a few lines above, but I think applying the same coding pattern to both places is cleaner. Any