On Tue, Mar 27, 2018 at 3:03 PM, David G. Johnston < david.g.johns...@gmail.com> wrote:
> On Mon, Mar 26, 2018 at 8:24 PM, Michael Paquier <mich...@paquier.xyz> > wrote: > >> On Tue, Mar 27, 2018 at 11:43:27AM +1100, Haribabu Kommi wrote: >> > Patch attached with the above behavior along with other comments from >> > upthread. >> >> Thanks for the updated version. >> >> The function changes look logically good to me. >> >> + <para> >> + The <function>PQhost</function> function returns NULL when the >> + input conn parameter is NULL or an empty string if conn cannot be >> evaluated. >> + Applications of this function must carefully evaluate the return >> value. >> + </para> >> >> - "Applications of this function must carefully evaluate the return >> value" is rather vague, so I would append to the end "depending on the >> state of the connection involved." >> The same applies to PQport() for consistency. >> >> Perhaps the documentation should mention as well that making the >> difference between those different values is particularly relevant when >> using multiple-value strings? I would rather add one paragraph on the >> matter than nothing. I really think that we have been lacking clarity >> in the documentation for those APIs for too long, and people always >> argue about what they should do. If we have a base documented, then we >> can more easily argue for the future as well, and things are clear to >> the user. >> > > > "depending on the state of the connection" doesn't move the goal-posts > that far though...and "Applications of this function" would be better > written as "Callers of this function" if left in place. > > In any case something like the following framework seems more useful to > the reader who doesn't want to scan the source code for the PQhost/PQport > functions. > > The PQhost function returns NULL when the input conn parameter is NULL or > an empty string if conn cannot be evaluated. Otherwise, the return value > depends on the state of the conn: specifically (translate code to > documentation here). Furthermore, if both host and hostaddr properties > exist on conn the return value will contain only the host. > > I'm undecided on the need for a <note> element but would lean against it > in favor of the above, slightly longer, paragraph. > > And yes, while I'm not sure right now what the multi-value condition logic > results in it should be mentioned - at least if the goal of the docs is to > be a sufficient resource for using these functions. In particular what > happens when the connection is inactive (unless that falls under "cannot be > evaluated"...). > Thanks for the review. updated patch attached with additional doc updates as per the suggestion from the upthreads. Regards, Hari Babu Fujitsu Australia
PQhost-to-return-active-connected-host-and-hostaddr_v7.patch
Description: Binary data
