On 7/13/26 16:16, Daniel Gustafsson wrote:
> I've been collecting a few small fixups to the OpenSSL code and figured it was
> time to send them over to keep the number of fixes more manageable.
Nice!
> * 0001: Remove the use of static variable to not interfere with the future
> multithreading efforts
Looks good!
> * 0002: Replace SSLv23_method with TLS_method, it has been an alias to the new
> name since 2015 in OpenSSL
Looks good!
> * 0003: Replace deprecated API X509_NAME_get_text_by_NID with the recommended
> alternative X509_NAME_get_index_by_NID + X509_NAME_get_entry. The API was
> deprecated in OpenSSL 4 and risk getting removed, with the alternatives being
> available in all supported versions.
You should declare entry in the inner scope, not in the function scope.
The comment should maybe also add that entry and peer_cn_asn1 too are
OpenSSL owned pointers. Just saying it for peer_cn_internal seems a bit
odd to me.
Also this is subjective but I am not personally a fan of
MemoryContextAllocZero() + memcpy(). I prefer MemoryContextAlloc() +
memcpy() + peer_cn[len] = 0. I feel that explains better what is going
on. I have also seen us use strncpy() for this purpose which I guess
works too.
> * 0004: Fix TLS protocol detection. We have been using the wrong macro for
> feature test for a long time, replace with the actual feature test macro.
> (There is no protocol downgrade here, it will error out when misconfiguring).
Looks good! That was a funny and easy to make mistake :)
--
Andreas Karlsson
Percona