On Wed, May 17, 2017 at 11:49 AM, Tsunakawa, Takayuki <tsunakawa.ta...@jp.fujitsu.com> wrote: > From: pgsql-hackers-ow...@postgresql.org >> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Michael Paquier >> pqWait is internal to libpq, so we are free to set up what we want here. >> Still I think that we should be consistent with what pqSocketCheck returns: > > Please let this what it is now for the same reason Robert mentioned. > >> + int ret = 0; >> + int timeout = 0; >> The declaration of "ret" should be internal in the for(;;) loop. > > Done. > >> + /* Attempt connection to the next host, starting the >> connect_timeout timer */ >> + pqDropConnection(conn, true); >> + conn->addr_cur = conn->connhost[conn->whichhost].addrlist; >> + conn->status = CONNECTION_NEEDED; >> + finish_time = time(NULL) + timeout; >> + } >> I think that it would be safer to not set finish_time if >> conn->connect_timeout is NULL. I agree that your code works because >> pqWaitTimed() will never complain on timeout reached if finish_time is -1. >> That's for robustness sake. > > Done, but I'm not sure how this contributes to the robustness. I guess you > were concerned just in case pqWaitTimed() returned 0 (timeout) even when it > should not.
Thanks for the updated patch. This looks good to me. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
