minfrin commented on code in PR #9:
URL: https://github.com/apache/serf/pull/9#discussion_r2179446864


##########
buckets/ssl_buckets.c:
##########
@@ -334,10 +338,20 @@ detect_renegotiate(const SSL *s, int where, int ret)
 
 static void log_ssl_error(serf_ssl_context_t *ctx)
 {
-    unsigned long e = ERR_get_error();
-    serf__log(LOGLVL_ERROR, LOGCOMP_SSL, __FILE__, ctx->config,
-              "SSL Error: %s\n", ERR_error_string(e, NULL));
+    unsigned long err;
+
+    while ((err = ERR_get_error())) {
+
+        serf__log(LOGLVL_ERROR, LOGCOMP_SSL, __FILE__, ctx->config,
+                  "SSL Error: %s\n", ERR_error_string(err, NULL));
 
+        if (err && ctx->error_callback) {
+            char ebuf[256];
+            ERR_error_string_n(err, ebuf, sizeof(ebuf));

Review Comment:
   The logging was left unchanged, but really should go.
   
   The logging is the crutch that was supporting the lack of error handling. 
Libraries should definitely not be logging anything.
   



##########
buckets/ssl_buckets.c:
##########
@@ -1652,22 +1658,32 @@ static int ssl_need_client_cert(SSL *ssl, X509 **cert, 
EVP_PKEY **pkey)
                             }
                             return 1;
                         }
+                        else {
+                            err = ERR_get_error();
+                            ERR_clear_error();

Review Comment:
   This was from before we used log_ssl_error(), I've simplified this.



-- 
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