> On 14 Apr 2023, at 01:27, Tom Lane <t...@sss.pgh.pa.us> wrote: > > Daniel Gustafsson <dan...@yesql.se> writes: >> Good points, it should of course be SOCK_ERRNO. The attached saves off errno >> and reinstates it to avoid clobbering. Will test it on Windows in the >> morning >> as well. > > I think instead of this: > > + SOCK_ERRNO_SET(save_errno); > > you could just do this: > > libpq_append_conn_error(conn, "SSL SYSCALL error: %s", > - SOCK_STRERROR(SOCK_ERRNO, sebuf, > sizeof(sebuf))); > + SOCK_STRERROR(save_errno, sebuf, > sizeof(sebuf))); > > Although ... we're already assuming that SSL_get_error and ERR_get_error > don't clobber errno. Maybe SSL_get_verify_result doesn't either. > Or we could make it look like this: > > + SOCK_ERRNO_SET(0); > ERR_clear_error(); > r = SSL_connect(conn->ssl); > if (r <= 0) > + int save_errno = SOCK_ERRNO; > int err = SSL_get_error(conn->ssl, r); > unsigned long ecode; > > ... > > - SOCK_STRERROR(SOCK_ERRNO, sebuf, > sizeof(sebuf))); > + SOCK_STRERROR(save_errno, sebuf, > sizeof(sebuf))); > > to remove all doubt.
I mainly put save_errno back into SOCK_ERRNO for greppability, I don't have any strong opinions either way so I went with the latter suggestion. Attached v3 does the above change and passes the tests both with a broken and working system CA pool. Unless objections from those with failing local envs I propose this is pushed to close the open item. -- Daniel Gustafsson
libpq_system_ca_fix_v3.diff
Description: Binary data