> On 14 Mar 2025, at 00:02, Abhishek Chanda <abhishek.b...@gmail.com> wrote: > > Thanks, Daniel. > > Should there be the ifdef guard in here as well? > > + {"sslkeylogfile", NULL, NULL, NULL, > + "SSL-Key-Log-File", "", 0, /* sizeof("") = 0 */ > + offsetof(struct pg_conn, sslkeylogfile)},
No, we want the option to work even if the feature doesn't in order to not make connection strings dependent on compilation options. > A small nit, this line should say NULL > > + /* line is guaranteed by OpenSSL to be NUL terminated */ The variable is terminated by the NUL character so I believe this is actually correct, if you look around in the source tree you'll find many more references. > On Thu, Mar 13, 2025 at 5:07 PM Daniel Gustafsson <dan...@yesql.se> wrote: >> >>> On 13 Mar 2025, at 19:31, Tom Lane <t...@sss.pgh.pa.us> wrote: >>> >>> Jacob Champion <jacob.champ...@enterprisedb.com> writes: >>>> Adding the PG prefix to the envvar name addresses my collision >>>> concern, but I think Tom's comment upthread [1] was saying that we >>>> should not provide any envvar at all: >>> >>>>> I think it might be safer if we only accepted it as a connection >>>>> parameter and not via an environment variable. >>> >>>> Is the addition of the PG prefix enough to address that concern too? >>> >>> Indeed, I was advocating for *no* environment variable. The PG prefix >>> does not comfort me. >> >> Attached is a rebased version which fixes the test failure under autoconf (I >> had missed git adding the configure file..) and Windows where the backslashes >> weren't escaped properly. It also removes the environment variable and has >> documentation touchups. -- Daniel Gustafsson