I notice a number of places in fe-connect.c have copied this idiom
where if an option is present they validate the legal options and
otherwise they strdup a default value. This strdup of the default
option I think is being copied from sslmode's validation which is a
bit special but afaics the following still applies to it.
/*
* validate channel_binding option
*/
if (conn->channel_binding)
{
if (strcmp(conn->channel_binding, "disable") != 0
&& strcmp(conn->channel_binding, "prefer") != 0
&& strcmp(conn->channel_binding, "require") != 0)
{
conn->status = CONNECTION_BAD;
libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
"channel_binding", conn->channel_binding);
return false;
}
}
else
{
conn->channel_binding = strdup(DefaultChannelBinding);
if (!conn->channel_binding)
goto oom_error;
}
AFAICS the else branch of this is entirely dead code. These options
cannot be NULL because the default option is present in the
PQconninfoOptions array as the "compiled in default value" which
conninfo_add_defaults() will strdup in for us.
Unless..... conninfo_array_parse() is passed use_defaults=false in
which case no... But why does this parameter exist? This is a static
function with one call site and this parameter is passed as true at
that call site.
So I think this is just dead from some no longer extant code path
that's being copied by new parameters that are added.
As an aside conninfo_add_defaults doesn't put the default value there
if the option is an empty string. I think we should make it do that,
effectively forcing all options to treat empty strings as missing
options. Otherwise it's annoying to use environment variables when you
want to explicitly set a parameter to a default value since it's much
less convenient to "remove" an environment variable in a shell than
pass it as an empty string. And it would just be confusing to have
empty strings behave differently from omitted parameters.
--
greg
