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