Hi Zsolt,
Thanks for reporting the bug and the patch. If an operator changes the
hosts_file GUC, without the patch those three messages don't get updated
and it makes debugging a lot harder. The fix is clearly right.
A couple of observations:
1. Small typo in the commit message "harcoded" → "hardcoded".
2. While reviewing I noticed a related NULL-safety question. This isn't
caused by your patch, I was just digging to see whether it could turn into
a crash or segfault after the change.
guc_parameters.dat declares hosts_file with boot_val => 'NULL', so
HostsFileName can be NULL if the GUC is never assigned. After the patch,
the three messages pass HostsFileName straight to errmsg("... \"%s\" ...",
HostsFileName), and passing NULL to %s is technically undefined behaviour.
I traced the code path and ran a small reproducer, if HostsFileName is NULL
and ssl_sni is on, execution flows into load_hosts() ->
open_auth_file(NULL, ...) -> fopen(NULL, "r"), which on macOS returns NULL
with errno=EFAULT (glibc behaves the same way).
The subsequent errmsg(..., "%s", NULL) prints (null) rather than crashing.
So the observable difference from this patch in the NULL corner case is:
before the patch we would see: 'could not load "pg_hosts.conf": ...'
after the patch we would see: 'could not load "(null)": ...'
Both are misleading but neither is fatal. Also the corner case itself is
hard to reach with configuration alone as SelectConfigFiles() in guc.c
fills in configdir/pg_hosts.conf if hosts_file is unset.
Regards,
Surya Poondla
