Re: PQinitSSL broken in some use casesf

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/