WL#2936

  "Plugin Server Variables"
  Post review cleanups.


include/mysql/plugin.h:
  WL2936
    post review clean up
    fix macros to ensure correct type declarations
sql/ha_partition.cc:
  WL2936
    post review clean up
    add in missing my_afree()
sql/handler.cc:
  WL2936
    post review clean up
    remove stray semicolon
sql/sql_plugin.cc:
  WL2936
    post review clean up
    fixup comments and debug code
storage/innobase/handler/ha_innodb.cc:
  WL2936
    Include some of Marko Mäkelä's patches
storage/innobase/handler/ha_innodb.h:
  WL2936
    Include some of Marko Mäkelä's patches

6 files changed, 86 insertions, 76 deletions

diff --git a/include/mysql/plugin.h b/include/mysql/plugin.h
index 200b23cc93d..ef4392a485c 100644
--- a/include/mysql/plugin.h
+++ b/include/mysql/plugin.h
@@ -193,13 +193,14 @@ typedef void (*mysql_var_update_func)(MYSQL_THD thd,
 
 #define DECLARE_MYSQL_SYSVAR_BASIC(name, type) struct { \
   MYSQL_PLUGIN_VAR_HEADER;      \
-  type *value, def_val;         \
+  type *value; type def_val;    \
 } MYSQL_SYSVAR_NAME(name)
 
 #define DECLARE_MYSQL_SYSVAR_SIMPLE(name, type) struct { \
   MYSQL_PLUGIN_VAR_HEADER;      \
-  type *value, def_val, min_val,\
-       max_val, blk_sz;         \
+  type *value; type def_val;    \
+  type min_val; type max_val;   \
+  type blk_sz;                  \
 } MYSQL_SYSVAR_NAME(name)
 
 #define DECLARE_MYSQL_SYSVAR_TYPELIB(name) struct { \
@@ -221,7 +222,8 @@ typedef void (*mysql_var_update_func)(MYSQL_THD thd,
 #define DECLARE_MYSQL_THDVAR_SIMPLE(name, type) struct { \
   MYSQL_PLUGIN_VAR_HEADER;      \
   int offset;                   \
-  type def_val, min_val, max_val, blk_sz;  \
+  type def_val; type min_val;   \
+  type max_val; type blk_sz;    \
   DECLARE_THDVAR_FUNC(type);    \
 } MYSQL_SYSVAR_NAME(name)
 
diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc
index d8fd3b5f934..d37fcea339b 100644
--- a/sql/ha_partition.cc
+++ b/sql/ha_partition.cc
@@ -2173,7 +2173,7 @@ bool ha_partition::get_from_handler_file(const char *name, MEM_ROOT *mem_root)
   address_tot_name_len= file_buffer + 12 + 4 * tot_partition_words;
   tot_name_words= (uint4korr(address_tot_name_len) + 3) / 4;
   if (len_words != (tot_partition_words + tot_name_words + 4))
-    goto err2;
+    goto err3;
   name_buffer_ptr= file_buffer + 16 + 4 * tot_partition_words;
   VOID(my_close(file, MYF(0)));
   m_file_buffer= file_buffer;          // Will be freed in clear_handler_file()
@@ -2181,10 +2181,12 @@ bool ha_partition::get_from_handler_file(const char *name, MEM_ROOT *mem_root)
   
   if (!(m_engine_array= (plugin_ref*)
                 my_malloc(m_tot_parts * sizeof(plugin_ref), MYF(MY_WME))))
-    goto err2;
+    goto err3;
 
   for (i= 0; i < m_tot_parts; i++)
     m_engine_array[i]= ha_lock_engine(NULL, engine_array[i]);
+
+  my_afree((gptr) engine_array);
     
   if (!m_file && create_handlers(mem_root))
   {
@@ -2193,6 +2195,8 @@ bool ha_partition::get_from_handler_file(const char *name, MEM_ROOT *mem_root)
   }
   DBUG_RETURN(FALSE);
 
+err3:
+  my_afree((gptr) engine_array);
 err2:
   my_free(file_buffer, MYF(0));
 err1:
