Re: PQHost() undefined behavior if connecting string contains bothhost and hostaddr types - Mailing list pgsql-hackers
| From | David G. Johnston |
|---|---|
| Subject | Re: PQHost() undefined behavior if connecting string contains bothhost and hostaddr types |
| Date | |
| Msg-id | CAKFQuwYZyw6gYEN2Ki6MW_jhLE4Lb+wXYRVwzrWeT4H-wfM_4w@mail.gmail.com Whole thread Raw |
| In response to | Re: PQHost() undefined behavior if connecting string contains bothhost and hostaddr types (Michael Paquier <michael@paquier.xyz>) |
| Responses |
Re: PQHost() undefined behavior if connecting string contains bothhost and hostaddr types
(Michael Paquier <michael@paquier.xyz>)
Re: PQHost() undefined behavior if connecting string contains bothhost and hostaddr types (Haribabu Kommi <kommi.haribabu@gmail.com>) |
| List | pgsql-hackers |
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"...).
David J.
pgsql-hackers by date: