wl#2936 - fixing problems

include/mysql/plugin.h:
  warning. no int/realsize
mysys/my_getopt.c:
  combination of prefixes (e.g. --loose-skip-) didn't work
sql/event_queue.cc:
  warning
sql/handler.cc:
  assert
sql/handler.h:
  comment
sql/item_create.cc:
  warnings
sql/log.cc:
  warnings
sql/mysqld.cc:
  wl#2936 - fixing problems (memory leak in load_defaults,
  garbage in opt_[slow_]logname
sql/partition_element.h:
  warnings
sql/rpl_utility.h:
  warnings
sql/set_var.cc:
  comment
sql/set_var.h:
  warnings
sql/sql_class.cc:
  warnings
sql/sql_parse.cc:
  warnings
sql/sql_plugin.cc:
  wl#2936 - fixing problems: portability, coding style,
  reporting of spurious errors, crashes, incorrect help output
storage/example/ha_example.cc:
  comment
storage/innobase/handler/ha_innodb.cc:
  wl#2936 - fixing problems (crash)

include/mysql/plugin.h

@@ -632,8 +632,8 @@ struct st_mysql_value
 {
   int (*value_type)(struct st_mysql_value *);
   const char *(*val_str)(struct st_mysql_value *, char *buffer, int *length);
-  int (*val_real)(struct st_mysql_value *, void *realbuf, int realsize);
-  int (*val_int)(struct st_mysql_value *, void *intbuf, int intsize);
+  int (*val_real)(struct st_mysql_value *, double *realbuf);
+  int (*val_int)(struct st_mysql_value *, long long *intbuf);
 };
 
 
@@ -651,8 +651,8 @@ long long thd_test_options(const MYSQL_THD thd, long long test_options);
 int thd_sql_command(const MYSQL_THD thd);
 const char *thd_proc_info(MYSQL_THD thd, const char *info);
 void **thd_ha_data(const MYSQL_THD thd, const struct handlerton *hton);
-char *thd_security_context(MYSQL_THD thd, char *buffer, int length,
-                           int max_query_len);
+char *thd_security_context(MYSQL_THD thd, char *buffer, unsigned int length,
+                           unsigned int max_query_len);
 
 
 #ifdef __cplusplus

mysql-test/mysql-test-run.pl

