maskit commented on a change in pull request #8014:
URL: https://github.com/apache/trafficserver/pull/8014#discussion_r669222830



##########
File path: iocore/net/SSLUtils.cc
##########
@@ -412,12 +424,48 @@ ssl_client_hello_callback(SSL *s, int *al, void *arg)
 }
 #endif
 
+#ifdef OPENSSL_IS_BORINGSSL
+static ssl_select_cert_result_t
+ssl_client_hello_callback(const SSL_CLIENT_HELLO *client_hello)

Review comment:
       I don't think this function should have a different name, although it's 
a shame that we have two very similar functions.
   
   The lambda expression in `_set_handshake_callbacks` mimics OpenSSL's 
behavior, and it enables reusing two other callback functions as well. In that 
sense I think having the same name make sense.
   
   Having this in a `#elif` clause may ease the confusion.
   ```cpp
   #if TS_USE_HELLO_CB
   static int
   ssl_client_hello_callback(SSL *s, int *al, void *arg)
   {
   }
   #elif defined(OPENSSL_IS_BORINGSSL)
   static ssl_select_cert_result_t
   ssl_client_hello_callback(const SSL_CLIENT_HELLO *client_hello)
   {
   }
   #endif
   ```
   
   And we may also be able to have exactly the same function signature by 
passing `SSL_CLIENT_HELLO` to `arg`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to