Re: [Maria-developers] [Commits] 11deb41: MDEV-7499 - System variables have broken default values on big endian
Hi, Sergey! On Jan 30, svoj@mariadb.org wrote:
revision-id: 11deb416fe12ebfd65a1f089c7eb6087c192e2dd parent(s): edf34f38ac4fad7996bf19cd9ac669d2a6825400 committer: Sergey Vojtovich branch nick: 10.1 timestamp: 2015-01-30 16:13:28 +0400 message:
MDEV-7499 - System variables have broken default values on big endian
INFORMATION_SCHEMA.SYSTEM_VARIABLES.DEFAULT_VALUE had broken values on big endian.
Default value is internally stored as longlong, while I_S references it's pointer (longlong *) according to variable type (e.g. int, my_bool, etc). This works well on little endian, but on big endian we always get 0 for such variables.
Ok to push, thanks! One suggestion below
diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index 46849e2..1928df5 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -3254,7 +3254,29 @@ static int pluginvar_sysvar_flags(const st_mysql_sys_var *p) uchar* sys_var_pluginvar::real_value_ptr(THD *thd, enum_var_type type) { if (type == OPT_DEFAULT) - return (uchar*)&option.def_value; + { + switch (plugin_var->flags & PLUGIN_VAR_TYPEMASK) { + case PLUGIN_VAR_BOOL: + thd->sys_var_tmp.my_bool_value= option.def_value; + return (uchar*) &thd->sys_var_tmp.my_bool_value; + case PLUGIN_VAR_INT: + thd->sys_var_tmp.int_value= option.def_value; + return (uchar*) &thd->sys_var_tmp.int_value; + case PLUGIN_VAR_LONG: + case PLUGIN_VAR_ENUM: + thd->sys_var_tmp.long_value= option.def_value; + return (uchar*) &thd->sys_var_tmp.long_value; + case PLUGIN_VAR_LONGLONG: + case PLUGIN_VAR_SET: + case PLUGIN_VAR_DOUBLE: + return (uchar*) &option.def_value;
PLUGIN_VAR_LONGLONG and PLUGIN_VAR_SET - ok, but for PLUGIN_VAR_DOUBLE I'd either copy it to thd->sys_var_tmp too or add an assert, like this case PLUGIN_VAR_DOUBLE: compile_time_assert(sizeof(double) == sizeof(option.def_value)); /* fall through */ case PLUGIN_VAR_LONGLONG: case PLUGIN_VAR_SET: return (uchar*) &option.def_value; More for documentation purposes than for protection agains a wrong sizeof(double). It explains why you don't copy the value.
+ case PLUGIN_VAR_STR: + thd->sys_var_tmp.ptr_value= (void*) option.def_value; + return (uchar*) &thd->sys_var_tmp.ptr_value; + default: + DBUG_ASSERT(0); + } + }
Regards, Sergei
Hi Sergei, thanks for your review. On Wed, Feb 04, 2015 at 12:38:04PM +0100, Sergei Golubchik wrote:
MDEV-7499 - System variables have broken default values on big endian ...skip...
+ case PLUGIN_VAR_LONGLONG: + case PLUGIN_VAR_SET: + case PLUGIN_VAR_DOUBLE: + return (uchar*) &option.def_value;
PLUGIN_VAR_LONGLONG and PLUGIN_VAR_SET - ok, but for PLUGIN_VAR_DOUBLE I'd either copy it to thd->sys_var_tmp too or add an assert, like this
case PLUGIN_VAR_DOUBLE: compile_time_assert(sizeof(double) == sizeof(option.def_value)); /* fall through */ case PLUGIN_VAR_LONGLONG: case PLUGIN_VAR_SET: return (uchar*) &option.def_value;
More for documentation purposes than for protection agains a wrong sizeof(double). It explains why you don't copy the value. I did copy to thd->sys_var_tmp initially, but it returned scrambled value. But now I see that there is getopt_ulonglong2double(), which may do the trick. I'll try it. If it won't work I'll add an assertion.
Thanks, Sergey
participants (2)
-
Sergei Golubchik
-
Sergey Vojtovich