@@ -2930,8 +2930,8 @@ sub install_db ($$) {
   mtr_add_arg($args, "--skip-grant-tables");
   mtr_add_arg($args, "--basedir=%s", $path_my_basedir);
   mtr_add_arg($args, "--datadir=%s", $data_dir);
-  mtr_add_arg($args, "--skip-innodb");
-  mtr_add_arg($args, "--skip-ndbcluster");
+  mtr_add_arg($args, "--loose-skip-innodb");
+  mtr_add_arg($args, "--loose-skip-ndbcluster");
   mtr_add_arg($args, "--tmpdir=.");
   mtr_add_arg($args, "--core-file");
 
@@ -3037,8 +3037,8 @@ character-sets-dir  = $path_charsetsdir
 basedir             = $path_my_basedir
 server_id           = $server_id
 skip-stack-trace
-skip-innodb
-skip-ndbcluster
+loose-skip-innodb
+loose-skip-ndbcluster
 EOF
 ;
     if ( $mysql_version_id < 50100 )
@@ -3676,7 +3676,7 @@ sub mysqld_arguments ($$$$$) {
     if ( $opt_skip_ndbcluster ||
 	 !$cluster->{'pid'})
     {
-      mtr_add_arg($args, "%s--skip-ndbcluster", $prefix);
+      mtr_add_arg($args, "%s--loose-skip-ndbcluster", $prefix);
     }
     else
     {
@@ -3731,7 +3731,7 @@ sub mysqld_arguments ($$$$$) {
                 $slave->[$idx]->{'port'});
     mtr_add_arg($args, "%s--report-user=root", $prefix);
     mtr_add_arg($args, "%s--skip-innodb", $prefix);
-    mtr_add_arg($args, "%s--skip-ndbcluster", $prefix);
+    mtr_add_arg($args, "%s--loose-skip-ndbcluster", $prefix);
     mtr_add_arg($args, "%s--skip-slave-start", $prefix);
 
     # Directory where slaves find the dumps generated by "load data"
@@ -3767,7 +3767,7 @@ sub mysqld_arguments ($$$$$) {
          $slave->[$idx]->{'cluster'} == -1 ||
 	 !$clusters->[$slave->[$idx]->{'cluster'}]->{'pid'} )
     {
-      mtr_add_arg($args, "%s--skip-ndbcluster", $prefix);
+      mtr_add_arg($args, "%s--loose-skip-ndbcluster", $prefix);
     }
     else
     {

mysql-test/r/flush2.result

@@ -5,6 +5,7 @@ Variable_name	Value
 log	ON
 log_bin	OFF
 log_bin_trust_function_creators	ON
+log_bin_trust_routine_creators	ON
 log_error	
 log_output	TABLE
 log_queries_not_using_indexes	OFF
@@ -17,6 +18,7 @@ Variable_name	Value
 log	ON
 log_bin	OFF
 log_bin_trust_function_creators	ON
+log_bin_trust_routine_creators	ON
 log_error	
 log_output	TABLE
 log_queries_not_using_indexes	OFF

mysql-test/r/im_utils.result

@@ -20,8 +20,8 @@ character-sets-dir	VALUE
 basedir	VALUE
 server_id	VALUE
 skip-stack-trace	VALUE
-skip-plugin-innodb	VALUE
-skip-plugin-ndbcluster	VALUE
+loose-skip-innodb	VALUE
+loose-skip-ndbcluster	VALUE
 log-output	VALUE
 SHOW INSTANCE OPTIONS mysqld2;
 option_name	value
@@ -38,8 +38,8 @@ character-sets-dir	VALUE
 basedir	VALUE
 server_id	VALUE
 skip-stack-trace	VALUE
-skip-plugin-innodb	VALUE
-skip-plugin-ndbcluster	VALUE
+loose-skip-innodb	VALUE
+loose-skip-ndbcluster	VALUE
 nonguarded	VALUE
 log-output	VALUE
 START INSTANCE mysqld2;

mysys/my_getopt.c

@@ -97,18 +97,18 @@ void my_getopt_register_get_addr(gptr* (*func_addr)(const char *, uint,
   getopt_get_addr= func_addr;
 }
 
-int handle_options(int *argc, char ***argv, 
+int handle_options(int *argc, char ***argv,
 		   const struct my_option *longopts,
                    my_get_one_option get_one_option)
 {
-  uint opt_found, argvpos= 0, length, i;
+  uint opt_found, argvpos= 0, length;
   my_bool end_of_options= 0, must_be_var, set_maximum_value,
           option_is_loose;
   char **pos, **pos_end, *optend, *prev_found,
        *opt_str, key_name[FN_REFLEN];
   const struct my_option *optp;
   gptr *value;
-  int error;
+  int error, i;
 
   LINT_INIT(opt_found);
   (*argc)--; /* Skip the program name */
@@ -224,12 +224,11 @@ int handle_options(int *argc, char ***argv,
 		/*
 		  We were called with a special prefix, we can reuse opt_found
 		*/
-		opt_str+= (special_opt_prefix_lengths[i] + 1);
+		opt_str+= special_opt_prefix_lengths[i] + 1;
+                length-= special_opt_prefix_lengths[i] + 1;
 		if (i == OPT_LOOSE)
 		  option_is_loose= 1;
-		if ((opt_found= findopt(opt_str, length -
-					(special_opt_prefix_lengths[i] + 1),
-					&optp, &prev_found)))
+		if ((opt_found= findopt(opt_str, length, &optp, &prev_found)))
 		{
 		  if (opt_found > 1)
 		  {
@@ -253,7 +252,7 @@ int handle_options(int *argc, char ***argv,
 		    break;
 		  case OPT_ENABLE:
 		    optend= (optend && *optend == '0' && !(*(optend + 1))) ?
- 		      disabled_my_option : (char*) "1";
+                      disabled_my_option : (char*) "1";
 		    break;
 		  case OPT_MAXIMUM:
 		    set_maximum_value= 1;
@@ -262,6 +261,7 @@ int handle_options(int *argc, char ***argv,
 		  }
 		  break; /* break from the inner loop, main loop continues */
 		}
+                i= -1; /* restart the loop */
 	      }
 	    }
 	  }

sql/event_queue.cc

@@ -79,7 +79,7 @@ event_queue_element_compare_q(void *vptr, byte* a, byte *b)
 */
 
 Event_queue::Event_queue()
-  :mutex_last_unlocked_at_line(0), mutex_last_locked_at_line(0),
+  :mutex_last_locked_at_line(0), mutex_last_unlocked_at_line(0),
    mutex_last_attempted_lock_at_line(0),
    mutex_queue_data_locked(FALSE), mutex_queue_data_attempting_lock(FALSE)
 {

sql/handler.cc

@@ -1508,6 +1508,7 @@ void **handler::ha_data(void) const
 
 THD *handler::ha_thd(void) const
 {
+  DBUG_ASSERT(!table || !table->in_use || table->in_use == current_thd);
   return (table && table->in_use) ? table->in_use : current_thd;
 }
 

sql/handler.h

@@ -1516,6 +1516,10 @@ public:
 
   /* lock_count() can be more than one if the table is a MERGE */
   virtual uint lock_count(void) const { return 1; }
+  /*
+    NOTE that one can NOT rely on table->in_use in store_lock().  It may
+    refer to a different thread if called from mysql_lock_abort_for_thread().
+  */
   virtual THR_LOCK_DATA **store_lock(THD *thd,
 				     THR_LOCK_DATA **to,
 				     enum thr_lock_type lock_type)=0;

sql/log.cc

@@ -1914,8 +1914,8 @@ shutdown the MySQL server and restart it.", name, errno);
 }
 
 MYSQL_LOG::MYSQL_LOG()
-  : name(0),  log_type(LOG_UNKNOWN), log_state(LOG_CLOSED), write_error(FALSE),
-    inited(FALSE)
+  : name(0), write_error(FALSE), inited(FALSE), log_type(LOG_UNKNOWN),
+    log_state(LOG_CLOSED)
 {
   /*
     We don't want to initialize LOCK_Log here as such initialization depends on

sql/mysqld.cc

@@ -2651,7 +2651,7 @@ static bool init_global_datetime_format(timestamp_type format_type,
 static int init_common_variables(const char *conf_file_name, int argc,
 				 char **argv, const char **groups)
 {
-  char buff[FN_REFLEN];
+  char buff[FN_REFLEN], *s;
   umask(((~my_umask) & 0666));
   my_decimal_set_zero(&decimal_zero); // set decimal_zero constant;
   tzset();			// Set tzname
@@ -2857,15 +2857,13 @@ static int init_common_variables(const char *conf_file_name, int argc,
                       "--log-slow-queries option, log tables are used. "
                       "To enable logging to files use the --log-output option.");
 
-  if (!opt_logname)
-    opt_logname= make_default_log_name(buff, ".log");
-  sys_var_general_log_path.value= my_strdup(opt_logname, MYF(0));
-  sys_var_general_log_path.value_length= strlen(opt_logname);
+  s= opt_logname ? opt_logname : make_default_log_name(buff, ".log");
+  sys_var_general_log_path.value= my_strdup(s, MYF(0));
+  sys_var_general_log_path.value_length= strlen(s);
 
-  if (!opt_slow_logname)
-    opt_slow_logname= make_default_log_name(buff, "-slow.log");
-  sys_var_slow_log_path.value= my_strdup(opt_slow_logname, MYF(0));
-  sys_var_slow_log_path.value_length= strlen(opt_slow_logname);
+  s= opt_slow_logname ? opt_slow_logname : make_default_log_name(buff, "-slow.log");
+  sys_var_slow_log_path.value= my_strdup(s, MYF(0));
+  sys_var_slow_log_path.value_length= strlen(s);
 
   if (use_temp_pool && bitmap_init(&temp_pool,0,1024,1))
     return 1;
@@ -3280,7 +3278,7 @@ server.");
     using_update_log=1;
   }
 
-  if (plugin_init(&defaults_argc, defaults_argv, 
+  if (plugin_init(&defaults_argc, defaults_argv,
                   (opt_noacl ? PLUGIN_INIT_SKIP_PLUGIN_TABLE : 0) |
                   (opt_help ? PLUGIN_INIT_SKIP_INITIALIZATION : 0)))
   {
@@ -3295,25 +3293,28 @@ server.");
   if (defaults_argc > 1)
   {
     int ho_error;
-    struct my_option no_opts[] =
+    char **tmp_argv= defaults_argv;
+    struct my_option no_opts[]=
     {
       {0, 0, 0, 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}
     };
     /*
       We need to eat any 'loose' arguments first before we conclude
-      that there are unprocessed options
+      that there are unprocessed options.
+      But we need to preserve defaults_argv pointer intact for
+      free_defaults() to work. Thus we use a copy here.
     */
     my_getopt_skip_unknown= 0;
-    
-    if ((ho_error= handle_options(&defaults_argc, &defaults_argv, no_opts,
+
+    if ((ho_error= handle_options(&defaults_argc, &tmp_argv, no_opts,
                                   get_one_option)))
       unireg_abort(ho_error);
-    
+
     if (defaults_argc)
     {
       fprintf(stderr, "%s: Too many arguments (first extra is '%s').\n"
-              "Use --verbose --help to get a list of available options\n", 
-              my_progname, *defaults_argv);
+              "Use --verbose --help to get a list of available options\n",
+              my_progname, *tmp_argv);
       unireg_abort(1);
     }
   }

sql/partition_element.h

@@ -84,9 +84,9 @@ public:
   partition_element(partition_element *part_elem)
   : part_max_rows(part_elem->part_max_rows),
     part_min_rows(part_elem->part_min_rows),
-    partition_name(NULL),
+    range_value(0), partition_name(NULL),
     tablespace_name(part_elem->tablespace_name),
-    range_value(0), part_comment(part_elem->part_comment),
+    part_comment(part_elem->part_comment),
     data_file_name(part_elem->data_file_name),
     index_file_name(part_elem->index_file_name),
     engine_type(part_elem->engine_type),

sql/rpl_utility.h

@@ -64,7 +64,7 @@ public:
       size  Number of elements in array 'types'
    */
   table_def(field_type *types, my_size_t size)
-    : m_type(types), m_size(size)
+    : m_size(size), m_type(types)
   {
   }
 

sql/set_var.cc

@@ -27,8 +27,6 @@
     of it in the my_option structure list in mysqld.cc
   - Don't forget to initialize new fields in global_system_variables and
     max_system_variables!
-  - If the variable should show up in 'show variables' add it to the
-    init_vars[] struct in this file
 
   NOTES:
     - Be careful with var->save_result: sys_var::check() only updates

sql/set_var.h

@@ -641,12 +641,12 @@ public:
 
 class sys_var_character_set_sv :public sys_var_character_set
 {
-  CHARSET_INFO **global_default;
   CHARSET_INFO *SV::*offset;
+  CHARSET_INFO **global_default;
 public:
   sys_var_character_set_sv(sys_var_chain *chain, const char *name_arg,
                             CHARSET_INFO *SV::*offset_arg,
-                            CHARSET_INFO **global_default_arg, 
+                            CHARSET_INFO **global_default_arg,
                             bool is_nullable= 0)
     : sys_var_character_set(name_arg, is_nullable),
       offset(offset_arg), global_default(global_default_arg)
@@ -668,8 +668,8 @@ public:
 
 class sys_var_collation_sv :public sys_var_collation
 {
-  CHARSET_INFO **global_default;
   CHARSET_INFO *SV::*offset;
+  CHARSET_INFO **global_default;
 public:
   sys_var_collation_sv(sys_var_chain *chain, const char *name_arg,
                                CHARSET_INFO *SV::*offset_arg,

sql/sql_class.cc

@@ -227,8 +227,8 @@ int thd_sql_command(const THD *thd)
     pointer to string
 */
 extern "C"
-char *thd_security_context(THD *thd, char *buffer, int length,
-                           int max_query_len)
+char *thd_security_context(THD *thd, char *buffer, unsigned int length,
+                           unsigned int max_query_len)
 {
   String str(buffer, length, &my_charset_latin1);
   const Security_context *sctx= &thd->main_security_ctx;
@@ -293,13 +293,16 @@ THD::THD()
    user_time(0), in_sub_stmt(0),
    binlog_table_maps(0),
    global_read_lock(0), is_fatal_error(0),
-   rand_used(0), time_zone_used(0),
    arg_of_last_insert_id_function(FALSE),
    first_successful_insert_id_in_prev_stmt(0),
    first_successful_insert_id_in_prev_stmt_for_binlog(0),
    first_successful_insert_id_in_cur_stmt(0),
-   in_lock_tables(0), bootstrap(0), derived_tables_processing(FALSE),
+   rand_used(0),
+   time_zone_used(0),
+   in_lock_tables(0),
    stmt_depends_on_first_successful_insert_id_in_prev_stmt(FALSE),
+   bootstrap(0),
+   derived_tables_processing(FALSE),
    spcont(NULL)
 {
   ulong tmp;
@@ -1765,15 +1768,16 @@ void Query_arena::cleanup_stmt()
 }
 
 /*
-  Statement functions 
+  Statement functions
 */
 
 Statement::Statement(enum enum_state state_arg, ulong id_arg,
                      ulong alloc_block_size, ulong prealloc_size)
   :Query_arena(&main_mem_root, state_arg),
+  main_lex(),
   id(id_arg),
   mark_used_columns(MARK_COLUMNS_READ),
-  main_lex(), lex(&main_lex),
+  lex(&main_lex),
   query(0),
   query_length(0),
   cursor(0)

sql/sql_parse.cc

@@ -43,37 +43,37 @@ static bool execute_sqlcom_select(THD *thd, TABLE_LIST *all_tables);
 const char *any_db="*any*";	// Special symbol for check_access
 
 const LEX_STRING command_name[]={
-  C_STRING_WITH_LEN("Sleep"),
-  C_STRING_WITH_LEN("Quit"),
-  C_STRING_WITH_LEN("Init DB"),
-  C_STRING_WITH_LEN("Query"),
-  C_STRING_WITH_LEN("Field List"),
-  C_STRING_WITH_LEN("Create DB"),
-  C_STRING_WITH_LEN("Drop DB"),
-  C_STRING_WITH_LEN("Refresh"),
-  C_STRING_WITH_LEN("Shutdown"),
-  C_STRING_WITH_LEN("Statistics"),
-  C_STRING_WITH_LEN("Processlist"),
-  C_STRING_WITH_LEN("Connect"),
-  C_STRING_WITH_LEN("Kill"),
-  C_STRING_WITH_LEN("Debug"),
-  C_STRING_WITH_LEN("Ping"),
-  C_STRING_WITH_LEN("Time"),
-  C_STRING_WITH_LEN("Delayed insert"),
-  C_STRING_WITH_LEN("Change user"),
-  C_STRING_WITH_LEN("Binlog Dump"),
-  C_STRING_WITH_LEN("Table Dump"),
-  C_STRING_WITH_LEN("Connect Out"),
-  C_STRING_WITH_LEN("Register Slave"),
-  C_STRING_WITH_LEN("Prepare"),
-  C_STRING_WITH_LEN("Execute"),
-  C_STRING_WITH_LEN("Long Data"),
-  C_STRING_WITH_LEN("Close stmt"),
-  C_STRING_WITH_LEN("Reset stmt"),
-  C_STRING_WITH_LEN("Set option"),
-  C_STRING_WITH_LEN("Fetch"),
-  C_STRING_WITH_LEN("Daemon"),
-  C_STRING_WITH_LEN("Error")  // Last command number
+  { STRING_WITH_LEN("Sleep") },
+  { STRING_WITH_LEN("Quit") },
+  { STRING_WITH_LEN("Init DB") },
+  { STRING_WITH_LEN("Query") },
+  { STRING_WITH_LEN("Field List") },
+  { STRING_WITH_LEN("Create DB") },
+  { STRING_WITH_LEN("Drop DB") },
+  { STRING_WITH_LEN("Refresh") },
+  { STRING_WITH_LEN("Shutdown") },
+  { STRING_WITH_LEN("Statistics") },
+  { STRING_WITH_LEN("Processlist") },
+  { STRING_WITH_LEN("Connect") },
+  { STRING_WITH_LEN("Kill") },
+  { STRING_WITH_LEN("Debug") },
+  { STRING_WITH_LEN("Ping") },
+  { STRING_WITH_LEN("Time") },
+  { STRING_WITH_LEN("Delayed insert") },
+  { STRING_WITH_LEN("Change user") },
+  { STRING_WITH_LEN("Binlog Dump") },
+  { STRING_WITH_LEN("Table Dump") },
+  { STRING_WITH_LEN("Connect Out") },
+  { STRING_WITH_LEN("Register Slave") },
+  { STRING_WITH_LEN("Prepare") },
+  { STRING_WITH_LEN("Execute") },
+  { STRING_WITH_LEN("Long Data") },
+  { STRING_WITH_LEN("Close stmt") },
+  { STRING_WITH_LEN("Reset stmt") },
+  { STRING_WITH_LEN("Set option") },
+  { STRING_WITH_LEN("Fetch") },
+  { STRING_WITH_LEN("Daemon") },
+  { STRING_WITH_LEN("Error") }  // Last command number
 };
 
 const char *xa_state_names[]={

storage/example/ha_example.cc

@@ -717,6 +717,11 @@ int ha_example::external_lock(THD *thd, int lock_type)
 
   Called from lock.cc by get_lock_data().
 
+    @note
+  In this method one should NEVER rely on table->in_use, it may, in fact,
+  refer to a different thread! (this happens if get_lock_data() is called
+  from mysql_lock_abort_for_thread() function)
+
     @see
   get_lock_data() in lock.cc
 */

storage/innobase/handler/ha_innodb.cc

@@ -6771,7 +6771,7 @@ ha_innobase::store_lock(
 						ha_tx_isolation());
 	}
 
-	DBUG_ASSERT(thd == ha_thd());
+	DBUG_ASSERT(thd == current_thd);
 	const bool in_lock_tables = thd_in_lock_tables(thd);
 	const uint sql_command = ha_sql_command();