diff --git a/sql/handler.cc b/sql/handler.cc
index bb7c39be262..f548dc32b7e 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -165,7 +165,7 @@ plugin_ref ha_lock_engine(THD *thd, handlerton *hton)
   {
     st_plugin_int **plugin= hton2plugin + hton->slot;
     
-#ifdef DBUG_OFF;
+#ifdef DBUG_OFF
     return my_plugin_lock(thd, plugin);
 #else
     return my_plugin_lock(thd, &plugin);
diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc
index af73a42dab1..a2006d2e072 100644
--- a/sql/sql_plugin.cc
+++ b/sql/sql_plugin.cc
@@ -628,6 +628,11 @@ static plugin_ref intern_plugin_lock(LEX *lex, plugin_ref rc CALLER_INFO_PROTO)
 
     plugin= pi;
 #else
+    /*
+      For debugging, we do an additional malloc which allows the
+      memory manager and/or valgrind to track locked references and
+      double unlocks to aid resolving reference counting.problems.
+    */
     if (!(plugin= (plugin_ref) my_malloc_ci(sizeof(pi), MYF(MY_WME))))
       DBUG_RETURN(NULL);
 
@@ -927,7 +932,6 @@ static void intern_plugin_unlock(LEX *lex, plugin_ref plugin)
   if (!pi->plugin_dl)
     DBUG_VOID_RETURN;
 #else
-  *(long *) plugin= -1; /* salt the ground, we're debugging */
   my_free((gptr) plugin, MYF(MY_WME));
 #endif
 
@@ -1085,7 +1089,7 @@ static byte *get_bookmark_hash_key(const byte *buff, uint *length,
   From there we load up the dynamic types (assuming we have not been told to
   skip this part).
 
-  Finally we inializie everything, aka the dynamic that have yet to initialize.
+  Finally we initialize everything, aka the dynamic that have yet to initialize.
 */
 int plugin_init(int *argc, char **argv, int flags)
 {
@@ -1240,7 +1244,7 @@ static bool register_builtin(struct st_mysql_plugin *plugin,
                              struct st_plugin_int *tmp,
                              struct st_plugin_int **ptr)
 {
-  DBUG_ENTER("plugin_register_builtin");
+  DBUG_ENTER("register_builtin");
 
   tmp->plugin= plugin;
   tmp->name.str= (char *)plugin->name;
@@ -1262,8 +1266,10 @@ static bool register_builtin(struct st_mysql_plugin *plugin,
 }
 
 
+#ifdef NOT_USED_YET
 /*
   Register a plugin at run time. (note, this doesn't initialize a plugin)
+  Will be useful for embedded applications.
 
   SYNOPSIS
     plugin_register_builtin()
@@ -1298,6 +1304,7 @@ end:
 
   DBUG_RETURN(result);;
 }
+#endif /* NOT_USED_YET */
 
 
 /*
@@ -1523,15 +1530,12 @@ void plugin_shutdown(void)
     /*
       Now we can deallocate all memory.
     */
-#if defined(SAFE_MUTEX) && !defined(DBUG_OFF)
+
     /* neccessary to avoid safe_mutex_assert_owner() trap */
     pthread_mutex_lock(&LOCK_plugin);
-#endif
     cleanup_variables(NULL, &global_system_variables, true);
     cleanup_variables(NULL, &max_system_variables, true);
-#if defined(SAFE_MUTEX) && !defined(DBUG_OFF)
     pthread_mutex_unlock(&LOCK_plugin);
-#endif
 
     initialized= 0;
     pthread_mutex_destroy(&LOCK_plugin);
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index 5c079ca2c65..16066a5ca40 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -142,7 +142,6 @@ extern "C" {
 #define HA_INNOBASE_ROWS_IN_TABLE 10000 /* to get optimization right */
 #define HA_INNOBASE_RANGE_COUNT	  100
 
-ulong	innobase_large_page_size = 0;
 
 /* The default values for the following, type long or longlong, start-up
 parameters are declared in mysqld.cc: */
@@ -169,18 +168,27 @@ char*	innobase_unix_file_flush_method		= NULL;
 /* Below we have boolean-valued start-up parameters, and their default
 values */
 
+static
 ulong	innobase_fast_shutdown			= 1;
+#ifdef UNIV_LOG_ARCHIVE
+static
 my_bool innobase_log_archive			= FALSE;/* unused */
+#endif /* UNIG_LOG_ARCHIVE */
+static
 my_bool innobase_use_doublewrite		= TRUE;
+static
 my_bool innobase_use_checksums			= TRUE;
-my_bool innobase_use_large_pages		= FALSE;
-my_bool	innobase_use_native_aio			= FALSE;
+static
 my_bool	innobase_file_per_table			= FALSE;
+static
 my_bool innobase_locks_unsafe_for_binlog	= FALSE;
+static
 my_bool innobase_rollback_on_timeout		= FALSE;
+static
 my_bool innobase_create_status_file		= FALSE;
 
-static char *internal_innobase_data_file_path	= NULL;
+static
+char*	internal_innobase_data_file_path	= NULL;
 
 /* The following counter is used to convey information to InnoDB
 about server activity: in selects it is not sensible to call
@@ -188,6 +196,7 @@ srv_active_wake_master_thread after each fetch or search, we only do
 it every INNOBASE_WAKE_INTERVAL'th step. */
 
 #define INNOBASE_WAKE_INTERVAL	32
+static
 ulong	innobase_active_counter	= 0;
 
 static HASH	innobase_open_tables;
@@ -241,6 +250,7 @@ innobase_commit_low(
 /*================*/
 	trx_t*	trx);	/* in: transaction handle */
 
+static
 SHOW_VAR innodb_status_variables[]= {
   {"buffer_pool_pages_data",
   (char*) &export_vars.innodb_buffer_pool_pages_data,	  SHOW_LONG},
@@ -1315,14 +1325,6 @@ innobase_init(void *p)
         innobase_hton->flags=HTON_NO_FLAGS;
         innobase_hton->release_temporary_latches=innobase_release_temporary_latches;
 
-#ifdef HAVE_LARGE_PAGES
-        if (my_use_large_pages)
-        {
-                innobase_use_large_pages= 1;
-                innobase_large_page_size= opt_large_page_size;
-        }
-#endif
-
 	ut_a(DATA_MYSQL_TRUE_VARCHAR == (ulint)MYSQL_TYPE_VARCHAR);
 
 #ifdef UNIV_DEBUG
@@ -1504,8 +1506,10 @@ innobase_init(void *p)
 	srv_use_doublewrite_buf = (ibool) innobase_use_doublewrite;
 	srv_use_checksums = (ibool) innobase_use_checksums;
 
-	os_use_large_pages = (ibool) innobase_use_large_pages;
-	os_large_page_size = (ulint) innobase_large_page_size;
+#ifdef HAVE_LARGE_PAGES
+        if ((os_use_large_pages = (ibool) my_use_large_pages))
+		os_large_page_size = (ulint) opt_large_page_size;
+#endif
 
 	row_rollback_on_timeout = (ibool) innobase_rollback_on_timeout;
 
@@ -4947,29 +4951,29 @@ ha_innobase::delete_all_rows(void)
 
 	DBUG_ENTER("ha_innobase::delete_all_rows");
 
-	if (ha_sql_command() != SQLCOM_TRUNCATE) {
-	fallback:
-		/* We only handle TRUNCATE TABLE t as a special case.
-		DELETE FROM t will have to use ha_innobase::delete_row(). */
-		DBUG_RETURN(my_errno=HA_ERR_WRONG_COMMAND);
-	}
-
 	/* Get the transaction associated with the current thd, or create one
 	if not yet created, and update prebuilt->trx */
 
 	update_thd(thd);
 
-	/* Truncate the table in InnoDB */
+	if (ha_sql_command() == SQLCOM_TRUNCATE) {
+		/* Truncate the table in InnoDB */
 
-	error = row_truncate_table_for_mysql(prebuilt->table, prebuilt->trx);
-	if (error == DB_ERROR) {
-		/* Cannot truncate; resort to ha_innobase::delete_row() */
-		goto fallback;
-	}
+		error = row_truncate_table_for_mysql(prebuilt->table, prebuilt->trx);
+		if (error == DB_ERROR) {
+			/* Cannot truncate; resort to ha_innobase::delete_row() */
+			goto fallback;
+		}
 
-	error = convert_error_code_to_mysql(error, NULL);
+		error = convert_error_code_to_mysql(error, NULL);
 
-	DBUG_RETURN(error);
+		DBUG_RETURN(error);
+	}
+
+fallback:
+	/* We only handle TRUNCATE TABLE t as a special case.
+	DELETE FROM t will have to use ha_innobase::delete_row(). */
+	DBUG_RETURN(my_errno=HA_ERR_WRONG_COMMAND);
 }
 
 /*********************************************************************
@@ -6767,17 +6771,21 @@ ha_innobase::store_lock(
 						ha_tx_isolation());
 	}
 
-	if (ha_sql_command() == SQLCOM_DROP_TABLE) {
+	DBUG_ASSERT(thd == ha_thd());
+	const bool in_lock_tables = thd_in_lock_tables(thd);
+	const uint sql_command = ha_sql_command();
+
+	if (sql_command == SQLCOM_DROP_TABLE) {
 
 		/* MySQL calls this function in DROP TABLE though this table
 		handle may belong to another thd that is running a query. Let
 		us in that case skip any changes to the prebuilt struct. */ 
 
-	} else if ((thd_in_lock_tables(thd) &&
+	} else if ((in_lock_tables &&
 		(lock_type == TL_READ || lock_type == TL_READ_HIGH_PRIORITY)) ||
 		lock_type == TL_READ_WITH_SHARED_LOCKS ||
 		lock_type == TL_READ_NO_INSERT ||
-		(ha_sql_command() != SQLCOM_SELECT
+		(sql_command != SQLCOM_SELECT
 			&& lock_type != TL_IGNORE)) {
 
 		/* The OR cases above are in this order:
@@ -6806,9 +6814,9 @@ ha_innobase::store_lock(
 			|| isolation_level == TRX_ISO_READ_COMMITTED)
 		&& isolation_level != TRX_ISO_SERIALIZABLE
 		&& (lock_type == TL_READ || lock_type == TL_READ_NO_INSERT)
-		&& (ha_sql_command() == SQLCOM_INSERT_SELECT
-			|| ha_sql_command() == SQLCOM_UPDATE
-			|| ha_sql_command() == SQLCOM_CREATE_TABLE)) {
+		&& (sql_command == SQLCOM_INSERT_SELECT
+			|| sql_command == SQLCOM_UPDATE
+			|| sql_command == SQLCOM_CREATE_TABLE)) {
 
 			/* If we either have innobase_locks_unsafe_for_binlog
 			option set or this session is using READ COMMITTED
@@ -6821,7 +6829,7 @@ ha_innobase::store_lock(
 
 			prebuilt->select_lock_type = LOCK_NONE;
 			prebuilt->stored_select_lock_type = LOCK_NONE;
-		} else if (ha_sql_command() == SQLCOM_CHECKSUM) {
+		} else if (sql_command == SQLCOM_CHECKSUM) {
 			/* Use consistent read for checksum table */
 
 			prebuilt->select_lock_type = LOCK_NONE;
@@ -6851,7 +6859,7 @@ ha_innobase::store_lock(
 		(if it does not use a consistent read). */
 
 		if (lock_type == TL_READ
-		    && ha_sql_command() == SQLCOM_LOCK_TABLES) {
+		    && sql_command == SQLCOM_LOCK_TABLES) {
 			/* We come here if MySQL is processing LOCK TABLES
 			... READ LOCAL. MyISAM under that table lock type
 			reads the table as it was at the time the lock was
@@ -6878,23 +6886,23 @@ ha_innobase::store_lock(
 
 		if ((lock_type >= TL_WRITE_CONCURRENT_INSERT
 		&& lock_type <= TL_WRITE)
-		&& !(thd_in_lock_tables(thd)
-			&& ha_sql_command() == SQLCOM_LOCK_TABLES)
+		&& !(in_lock_tables
+			&& sql_command == SQLCOM_LOCK_TABLES)
 		&& !thd_tablespace_op(thd)
-		&& ha_sql_command() != SQLCOM_TRUNCATE
-		&& ha_sql_command() != SQLCOM_OPTIMIZE
+		&& sql_command != SQLCOM_TRUNCATE
+		&& sql_command != SQLCOM_OPTIMIZE
 
 #ifdef __WIN__
-                /* For alter table on win32 for succesful operation
-                completion it is used TL_WRITE(=10) lock instead of
-                TL_WRITE_ALLOW_READ(=6), however here in innodb handler
-                TL_WRITE is lifted to TL_WRITE_ALLOW_WRITE, which causes
-                race condition when several clients do alter table
-                simultaneously (bug #17264). This fix avoids the problem. */
-                && ha_sql_command() != SQLCOM_ALTER_TABLE
+		/* For alter table on win32 for succesful operation
+		completion it is used TL_WRITE(=10) lock instead of
+		TL_WRITE_ALLOW_READ(=6), however here in innodb handler
+		TL_WRITE is lifted to TL_WRITE_ALLOW_WRITE, which causes
+		race condition when several clients do alter table
+		simultaneously (bug #17264). This fix avoids the problem. */
+		&& sql_command != SQLCOM_ALTER_TABLE
 #endif
 
-		&& ha_sql_command() != SQLCOM_CREATE_TABLE) {
+		&& sql_command != SQLCOM_CREATE_TABLE) {
 
 			lock_type = TL_WRITE_ALLOW_WRITE;
 		}
@@ -6910,7 +6918,7 @@ ha_innobase::store_lock(
 		(MySQL does have thd_in_lock_tables() TRUE there). */
 
 		if (lock_type == TL_READ_NO_INSERT
-		    && ha_sql_command() != SQLCOM_LOCK_TABLES) {
+		    && sql_command != SQLCOM_LOCK_TABLES) {
 
 			lock_type = TL_READ;
 		}
@@ -7652,9 +7660,11 @@ static MYSQL_SYSVAR_STR(log_arch_dir, innobase_log_arch_dir,
   PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_READONLY,
   "Where full logs should be archived.", NULL, NULL, NULL);
 
+#ifdef UNIV_LOG_ARCHIVE
 static MYSQL_SYSVAR_BOOL(log_archive, innobase_log_archive,
   PLUGIN_VAR_OPCMDARG | PLUGIN_VAR_READONLY,
   "Set to 1 if you want to have logs archived.", NULL, NULL, FALSE);
+#endif /* UNIV_LOG_ARCHIVE */
 
 static MYSQL_SYSVAR_STR(log_group_home_dir, innobase_log_group_home_dir,
   PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_READONLY,
@@ -7784,7 +7794,9 @@ static struct st_mysql_sys_var* innobase_system_variables[]= {
   MYSQL_SYSVAR(locks_unsafe_for_binlog),
   MYSQL_SYSVAR(lock_wait_timeout),
   MYSQL_SYSVAR(log_arch_dir),
+#ifdef UNIV_LOG_ARCHIVE
   MYSQL_SYSVAR(log_archive),
+#endif /* UNIV_LOG_ARCHIVE */
   MYSQL_SYSVAR(log_buffer_size),
   MYSQL_SYSVAR(log_file_size),
   MYSQL_SYSVAR(log_files_in_group),
diff --git a/storage/innobase/handler/ha_innodb.h b/storage/innobase/handler/ha_innodb.h
index bc19f6fbf93..75cccae0e92 100644
--- a/storage/innobase/handler/ha_innodb.h
+++ b/storage/innobase/handler/ha_innodb.h
@@ -202,9 +202,6 @@ class ha_innobase: public handler
 					uint table_changes);
 };
 
-extern SHOW_VAR innodb_status_variables[];
-extern ulong innobase_fast_shutdown;
-extern ulong innobase_large_page_size;
 extern long innobase_mirrored_log_groups, innobase_log_files_in_group;
 extern longlong innobase_buffer_pool_size, innobase_log_file_size;
 extern long innobase_log_buffer_size;
@@ -216,15 +213,6 @@ extern long innobase_open_files;
 extern char *innobase_data_home_dir, *innobase_data_file_path;
 extern char *innobase_log_group_home_dir, *innobase_log_arch_dir;
 extern char *innobase_unix_file_flush_method;
-/* The following variables have to be my_bool for SHOW VARIABLES to work */
-extern my_bool innobase_log_archive,
-	innobase_use_doublewrite,
-	innobase_use_checksums,
-	innobase_use_large_pages,
-	innobase_use_native_aio,
-	innobase_file_per_table, innobase_locks_unsafe_for_binlog,
-	innobase_rollback_on_timeout,
-	innobase_create_status_file;
 extern "C" {
 extern ulong srv_max_buf_pool_modified_pct;
 extern ulong srv_max_purge_lag;