On Tue Jul 14, 2026 at 8:35 AM UTC, Daniel Gustafsson wrote: >> On 14 Jul 2026, at 00:25, Andreas Karlsson <[email protected]> wrote: >> On 7/13/26 16:16, Daniel Gustafsson wrote: >>> * 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. > > Ah, yes. It was already in the inner scope, but it should've been in > inner-inner 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. > > The other pointers are of OpenSSL data types and those are rarely freed unless > a matching _new or _dup method has been called, whereas peer_cn_internal is a > char pointer extracted with _get0_data. To me the latter seemed more > reasonable to believe it was caller owned, but I can remove the comment > altogether if it's deemed useless. > >> 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. > > We can't use strncpy or strlcpy here since OpenSSL had defined the string as > const unsigned char * so we'd get pointer-sign compiler warnings unless we do > trickery, which seems not worth it. I don't have strong opinions on how to > zero the buffer so changed to your proposal.
I wonder if it is worth leaving your justification in a comment. Other than that, nothing to add. -- Tristan Partin PostgreSQL Contributors Team AWS (https://aws.amazon.com)
