Andrew Chernow wrote:
> Andrew Chernow wrote:
>> Robert Haas wrote:
>>> On Fri, Feb 13, 2009 at 12:06 PM, Andrew Chernow <ac@esilo.com> wrote:
>>>> Patch attached.
>>>>
>>>> One thing I noticed is the ssl_open_connections variable is ref
>>>> counting
>>>> connections when pq_initssllib is true. But, it now only affects
>>>> crypto
>>>> library init and cleanup calls. Point is, ref counting is only
>>>> needed if
>>>> pq_initcryptolib is true and it should be renamed to
>>>> crypto_open_connections. I didn't do this in the patch. Its the
>>>> same old
>>>> name and the counter is incremented if pq_initssllib or
>>>> pq_initcryptolib is
>>>> true. Please advise.
>>>
>>> I'll review this in more detail when I have a chance, but it certainly
>>> won't be committable without doc changes, and it's probably best if
>>> you write those and include them in the patch.
>>>
>
> One problem with this patch is that a libpq app using PQinitSSL(0) is
> under the assumption that this shuts off ssl init and crypto init. That
> app might be doing its own crypto init which would be overwritten by
> libpq because the app is unaware of PQinitCrypto (if and when it
> eventually links with 8.4 libpq). This feels like a very uncommon
> situation, but a possible gotcha.
>
(sorry I keep posting)
>This feels like a very uncommon situation
I take that back. Not so sure it is uncommon, any threaded libpq app
would probably get bit if they called PQinitSSL. On top of that, it
could take up to a year before complaints start rolling in, as 8.4 hits
the distros. Yuck.
I now think the the orignal suggestion of PQinitSSLExtended is better
than PQinitCrypto. With PQinitSSLExtended, PQinitSSL needs a minor
implementation adjustment but the behvior remains the same. The
extended version is probably:
/* IMHO appending "Ex" is a little nicer */
void PQinitSSLEx(int ssl_init, int crypto_init);
/* PQinitSSL wraps PQinitSSLEx */
void PQinitSSL(int do_init)
{ PQinitSSLEx(do_init, do_init);
}
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/