From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
Cc: | Дмитрий Воронин <carriingfate92(at)yandex(dot)ru>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: New functions |
Date: | 2015-08-23 13:21:09 |
Message-ID: | CAB7nPqSK_SN861XDnGjsiy43mW-TK9pCMWYE0ptRxKZZC=cgYQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Aug 22, 2015 at 11:24 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> On 07/08/2015 01:15 PM, Дмитрий Воронин wrote:
>>
>> 07.07.2015, 18:34, "Michael Paquier" <michael(dot)paquier(at)gmail(dot)com>:
>>
>>> Speaking of which, I have rewritten the patch as attached. This looks
>>> way cleaner than the previous version submitted. Dmitry, does that
>>> look fine for you?
>>> I am switching this patch as "Waiting on Author".
>>
>>
>> Michael, hello. I'm looking your variant of patch. You create
>> function ssl_extensions_info(), that gives information of SSL
>> extensions in more informative view. So, it's cool.
>
>
> Should check the return value of every OpenSSL call for errors. At least
> BIO_new() could return NULL, but check all the docs of the others too.
Right, agreed for BIO_new(). BIO_write and BIO_get_mem_data can return
negative error code, but that's not necessarily the indication of an
error by looking at the docs, so I'd rather let them as-is.
X509V3_EXT_print is not documented but it can return <= 0 state code
if the operation fails so I guess that it makes sense to elog an
ERROR. sk_X509_EXTENSION_value and X509_EXTENSION_get_object return
NULL in case of failure (looking at the code tree of openssl), and
OBJ_obj2nid will return NID_undef in this case, so I think the code
as-is is fine. Another interesting thing is that BIO_free can fail and
we don't track that.
By the way, perhaps it would be worth doing similar things for the
other calls of BIO_free and BIO_new, no? I have attached a second
patch.
> Are all the functions used in the patch available in all the versions of
> OpenSSL we support?
We support openssl down to 0.9.7, right? And those functions are
present there (I recall vaguely checking that when looking at this
patch, and I just rechecked in case I missed something).
> Other than those little things, looks good to me.
Thanks!
--
Michael
Attachment | Content-Type | Size |
---|---|---|
0001-Add-function-for-SSL-extension-information-in-sslinf.patch | text/x-patch | 9.4 KB |
0002-Add-more-sanity-checks-in-sslinfo.patch | text/x-patch | 2.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2015-08-23 13:34:35 | Re: New functions |
Previous Message | Fabien COELHO | 2015-08-23 11:25:37 | pgbench progress with timestamp |