minfrin commented on code in PR #9: URL: https://github.com/apache/serf/pull/9#discussion_r2211562355
########## buckets/ssl_buckets.c: ########## @@ -353,10 +357,17 @@ 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())) { + + if (err && ctx->error_callback) { + char ebuf[256]; + ERR_error_string_n(err, ebuf, sizeof(ebuf)); + ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuf); Review Comment: > Is it really necessary to use an internal char array and calling ERR_error_string_n to copy the error message to this buffer. The error_callback must copy the message to an application internal buffer anyway. Wouldn't it be enough to: > > char *ebuf = ERR_error_string(err, NULL); ctx->error_callback(ctx->error_baton, ctx->fatal_err, ebuff); Alas not. ERR_error_string() uses an openssl-internal buffer that is overwritten on each call, and is not thread safe. ERR_error_string_n() fixed 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