On Sun, Mar 25, 2018 at 12:56 AM, Michael Paquier <mich...@paquier.xyz> wrote:
> On Sat, Mar 24, 2018 at 01:49:28AM +1100, Haribabu Kommi wrote: > > Here I attached the updated patch that returns either the connected > > host/hostaddr > > or NULL in case if the connection is not established. > > > > I removed the returning default host details, because the default host > > details are also available with the connhost member itself. > > Thanks for the review. > As shaped, PQhost generates complains from gcc with -Wreturn-type. So I > would suggest to return NULL for the default code path. As far as I can > see from the code, PGconn->connhost cannot be NULL and it should have > at least one "host" or "hostaddr" defined, so I think that we could > consider adding an assertion about that and comment out this cannot > normally be reached. > Ok. Added an assert with an explanation comment. > If the connection is bad, then whichhost points to 0, which would cause > PQhost to return the first host or hostaddr value. I think that we > should document properly to not trust the value of PQhost if the status > is CONNECTION_BAD, or to return NULL if the connection is bad as this > would have no real value for multiple hosts. I am a bit afraid of > potential breakages if we do that, so the first method may make the most > sense. The existing behavior is currently returning wrong value when the connection status is CONNECTION_BAD. As we are changing the behavior of the function, it may be better to handle the CONNECTION_BAD scenario also instead of providing note in the manual? > The same things apply to PQport as multiple ports can be > defined. Thoughts? > Yes, I changed PQport also to return the connected port or NULL, Removed the returning of all the ports specified in the connection string. > I have quickly looked at the callers of PQhost in the core code and > those seem safe. Something to keep in mind. > > More details in the documentation would be nice. Let's detail the > following: > - PQhost returns NULL if the connection is not established yet. > - PQhost may return an incorrect value when with CONNECTION_BAD as > status. > - If both hostaddr and host are precised in the conneciton string, then > host has the priority. > - If only hostaddr is precised, then this is the value returned. > Docs are updated with the new behavior of the functions. Updated patch attached with behavior of returning NULL for connections of CONNECTION_BAD status. Regards, Hari Babu Fujitsu Australia
PQhost-to-return-connected-host-and-hostaddr-details_v4.patch
Description: Binary data
