On Fri, Oct 29, 2010 at 19:56, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Magnus Hagander <mag...@hagander.net> writes:
>> On Fri, Oct 29, 2010 at 10:01, Josh Berkus <j...@agliodbs.com> wrote:
>>> Mind you, we *also* need a doc patch. ("This parameter is only available if
>>> PostgreSQL was built with SSL support").
>
>> Actually, I think it'd be more user friendly to make the parameter
>> still exist and be a no-op (maybe show the value NULL?) on non-SSL
>> enabled builds. Same goes for all other parameters that depend on a
>> specific compile flag. That would make life easier on automated tools
>> *and* on people sitting down at a new installation.
>
> Yeah, we're a bit schizophrenic about this. Some parameters still
> exist, but can't be set to any value but "disabled", when the relevant
> feature wasn't compiled. Others just aren't there.
>
> It takes code space and work to make a parameter be present but behave
> differently if disabled. I don't think that we should institute a
> blanket policy of requiring that to happen; in particular, there are a
> number of debug-type parameters for which I argue that it's not worth
> the trouble. But for user-facing parameters I agree we should do it,
> and ssl_ciphers is one of those.
Here's a patch that does this for HEAD. AFAICT, only ssl_ciphers and
the syslog parameters are actually "user facing parameters" - all the
other ifdef'ed out ones are debug-style parameters.
> In any case, a doc patch would be the right thing for the back branches.
I can look at this too (yes, I know we just wrapped, but I'm working
down the backlog :S). You mean something as simple as "this parameter
is unavailable if the server was not compiled with support for SSL"?
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
*** a/src/backend/libpq/be-secure.c
--- b/src/backend/libpq/be-secure.c
***************
*** 103,112 **** int ssl_renegotiation_limit;
#ifdef USE_SSL
static SSL_CTX *SSL_context = NULL;
static bool ssl_loaded_verify_locations = false;
/* GUC variable controlling SSL cipher list */
char *SSLCipherSuites = NULL;
- #endif
/* ------------------------------------------------------------ */
/* Hardcoded values */
--- 103,112 ----
#ifdef USE_SSL
static SSL_CTX *SSL_context = NULL;
static bool ssl_loaded_verify_locations = false;
+ #endif
/* GUC variable controlling SSL cipher list */
char *SSLCipherSuites = NULL;
/* ------------------------------------------------------------ */
/* Hardcoded values */
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 128,133 **** extern char *temp_tablespaces;
--- 128,134 ----
extern bool synchronize_seqscans;
extern bool fullPageWrites;
extern int ssl_renegotiation_limit;
+ extern char *SSLCipherSuites;
#ifdef TRACE_SORT
extern bool trace_sort;
***************
*** 139,148 **** extern bool trace_syncscan;
extern bool optimize_bounded_sort;
#endif
- #ifdef USE_SSL
- extern char *SSLCipherSuites;
- #endif
-
static void set_config_sourcefile(const char *name, char *sourcefile,
int sourceline);
--- 140,145 ----
***************
*** 151,162 **** static const char *assign_log_destination(const char *value,
#ifdef HAVE_SYSLOG
static int syslog_facility = LOG_LOCAL0;
static bool assign_syslog_facility(int newval,
bool doit, GucSource source);
static const char *assign_syslog_ident(const char *ident,
bool doit, GucSource source);
- #endif
static bool assign_session_replication_role(int newval, bool doit,
GucSource source);
--- 148,161 ----
#ifdef HAVE_SYSLOG
static int syslog_facility = LOG_LOCAL0;
+ #else
+ static int syslog_facility = 0;
+ #endif
static bool assign_syslog_facility(int newval,
bool doit, GucSource source);
static const char *assign_syslog_ident(const char *ident,
bool doit, GucSource source);
static bool assign_session_replication_role(int newval, bool doit,
GucSource source);
***************
*** 280,287 **** static const struct config_enum_entry session_replication_role_options[] = {
{NULL, 0, false}
};
- #ifdef HAVE_SYSLOG
static const struct config_enum_entry syslog_facility_options[] = {
{"local0", LOG_LOCAL0, false},
{"local1", LOG_LOCAL1, false},
{"local2", LOG_LOCAL2, false},
--- 279,286 ----
{NULL, 0, false}
};
static const struct config_enum_entry syslog_facility_options[] = {
+ #ifdef HAVE_SYSLOG
{"local0", LOG_LOCAL0, false},
{"local1", LOG_LOCAL1, false},
{"local2", LOG_LOCAL2, false},
***************
*** 290,298 **** static const struct config_enum_entry syslog_facility_options[] = {
{"local5", LOG_LOCAL5, false},
{"local6", LOG_LOCAL6, false},
{"local7", LOG_LOCAL7, false},
{NULL, 0}
};
- #endif
static const struct config_enum_entry track_function_options[] = {
{"none", TRACK_FUNC_OFF, false},
--- 289,299 ----
{"local5", LOG_LOCAL5, false},
{"local6", LOG_LOCAL6, false},
{"local7", LOG_LOCAL7, false},
+ #else
+ {"none", 0, false},
+ #endif
{NULL, 0}
};
static const struct config_enum_entry track_function_options[] = {
{"none", TRACK_FUNC_OFF, false},
***************
*** 410,418 **** int tcp_keepalives_count;
*/
static char *log_destination_string;
- #ifdef HAVE_SYSLOG
static char *syslog_ident_str;
- #endif
static bool phony_autocommit;
static bool session_auth_is_superuser;
static double phony_random_seed;
--- 411,417 ----
***************
*** 2531,2537 **** static struct config_string ConfigureNamesString[] =
"postgresql-%Y-%m-%d_%H%M%S.log", NULL, NULL
},
- #ifdef HAVE_SYSLOG
{
{"syslog_ident", PGC_SIGHUP, LOGGING_WHERE,
gettext_noop("Sets the program name used to identify PostgreSQL "
--- 2530,2535 ----
***************
*** 2541,2547 **** static struct config_string ConfigureNamesString[] =
&syslog_ident_str,
"postgres", assign_syslog_ident, NULL
},
- #endif
{
{"TimeZone", PGC_USERSET, CLIENT_CONN_LOCALE,
--- 2539,2544 ----
***************
*** 2680,2686 **** static struct config_string ConfigureNamesString[] =
"pg_catalog.simple", assignTSCurrentConfig, NULL
},
- #ifdef USE_SSL
{
{"ssl_ciphers", PGC_POSTMASTER, CONN_AUTH_SECURITY,
gettext_noop("Sets the list of allowed SSL ciphers."),
--- 2677,2682 ----
***************
*** 2688,2696 **** static struct config_string ConfigureNamesString[] =
GUC_SUPERUSER_ONLY
},
&SSLCipherSuites,
! "ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH", NULL, NULL
},
- #endif /* USE_SSL */
{
{"application_name", PGC_USERSET, LOGGING_WHAT,
--- 2684,2696 ----
GUC_SUPERUSER_ONLY
},
&SSLCipherSuites,
! #ifdef USE_SSL
! "ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH",
! #else
! "none",
! #endif
! NULL, NULL
},
{
{"application_name", PGC_USERSET, LOGGING_WHAT,
***************
*** 2807,2822 **** static struct config_enum ConfigureNamesEnum[] =
LOGSTMT_NONE, log_statement_options, NULL, NULL
},
- #ifdef HAVE_SYSLOG
{
{"syslog_facility", PGC_SIGHUP, LOGGING_WHERE,
gettext_noop("Sets the syslog \"facility\" to be used when syslog enabled."),
NULL
},
&syslog_facility,
! LOG_LOCAL0, syslog_facility_options, assign_syslog_facility, NULL
! },
#endif
{
{"session_replication_role", PGC_SUSET, CLIENT_CONN_STATEMENT,
--- 2807,2825 ----
LOGSTMT_NONE, log_statement_options, NULL, NULL
},
{
{"syslog_facility", PGC_SIGHUP, LOGGING_WHERE,
gettext_noop("Sets the syslog \"facility\" to be used when syslog enabled."),
NULL
},
&syslog_facility,
! #ifdef HAVE_SYSLOG
! LOG_LOCAL0,
! #else
! 0,
#endif
+ syslog_facility_options, assign_syslog_facility, NULL
+ },
{
{"session_replication_role", PGC_SUSET, CLIENT_CONN_STATEMENT,
***************
*** 7637,7650 **** assign_log_destination(const char *value, bool doit, GucSource source)
return value;
}
- #ifdef HAVE_SYSLOG
-
static bool
assign_syslog_facility(int newval, bool doit, GucSource source)
{
if (doit)
set_syslog_parameters(syslog_ident_str ? syslog_ident_str : "postgres",
newval);
return true;
}
--- 7640,7654 ----
return value;
}
static bool
assign_syslog_facility(int newval, bool doit, GucSource source)
{
+ #ifdef HAVE_SYSLOG
if (doit)
set_syslog_parameters(syslog_ident_str ? syslog_ident_str : "postgres",
newval);
+ #endif
+ /* Without syslog support, just ignore it */
return true;
}
***************
*** 7652,7663 **** assign_syslog_facility(int newval, bool doit, GucSource source)
static const char *
assign_syslog_ident(const char *ident, bool doit, GucSource source)
{
if (doit)
set_syslog_parameters(ident, syslog_facility);
return ident;
}
- #endif /* HAVE_SYSLOG */
static bool
--- 7656,7669 ----
static const char *
assign_syslog_ident(const char *ident, bool doit, GucSource source)
{
+ #ifdef HAVE_SYSLOG
if (doit)
set_syslog_parameters(ident, syslog_facility);
+ #endif
+ /* Without syslog support, it will always be set to "none", so ignore */
return ident;
}
static bool
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
