diff options
author | Nirbhay Choubey <nirbhay@skysql.com> | 2014-07-22 09:27:35 -0400 |
---|---|---|
committer | Nirbhay Choubey <nirbhay@skysql.com> | 2014-07-22 09:27:35 -0400 |
commit | 7b69cab89163a8d6e1a7a5cdcb66a66366136d7c (patch) | |
tree | 697bb6bb4e7651fed33826ca71c0c51ec0e908f6 | |
parent | 0dadd017501e379546ffe3edae7728b931007be0 (diff) | |
download | mariadb-git-7b69cab89163a8d6e1a7a5cdcb66a66366136d7c.tar.gz |
MDEV-4647: Crash on setting wsep system variables to default
The variables' ON_CHECK functions relied on set_var's "value"
member, which is NULL for SET ... =default. Fixed by using
save_result instead.
Also, for many wsrep variables, pointers to their respective
global variables were used to provide default values. The patch
fixes this by using appropriate macros and string literals.
-rw-r--r-- | sql/sys_vars.cc | 28 | ||||
-rw-r--r-- | sql/wsrep_sst.cc | 75 | ||||
-rw-r--r-- | sql/wsrep_sst.h | 28 | ||||
-rw-r--r-- | sql/wsrep_var.cc | 144 | ||||
-rw-r--r-- | sql/wsrep_var.h | 4 |
5 files changed, 140 insertions, 139 deletions
diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index aba051d214d..3abfec66364 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -3691,8 +3691,7 @@ static Sys_var_tz Sys_time_zone( static Sys_var_charptr Sys_wsrep_provider( "wsrep_provider", "Path to replication provider library", PREALLOCATED GLOBAL_VAR(wsrep_provider), CMD_LINE(REQUIRED_ARG, OPT_WSREP_PROVIDER), - IN_FS_CHARSET, DEFAULT(wsrep_provider), - // IN_FS_CHARSET, DEFAULT(wsrep_provider_default), + IN_FS_CHARSET, DEFAULT(WSREP_NONE), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(wsrep_provider_check), ON_UPDATE(wsrep_provider_update)); @@ -3700,8 +3699,7 @@ static Sys_var_charptr Sys_wsrep_provider_options( "wsrep_provider_options", "provider specific options", PREALLOCATED GLOBAL_VAR(wsrep_provider_options), CMD_LINE(REQUIRED_ARG, OPT_WSREP_PROVIDER_OPTIONS), - IN_FS_CHARSET, DEFAULT(wsrep_provider_options), - NO_MUTEX_GUARD, NOT_IN_BINLOG, + IN_FS_CHARSET, DEFAULT(""), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(wsrep_provider_options_check), ON_UPDATE(wsrep_provider_options_update)); @@ -3714,7 +3712,7 @@ static Sys_var_charptr Sys_wsrep_data_home_dir( static Sys_var_charptr Sys_wsrep_cluster_name( "wsrep_cluster_name", "Name for the cluster", PREALLOCATED GLOBAL_VAR(wsrep_cluster_name), CMD_LINE(REQUIRED_ARG), - IN_FS_CHARSET, DEFAULT(wsrep_cluster_name), + IN_FS_CHARSET, DEFAULT(WSREP_CLUSTER_NAME), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(wsrep_cluster_name_check), ON_UPDATE(wsrep_cluster_name_update)); @@ -3724,7 +3722,7 @@ static Sys_var_charptr Sys_wsrep_cluster_address ( "wsrep_cluster_address", "Address to initially connect to cluster", PREALLOCATED GLOBAL_VAR(wsrep_cluster_address), CMD_LINE(REQUIRED_ARG, OPT_WSREP_CLUSTER_ADDRESS), - IN_FS_CHARSET, DEFAULT(wsrep_cluster_address), + IN_FS_CHARSET, DEFAULT(""), &PLock_wsrep_slave_threads, NOT_IN_BINLOG, ON_CHECK(wsrep_cluster_address_check), ON_UPDATE(wsrep_cluster_address_update)); @@ -3732,21 +3730,21 @@ static Sys_var_charptr Sys_wsrep_cluster_address ( static Sys_var_charptr Sys_wsrep_node_name ( "wsrep_node_name", "Node name", PREALLOCATED GLOBAL_VAR(wsrep_node_name), CMD_LINE(REQUIRED_ARG), - IN_FS_CHARSET, DEFAULT(wsrep_node_name), - NO_MUTEX_GUARD, NOT_IN_BINLOG); + IN_FS_CHARSET, DEFAULT(""), NO_MUTEX_GUARD, NOT_IN_BINLOG, + wsrep_node_name_check, wsrep_node_name_update); static Sys_var_charptr Sys_wsrep_node_address ( "wsrep_node_address", "Node address", PREALLOCATED GLOBAL_VAR(wsrep_node_address), CMD_LINE(REQUIRED_ARG), - IN_FS_CHARSET, DEFAULT(wsrep_node_address), + IN_FS_CHARSET, DEFAULT(""), NO_MUTEX_GUARD, NOT_IN_BINLOG, - ON_CHECK(wsrep_node_address_check), + ON_CHECK(wsrep_node_address_check), ON_UPDATE(wsrep_node_address_update)); static Sys_var_charptr Sys_wsrep_node_incoming_address( "wsrep_node_incoming_address", "Client connection address", PREALLOCATED GLOBAL_VAR(wsrep_node_incoming_address),CMD_LINE(REQUIRED_ARG), - IN_FS_CHARSET, DEFAULT(wsrep_node_incoming_address), + IN_FS_CHARSET, DEFAULT(WSREP_NODE_INCOMING_AUTO), NO_MUTEX_GUARD, NOT_IN_BINLOG); static Sys_var_ulong Sys_wsrep_slave_threads( @@ -3794,7 +3792,7 @@ static Sys_var_mybool Sys_wsrep_drupal_282555_workaround( static Sys_var_charptr sys_wsrep_sst_method( "wsrep_sst_method", "State snapshot transfer method", GLOBAL_VAR(wsrep_sst_method),CMD_LINE(REQUIRED_ARG), - IN_FS_CHARSET, DEFAULT(wsrep_sst_method), NO_MUTEX_GUARD, NOT_IN_BINLOG, + IN_FS_CHARSET, DEFAULT(WSREP_SST_DEFAULT), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(wsrep_sst_method_check), ON_UPDATE(wsrep_sst_method_update)); @@ -3802,7 +3800,7 @@ static Sys_var_charptr Sys_wsrep_sst_receive_address( "wsrep_sst_receive_address", "Address where node is waiting for " "SST contact", GLOBAL_VAR(wsrep_sst_receive_address),CMD_LINE(REQUIRED_ARG), - IN_FS_CHARSET, DEFAULT(wsrep_sst_receive_address), NO_MUTEX_GUARD, + IN_FS_CHARSET, DEFAULT(WSREP_SST_ADDRESS_AUTO), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(wsrep_sst_receive_address_check), ON_UPDATE(wsrep_sst_receive_address_update)); @@ -3810,7 +3808,7 @@ static Sys_var_charptr Sys_wsrep_sst_receive_address( static Sys_var_charptr Sys_wsrep_sst_auth( "wsrep_sst_auth", "Authentication for SST connection", PREALLOCATED GLOBAL_VAR(wsrep_sst_auth), CMD_LINE(REQUIRED_ARG, OPT_WSREP_SST_AUTH), - IN_FS_CHARSET, DEFAULT(wsrep_sst_auth), NO_MUTEX_GUARD, + IN_FS_CHARSET, DEFAULT(NULL), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(wsrep_sst_auth_check), ON_UPDATE(wsrep_sst_auth_update)); @@ -3839,7 +3837,7 @@ static Sys_var_charptr Sys_wsrep_start_position ( "wsrep_start_position", "global transaction position to start from ", PREALLOCATED GLOBAL_VAR(wsrep_start_position), CMD_LINE(REQUIRED_ARG, OPT_WSREP_START_POSITION), - IN_FS_CHARSET, DEFAULT(wsrep_start_position), + IN_FS_CHARSET, DEFAULT(WSREP_START_POSITION_ZERO), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(wsrep_start_position_check), ON_UPDATE(wsrep_start_position_update)); diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc index be9efa86873..d2263e3e171 100644 --- a/sql/wsrep_sst.cc +++ b/sql/wsrep_sst.cc @@ -31,32 +31,6 @@ extern const char wsrep_defaults_file[]; -#define WSREP_SST_OPT_ROLE "--role" -#define WSREP_SST_OPT_ADDR "--address" -#define WSREP_SST_OPT_AUTH "--auth" -#define WSREP_SST_OPT_DATA "--datadir" -#define WSREP_SST_OPT_CONF "--defaults-file" -#define WSREP_SST_OPT_PARENT "--parent" - -// mysqldump-specific options -#define WSREP_SST_OPT_USER "--user" -#define WSREP_SST_OPT_PSWD "--password" -#define WSREP_SST_OPT_HOST "--host" -#define WSREP_SST_OPT_PORT "--port" -#define WSREP_SST_OPT_LPORT "--local-port" - -// donor-specific -#define WSREP_SST_OPT_SOCKET "--socket" -#define WSREP_SST_OPT_GTID "--gtid" -#define WSREP_SST_OPT_BYPASS "--bypass" - -#define WSREP_SST_MYSQLDUMP "mysqldump" -#define WSREP_SST_RSYNC "rsync" -#define WSREP_SST_SKIP "skip" -#define WSREP_SST_DEFAULT WSREP_SST_RSYNC -#define WSREP_SST_ADDRESS_AUTO "AUTO" -#define WSREP_SST_AUTH_MASK "********" - const char* wsrep_sst_method = WSREP_SST_DEFAULT; const char* wsrep_sst_receive_address = WSREP_SST_ADDRESS_AUTO; const char* wsrep_sst_donor = ""; @@ -68,17 +42,16 @@ my_bool wsrep_sst_donor_rejects_queries = FALSE; bool wsrep_sst_method_check (sys_var *self, THD* thd, set_var* var) { - char buff[FN_REFLEN]; - String str(buff, sizeof(buff), system_charset_info), *res; - const char* c_str = NULL; - - if ((res = var->value->val_str(&str)) && - (c_str = res->c_ptr()) && - strlen(c_str) > 0) - return 0; - - my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), "wsrep_sst_method", c_str ? c_str : "NULL"); + if ((! var->save_result.string_value.str) || + (var->save_result.string_value.length == 0 )) + { + my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, + var->save_result.string_value.str ? + var->save_result.string_value.str : "NULL"); return 1; + } + + return 0; } bool wsrep_sst_method_update (sys_var *self, THD* thd, enum_var_type type) @@ -86,6 +59,7 @@ bool wsrep_sst_method_update (sys_var *self, THD* thd, enum_var_type type) return 0; } +// TODO: Improve address verification. static bool sst_receive_address_check (const char* str) { if (!strncasecmp(str, "127.0.0.1", strlen("127.0.0.1")) || @@ -99,15 +73,30 @@ static bool sst_receive_address_check (const char* str) bool wsrep_sst_receive_address_check (sys_var *self, THD* thd, set_var* var) { - const char* c_str = var->value->str_value.c_ptr(); + char addr_buf[FN_REFLEN]; - if (sst_receive_address_check (c_str)) - { - my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), "wsrep_sst_receive_address", c_str ? c_str : "NULL"); - return 1; - } + if ((! var->save_result.string_value.str) || + (var->save_result.string_value.length > (FN_REFLEN - 1))) // safety + { + goto err; + } - return 0; + memcpy(addr_buf, var->save_result.string_value.str, + var->save_result.string_value.length); + addr_buf[var->save_result.string_value.length]= 0; + + if (sst_receive_address_check(addr_buf)) + { + goto err; + } + + return 0; + +err: + my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, + var->save_result.string_value.str ? + var->save_result.string_value.str : "NULL"); + return 1; } bool wsrep_sst_receive_address_update (sys_var *self, THD* thd, diff --git a/sql/wsrep_sst.h b/sql/wsrep_sst.h index dc7789f5a4e..ad02aa55114 100644 --- a/sql/wsrep_sst.h +++ b/sql/wsrep_sst.h @@ -16,7 +16,33 @@ #ifndef WSREP_SST_H #define WSREP_SST_H -#include <mysql.h> // my_bool +#include <mysql.h> // my_bool + +#define WSREP_SST_OPT_ROLE "--role" +#define WSREP_SST_OPT_ADDR "--address" +#define WSREP_SST_OPT_AUTH "--auth" +#define WSREP_SST_OPT_DATA "--datadir" +#define WSREP_SST_OPT_CONF "--defaults-file" +#define WSREP_SST_OPT_PARENT "--parent" + +// mysqldump-specific options +#define WSREP_SST_OPT_USER "--user" +#define WSREP_SST_OPT_PSWD "--password" +#define WSREP_SST_OPT_HOST "--host" +#define WSREP_SST_OPT_PORT "--port" +#define WSREP_SST_OPT_LPORT "--local-port" + +// donor-specific +#define WSREP_SST_OPT_SOCKET "--socket" +#define WSREP_SST_OPT_GTID "--gtid" +#define WSREP_SST_OPT_BYPASS "--bypass" + +#define WSREP_SST_MYSQLDUMP "mysqldump" +#define WSREP_SST_RSYNC "rsync" +#define WSREP_SST_SKIP "skip" +#define WSREP_SST_DEFAULT WSREP_SST_RSYNC +#define WSREP_SST_ADDRESS_AUTO "AUTO" +#define WSREP_SST_AUTH_MASK "********" /* system variables */ extern const char* wsrep_sst_method; diff --git a/sql/wsrep_var.cc b/sql/wsrep_var.cc index 54d0b1cdfe6..a633b72bc0d 100644 --- a/sql/wsrep_var.cc +++ b/sql/wsrep_var.cc @@ -26,9 +26,6 @@ #include <cstdio> #include <cstdlib> -#define WSREP_START_POSITION_ZERO "00000000-0000-0000-0000-000000000000:-1" -#define WSREP_CLUSTER_NAME "my_wsrep_cluster" - const char* wsrep_provider = 0; const char* wsrep_provider_options = 0; const char* wsrep_cluster_address = 0; @@ -98,22 +95,22 @@ static int wsrep_start_position_verify (const char* start_str) bool wsrep_start_position_check (sys_var *self, THD* thd, set_var* var) { - char buff[FN_REFLEN]; - String str(buff, sizeof(buff), system_charset_info), *res; - const char* start_str = NULL; - - if (!(res = var->value->val_str(&str))) goto err; + char start_pos_buf[FN_REFLEN]; - start_str = res->c_ptr(); + if ((! var->save_result.string_value.str) || + (var->save_result.string_value.length > (FN_REFLEN - 1))) // safety + goto err; - if (!start_str) goto err; + memcpy(start_pos_buf, var->save_result.string_value.str, + var->save_result.string_value.length); + start_pos_buf[var->save_result.string_value.length]= 0; - if (!wsrep_start_position_verify(start_str)) return 0; + if (!wsrep_start_position_verify(start_pos_buf)) return 0; err: - - my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, - start_str ? start_str : "NULL"); + my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, + var->save_result.string_value.str ? + var->save_result.string_value.str : "NULL"); return 1; } @@ -199,22 +196,22 @@ static int wsrep_provider_verify (const char* provider_str) bool wsrep_provider_check (sys_var *self, THD* thd, set_var* var) { - char buff[FN_REFLEN]; - String str(buff, sizeof(buff), system_charset_info), *res; - const char* provider_str = NULL; - - if (!(res = var->value->val_str(&str))) goto err; + char wsrep_provider_buf[FN_REFLEN]; - provider_str = res->c_ptr(); + if ((! var->save_result.string_value.str) || + (var->save_result.string_value.length > (FN_REFLEN - 1))) // safety + goto err; - if (!provider_str) goto err; + memcpy(wsrep_provider_buf, var->save_result.string_value.str, + var->save_result.string_value.length); + wsrep_provider_buf[var->save_result.string_value.length]= 0; - if (!wsrep_provider_verify(provider_str)) return 0; + if (!wsrep_provider_verify(wsrep_provider_buf)) return 0; err: - - my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, - provider_str ? provider_str : "NULL"); + my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, + var->save_result.string_value.str ? + var->save_result.string_value.str : "NULL"); return 1; } @@ -309,21 +306,23 @@ static int wsrep_cluster_address_verify (const char* cluster_address_str) bool wsrep_cluster_address_check (sys_var *self, THD* thd, set_var* var) { - char buff[FN_REFLEN]; - String str(buff, sizeof(buff), system_charset_info), *res; - const char* cluster_address_str = NULL; + char addr_buf[FN_REFLEN]; - if (!(res = var->value->val_str(&str))) goto err; + if ((! var->save_result.string_value.str) || + (var->save_result.string_value.length > (FN_REFLEN - 1))) // safety + goto err; - cluster_address_str = res->c_ptr(); + memcpy(addr_buf, var->save_result.string_value.str, + var->save_result.string_value.length); + addr_buf[var->save_result.string_value.length]= 0; - if (!wsrep_cluster_address_verify(cluster_address_str)) return 0; + if (!wsrep_cluster_address_verify(addr_buf)) return 0; err: - - my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, - cluster_address_str ? cluster_address_str : "NULL"); - return 1 ; + my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, + var->save_result.string_value.str ? + var->save_result.string_value.str : "NULL"); + return 1; } bool wsrep_cluster_address_update (sys_var *self, THD* thd, enum_var_type type) @@ -363,25 +362,18 @@ void wsrep_cluster_address_init (const char* value) wsrep_cluster_address = (value) ? my_strdup(value, MYF(0)) : NULL; } +/* wsrep_cluster_name cannot be NULL or an empty string. */ bool wsrep_cluster_name_check (sys_var *self, THD* thd, set_var* var) { - char buff[FN_REFLEN]; - String str(buff, sizeof(buff), system_charset_info), *res; - const char* cluster_name_str = NULL; - - if (!(res = var->value->val_str(&str))) goto err; - - cluster_name_str = res->c_ptr(); - - if (!cluster_name_str || strlen(cluster_name_str) == 0) goto err; - + if (!var->save_result.string_value.str || + (var->save_result.string_value.length == 0)) + { + my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, + (var->save_result.string_value.str ? + var->save_result.string_value.str : "NULL")); + return 1; + } return 0; - - err: - - my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, - cluster_name_str ? cluster_name_str : "NULL"); - return 1; } bool wsrep_cluster_name_update (sys_var *self, THD* thd, enum_var_type type) @@ -391,23 +383,15 @@ bool wsrep_cluster_name_update (sys_var *self, THD* thd, enum_var_type type) bool wsrep_node_name_check (sys_var *self, THD* thd, set_var* var) { - char buff[FN_REFLEN]; - String str(buff, sizeof(buff), system_charset_info), *res; - const char* node_name_str = NULL; - - if (!(res = var->value->val_str(&str))) goto err; - - node_name_str = res->c_ptr(); - - if (!node_name_str || strlen(node_name_str) == 0) goto err; - + // TODO: for now 'allow' 0-length string to be valid (default) + if (!var->save_result.string_value.str) + { + my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, + (var->save_result.string_value.str ? + var->save_result.string_value.str : "NULL")); + return 1; + } return 0; - - err: - - my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, - node_name_str ? node_name_str : "NULL"); - return 1; } bool wsrep_node_name_update (sys_var *self, THD* thd, enum_var_type type) @@ -418,22 +402,23 @@ bool wsrep_node_name_update (sys_var *self, THD* thd, enum_var_type type) // TODO: do something more elaborate, like checking connectivity bool wsrep_node_address_check (sys_var *self, THD* thd, set_var* var) { - char buff[FN_REFLEN]; - String str(buff, sizeof(buff), system_charset_info), *res; - const char* node_address_str = NULL; + char addr_buf[FN_REFLEN]; - if (!(res = var->value->val_str(&str))) goto err; + if ((! var->save_result.string_value.str) || + (var->save_result.string_value.length > (FN_REFLEN - 1))) // safety + goto err; - node_address_str = res->c_ptr(); - - if (!node_address_str || strlen(node_address_str) == 0) goto err; + memcpy(addr_buf, var->save_result.string_value.str, + var->save_result.string_value.length); + addr_buf[var->save_result.string_value.length]= 0; + // TODO: for now 'allow' 0-length string to be valid (default) return 0; - err: - +err: my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str, - node_address_str ? node_address_str : "NULL"); + var->save_result.string_value.str ? + var->save_result.string_value.str : "NULL"); return 1; } @@ -453,7 +438,8 @@ void wsrep_node_address_init (const char* value) bool wsrep_slave_threads_check (sys_var *self, THD* thd, set_var* var) { mysql_mutex_lock(&LOCK_wsrep_slave_threads); - wsrep_slave_count_change += (var->value->val_int() - wsrep_slave_threads); + wsrep_slave_count_change += (var->save_result.ulonglong_value - + wsrep_slave_threads); mysql_mutex_unlock(&LOCK_wsrep_slave_threads); return 0; @@ -471,7 +457,7 @@ bool wsrep_slave_threads_update (sys_var *self, THD* thd, enum_var_type type) bool wsrep_desync_check (sys_var *self, THD* thd, set_var* var) { - bool new_wsrep_desync = var->value->val_bool(); + bool new_wsrep_desync= (bool) var->save_result.ulonglong_value; if (wsrep_desync == new_wsrep_desync) { if (new_wsrep_desync) { push_warning (thd, MYSQL_ERROR::WARN_LEVEL_WARN, diff --git a/sql/wsrep_var.h b/sql/wsrep_var.h index b69f670a14b..2a5e94b6724 100644 --- a/sql/wsrep_var.h +++ b/sql/wsrep_var.h @@ -16,7 +16,9 @@ #ifndef WSREP_VAR_H #define WSREP_VAR_H -#define WSREP_NODE_INCOMING_AUTO "AUTO" +#define WSREP_CLUSTER_NAME "my_wsrep_cluster" +#define WSREP_NODE_INCOMING_AUTO "AUTO" +#define WSREP_START_POSITION_ZERO "00000000-0000-0000-0000-000000000000:-1" // MySQL variables funcs |