maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Sergey!

On Jan 30, svoj@xxxxxxxxxxx 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