Re: Adding support for SSLKEYLOGFILE in the frontend

On 03.04.25 17:51, Daniel Gustafsson wrote:
>> On 1 Apr 2025, at 22:22, Daniel Gustafsson <daniel@yesql.se> wrote:
>>
>> I took another pass at this one and did a few small tweaks, so I would to take
>> it for another spin across CI before looking at committing it.  There are no
>> functionality changes, only polish.
> 
> Committed, after another round of testing and looking.

A few more observations on this code:

What is the meaning of this:

+   old_umask = umask(077);
+   fd = open(conn->sslkeylogfile, O_WRONLY | O_APPEND | O_CREAT, 0600);
+   umask(old_umask);

If open() already specifies the file mode, do we still need umask()? 
Maybe I'm missing something.

Also, we usually use symbols for the modes.

+       libpq_append_conn_error(conn, "could not open ssl keylog file 
%s: %s",
+                               conn->sslkeylogfile, pg_strerror(errno));

pg_strerror() is not thread-safe, so it shouldn't be used here. 
Actually, pg_strerror() is not meant for direct use at all.  Probably 
easiest to just use %m.

Also, I can't actually trigger these errors.  This call just sticks the 
errors into the connection structure, but if the connection is otherwise 
successful, nothing will print the error.  Maybe the best we can do is 
print the error to stderr, like similarly in initialize_SSL().