Re: Add ssl_(supported|shared)_groups to sslinfo - Mailing list pgsql-hackers

+        /*
+         * SSL_group_to_name can return NULL in case of an error, e.g. when no
+         * such name was registered for some reason.
+         */
+        group_name = SSL_group_to_name(ssl, nid);
+        if (group_name == NULL)
+            ereport(ERROR,
+                    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                     errmsg("unknown OpenSSL group at position %d",
+                            call_cntr)));

I'm not sure if erroring out is the proper approach here. Won't this
mean that with a newer openssl version on the client side, the
function will potentially simply error out instead of returning proper
results?
For example, we could report

values[1] = StringGetTextDatum(psprintf("unknown (0x%04x)", nid & 0xffff));

in this case?


@@ -2901,6 +2904,8 @@ decl_checks = [
   ['strlcpy', 'string.h'],
   ['strsep',  'string.h'],
   ['timingsafe_bcmp',  'string.h'],
+  ['SSL_get1_groups',           'openssl/ssl.h'],
+  ['SSL_get_negotiated_group',  'openssl/ssl.h'],


This list isn't OpenSSL-aware, these won't get proper dependencies,
these should go inside the ssl.found() block

+#else
+        /* SSL groups API is not present, skip */
+        MemoryContextSwitchTo(oldcontext);
+        SRF_RETURN_DONE(funcctx);
+#endif

Shouldn't this branch have some documentation / diagnostics so it's
not a completely silent/undocumented skip?

And in the commit message:

    select * from ssl_group_info();
    type    |        name
    ------------+--------------------


That seems to be at typo/earlier leftover as it should be group_type now?