Re: New functions

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

In response to

Responses

Browse pgsql-hackers by date

  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