brainy commented on code in PR #8:
URL: https://github.com/apache/serf/pull/8#discussion_r2160046553


##########
buckets/ssl_buckets.c:
##########
@@ -1567,14 +1626,214 @@ static int ssl_need_client_cert(SSL *ssl, X509 **cert, 
EVP_PKEY **pkey)
         return 1;
     }
 
+#if defined(SERF_HAVE_OSSL_STORE_OPEN_EX)
+
+    if (ssl_x509_ex_data_idx < 0) {
+        ssl_x509_ex_data_idx = X509_get_ex_new_index(0, NULL, NULL, NULL, 
NULL);

Review Comment:
   This is not thread safe, nor are the other two uses of this variable. Serf's 
pipeline may be single-threaded asynchronous, but the restriction is only that 
a particular serf context may not be used in multiple threads. Access to the 
static variable must still be serialized. Could you use apr_atomic for this? Or 
maybe put this into the Serf context? Does it have to be a singleton per 
process?



##########
CMakeLists.txt:
##########
@@ -298,6 +298,9 @@ CheckNotFunction("X509_STORE_CTX_get0_chain" "NULL" 
"SERF_NO_SSL_X509_GET0_CHAIN
 CheckNotFunction("ASN1_STRING_get0_data" "NULL" 
"SERF_NO_SSL_ASN1_STRING_GET0_DATA"
                  "openssl/asn1.h" "${OPENSSL_INCLUDE_DIR}"
                  ${OPENSSL_LIBRARIES} ${SERF_STANDARD_LIBRARIES})
+CheckFunction("OSSL_STORE_open_ex" "NULL, NULL, NULL, NULL, NULL, NULL, NULL, 
NULL" "SERF_HAVE_OSSL_STORE_OPEN_EX"

Review Comment:
   Please try to stay below 80-columns (not just here).



##########
buckets/ssl_buckets.c:
##########
@@ -117,6 +126,8 @@
  *
  */
 
+static int ssl_x509_ex_data_idx = -1;

Review Comment:
   Oops ... see below.



-- 
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: dev-unsubscr...@serf.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to