On Mon, Mar 26, 2018 at 8:24 PM, Michael Paquier <michael@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"...).
