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.
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.
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 same things apply to PQport as multiple ports can be
defined. Thoughts?
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.
I would really love to see this patch go into v11, and this could unlock
the other one you wrote for pg_stat_wal_receiver.
Thanks,
--
Michael
