Re: bk commit into 5.1 tree (acurtis:1.2655) BUG#33358

[Sergei Golubchik]

Hi!

On Jan 21, antony@stripped wrote:
> ChangeSet@stripped, 2008-01-21 12:01:21-08:00, acurtis@stripped +4 -0
>   Bug#33358
>     "Plugin enum variables can't be set from command line"
>     fix crash of LOCK_plugins mutex when loading plug-ins from command line.
>     fix off-by-one bug when loading multiple plug-ins from the command line.
>     initialize command line handling for ENUM and SET plugin variable types.

> diff -Nrup a/mysql-test/r/plugin_bug33358.result
> b/mysql-test/r/plugin_bug33358.result
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/r/plugin_bug33358.result	2008-01-21 12:01:15 -08:00
> @@ -0,0 +1,3 @@
> +SELECT @@global.example_enum_var = 'e2';
> +@@global.example_enum_var = 'e2'
> +1
> diff -Nrup a/mysql-test/t/plugin_bug33358-master.opt
> b/mysql-test/t/plugin_bug33358-master.opt
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/t/plugin_bug33358-master.opt	2008-01-21 12:01:15 -08:00
> @@ -0,0 +1,3 @@
> +$EXAMPLE_PLUGIN_OPT
> +--plugin-load=EXAMPLE=ha_example.so::
> +--plugin-example-enum-var=e2
> diff -Nrup a/mysql-test/t/plugin_bug33358.test b/mysql-test/t/plugin_bug33358.test
> --- /dev/null	Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/t/plugin_bug33358.test	2008-01-21 12:01:15 -08:00
> @@ -0,0 +1,3 @@
> +--source include/have_example_plugin.inc
> +
> +SELECT @@global.example_enum_var = 'e2';

perhaps you could add this test to an existing test file that already
requires ha_example.so ? One restart less for the test suite...

> diff -Nrup a/sql/sql_plugin.cc b/sql/sql_plugin.cc
> --- a/sql/sql_plugin.cc	2007-12-13 03:49:55 -08:00
> +++ b/sql/sql_plugin.cc	2008-01-21 12:01:15 -08:00
> @@ -1412,7 +1412,11 @@ static bool plugin_load_list(MEM_ROOT *t
>    while (list)
>    {
>      if (p == buffer + sizeof(buffer) - 1)
> -      break;
> +	{

use spaces, not tabs

> @@ -1464,6 +1475,7 @@ static bool plugin_load_list(MEM_ROOT *t
>    }
>    DBUG_RETURN(FALSE);
>  error:
> +  pthread_mutex_unlock(&LOCK_plugin);

uhm, I don't know. better to unlock in the mutex in the same case: of
switch() where you locked it. I agree that currently this case: is the
only place where you do 'goto error', but it's fragile to rely on that.

please remove this unlock and add it before every goto error.

>    sql_print_error("Couldn't load plugin named '%s' with soname '%s'.",
>                    name.str, dl.str);
>    DBUG_RETURN(TRUE);
> @@ -3017,7 +3030,22 @@ static int construct_options(MEM_ROOT *m
>        optnamelen= namelen + optnamelen + 1;
>      }
>      else
> -      optname= (char*) memdup_root(mem_root, v->key + 1, (optnamelen=
> v->name_len) + 1);
> +    {
> +      if (!(v= find_bookmark(name, opt->name, opt->flags)))
> +      {
> +        sql_print_error("Thread local variable '%s' not allocated "
> +                        "in plugin '%s'.", opt->name, plugin_name);

why could that happen ?

> +    	DBUG_RETURN(-1);
> +      }
> +
> +      *(int*)(opt + 1)= offset= v->offset;
> +
> +      if (opt->flags & PLUGIN_VAR_NOCMDOPT)
> +        continue;
> +
> +      optname= (char*) memdup_root(mem_root, v->key + 1, 
> +                                   (optnamelen= v->name_len) + 1);
> +    }
>  
>      /* convert '_' to '-' */
>      for (p= optname; *p; p++)
 
Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Developer
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Dachauer Str. 37, D-80335 München
       <___/                  Geschäftsführer: Kaj Arnö - HRB
München 162140