Hi!
On Nov 29, antony@stripped wrote:
> ChangeSet@stripped, 2007-11-29 14:40:36-08:00, acurtis@stripped +34 -0
> WL#3771 - Plugable Audit Interface
> First commit for review
> Have run default tests with/without plugin:
> mysql-test-run.pl --mysqld=--plugin-dir=<path to plugin>
> --mysqld=--plugin-load=adt_null.so
> include/Makefile.am@stripped, 2007-11-29 14:40:15-08:00, acurtis@stripped +2 -1
> add new source files: mysql/ftparser.h mysql/audit.h
in wl#4017 patch (query rewrite plugin) BrianM also had a new file for
the rewrite plugin. But he called it rewrite_plugin.h.
I think that having _plugin as a part of include file name is a good
idea - we may have other non-plugin include files someday. Even better
to have a plugin_ prefix, not suffix, to keep all these files together.
See also my comment to your change in myisam/ha_myisam.cc (below).
Please rename your include files to plugin_ftparser.h and plugin_audit.h
> include/mysql/ftparser.h@stripped, 2007-11-29 14:40:19-08:00, acurtis@stripped +211
> -0
> New BitKeeper file ``include/mysql/ftparser.h''
It's good that you moved ftparser related code into a separate file.
Thanks.
> plugin/audit_null/configure.in@stripped, 2007-11-29 14:40:19-08:00, acurtis@stripped
> +9 -0
> New BitKeeper file ``plugin/audit_null/configure.in''
did you try to build audit_plugin outside of MySQL source tree ?
if not, and it's not needed (which, I suspect is the case) you don't
need configure.in, NEWS/AUTHORS/ChangeLog and other files that automake
> diff -Nrup a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc
> --- a/storage/myisam/ha_myisam.cc 2007-10-25 00:39:55 -07:00
> +++ b/storage/myisam/ha_myisam.cc 2007-11-29 14:40:18 -08:00
> @@ -20,7 +20,7 @@
>
> #define MYSQL_SERVER 1
> #include "mysql_priv.h"
> -#include <mysql/plugin.h>
> +#include <mysql/ftparser.h>
basically, what you've done is a change in the API (but not in ABI) -
old plugins win't compile anymore, they'll need to be changed.
I don't think it was necessary. And (I'm looking in a randomly picked
directory in /usr/include) in libgnome one is expected to write only
#include <libgnome/libgnome.h> and other 13 include files are
included by libgnome.h
Let's do the same - it's good to have plugin_ftparser.h, plugin_audit.h,
plugin_rewrite.h, it helps to keep include files structured and maintainable.
But it doesn't need to affect the API. Make all these plugin_*.h
files to be included from the main plugin.h
> #include <m_ctype.h>
> #include <my_bit.h>
> #include <myisampack.h>
> diff -Nrup a/include/mysql/plugin.h b/include/mysql/plugin.h
> --- a/include/mysql/plugin.h 2007-08-30 23:19:49 -07:00
> +++ b/include/mysql/plugin.h 2007-11-29 14:40:16 -08:00
> @@ -135,6 +136,7 @@ typedef int (*mysql_show_var_func)(MYSQL
> #define PLUGIN_VAR_STR 0x0005
> #define PLUGIN_VAR_ENUM 0x0006
> #define PLUGIN_VAR_SET 0x0007
> +#define PLUGIN_VAR_OPAQUE 0x0008
please remove it, as we've discussed on irc
> #define PLUGIN_VAR_UNSIGNED 0x0080
> #define PLUGIN_VAR_THDLOCAL 0x0100 /* Variable is per-connection */
> #define PLUGIN_VAR_READONLY 0x0200 /* Server variable is read only */
> @@ -143,6 +145,7 @@ typedef int (*mysql_show_var_func)(MYSQL
> #define PLUGIN_VAR_NOCMDARG 0x1000 /* No argument for cmd line */
> #define PLUGIN_VAR_RQCMDARG 0x0000 /* Argument required for cmd line */
> #define PLUGIN_VAR_OPCMDARG 0x2000 /* Argument optional for cmd line */
> +#define PLUGIN_VAR_NOGLOBAL 0x4000 /* per-connection has no global */
please remove it, as we've discussed on irc
> #define PLUGIN_VAR_MEMALLOC 0x8000 /* String needs memory allocated */
>
> struct st_mysql_sys_var;
> diff -Nrup a/sql/ha_ndbcluster_binlog.cc b/sql/ha_ndbcluster_binlog.cc
> --- a/sql/ha_ndbcluster_binlog.cc 2007-11-13 03:52:17 -08:00
> +++ b/sql/ha_ndbcluster_binlog.cc 2007-11-29 14:40:16 -08:00
> @@ -1610,6 +1615,9 @@ end:
> if (ndb_extra_logging)
> ndb_report_waiting(type_str, max_timeout,
> "distributing", ndb_schema_object->key);
> +
> + /* Release any held audit resources */
> + mysql_audit_release(thd);
why here ?
> }
> }
> if (have_lock_open)
> @@ -3402,6 +3410,9 @@ ndbcluster_handle_alter_table(THD *thd,
> if (ndb_extra_logging)
> ndb_report_waiting(type_str, max_timeout,
> type_str, share->key);
> +
> + /* Release any held audit resources */
> + mysql_audit_release(thd);
why here ?
> }
> }
> (void) pthread_mutex_unlock(&share->mutex);
> @@ -3472,6 +3483,9 @@ ndbcluster_handle_drop_table(THD *thd, N
> if (ndb_extra_logging)
> ndb_report_waiting(type_str, max_timeout,
> type_str, share->key);
> +
> + /* Release any held audit resources */
> + mysql_audit_release(thd);
why here ?
> }
> }
> (void) pthread_mutex_lock(&LOCK_open);
> diff -Nrup a/sql/scheduler.cc b/sql/scheduler.cc
> --- a/sql/scheduler.cc 2007-11-02 12:39:00 -07:00
> +++ b/sql/scheduler.cc 2007-11-29 14:40:17 -08:00
> @@ -396,6 +397,8 @@ static void libevent_connection_close(TH
>
> thd->killed= THD::KILL_CONNECTION; // Avoid error messages
>
> + mysql_audit_release(thd);
> +
why here ? it should be done later, e.g. in thd->cleanup()
as late as possible, to ensure there can be no more audit events
after mysql_audit_release()
> if (thd->net.vio->sd >= 0) // not already closed
> {
> end_connection(thd);
> diff -Nrup a/sql/slave.cc b/sql/slave.cc
> --- a/sql/slave.cc 2007-11-22 11:45:20 -08:00
> +++ b/sql/slave.cc 2007-11-29 14:40:17 -08:00
> @@ -1856,6 +1864,10 @@ static int try_to_reconnect(THD *thd, MY
> sql_print_information(buf);
> }
> }
> +
> + /* Release any held audit resources */
> + mysql_audit_release(thd);
> +
I don't think you need to do it that often...
> if (safe_reconnect(thd, mysql, mi, 1) || io_slave_killed(thd, mi))
> {
> if (global_system_variables.log_warnings)
> @@ -1951,6 +1963,8 @@ pthread_handler_t handle_slave_io(void *
> }
>
> connected:
> + /* Release any held audit resources */
> + mysql_audit_release(thd);
why here ?
> // TODO: the assignment below should be under mutex (5.0)
> mi->slave_running= MYSQL_SLAVE_RUN_CONNECT;
> diff -Nrup a/sql/sql_insert.cc b/sql/sql_insert.cc
> --- a/sql/sql_insert.cc 2007-11-22 11:45:22 -08:00
> +++ b/sql/sql_insert.cc 2007-11-29 14:40:18 -08:00
> @@ -2416,6 +2418,7 @@ pthread_handler_t handle_delayed_insert(
> di->table->file->ha_release_auto_increment();
> mysql_unlock_tables(thd, lock);
> di->group_count=0;
> + mysql_audit_release(thd);
why here ?
> pthread_mutex_lock(&di->mutex);
> }
> if (di->tables_in_use)
> diff -Nrup a/sql/sql_builtin.cc.in b/sql/sql_builtin.cc.in
> --- a/sql/sql_builtin.cc.in 2006-12-30 17:28:54 -08:00
> +++ b/sql/sql_builtin.cc.in 2007-11-29 14:40:17 -08:00
> @@ -13,6 +13,7 @@
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
>
> +#include <my_global.h>
why ?
> #include <mysql/plugin.h>
>
> typedef struct st_mysql_plugin builtin_plugin[];
> diff -Nrup a/sql/sql_class.h b/sql/sql_class.h
> --- a/sql/sql_class.h 2007-11-22 11:45:22 -08:00
> +++ b/sql/sql_class.h 2007-11-29 14:40:17 -08:00
> @@ -1100,6 +1100,10 @@ public:
> ulong max_client_packet_length;
>
> HASH handler_tables_hash;
> +#ifndef EMBEDDED_LIBRARY
> + DYNAMIC_ARRAY audit_class_plugins;
> + uint64 audit_class_mask1;
1. what does 1 mean in mask1 ?
2. both fields need a comment
> +#endif
> /*
> One thread can hold up to one named user-level lock. This variable
> points to a lock object if the lock is present. See item_func.cc and
> diff -Nrup a/sql/protocol.cc b/sql/protocol.cc
> --- a/sql/protocol.cc 2007-11-01 04:32:31 -07:00
> +++ b/sql/protocol.cc 2007-11-29 14:40:17 -08:00
> @@ -132,6 +133,12 @@ void net_send_error(THD *thd, uint sql_e
>
> DBUG_ASSERT(!thd->spcont);
>
> + MYSQL_AUDIT_GENERAL(thd,MYSQL_AUDIT_GENERAL_ERROR,sql_errno,my_time(0),
> + 0,0,err,err?strlen(err):0,
> + thd->query,thd->query_length,
> + thd->variables.character_set_client,
> + thd->row_count);
this wasn't part of the WL
> if (net && net->no_send_error)
> {
> thd->clear_error();
> @@ -210,6 +217,12 @@ send_ok(THD *thd, ha_rows affected_rows,
> NET *net= &thd->net;
> uchar buff[MYSQL_ERRMSG_SIZE+10],*pos;
> DBUG_ENTER("send_ok");
> +
> + MYSQL_AUDIT_GENERAL(thd,MYSQL_AUDIT_GENERAL_RESULT,0,my_time(0),
> + 0,0,message,message?strlen(message):0,
> + thd->query,thd->query_length,
> + thd->variables.character_set_client,
> + affected_rows);
this wasn't part of the WL
> if (net->no_send_ok || !net->vio) // hack for re-parsing queries
> {
> diff -Nrup a/sql/sql_connect.cc b/sql/sql_connect.cc
> --- a/sql/sql_connect.cc 2007-11-13 03:52:18 -08:00
> +++ b/sql/sql_connect.cc 2007-11-29 14:40:17 -08:00
> @@ -318,6 +319,9 @@ check_user(THD *thd, enum enum_server_co
> DBUG_ENTER("check_user");
> LEX_STRING db_str= { (char *) db, db ? strlen(db) : 0 };
>
> + MYSQL_AUDIT_SECURITY(thd,MYSQL_AUDIT_SECURITY_AUTHENTICATE,0,
> + "USER",&db_str,0,0,0);
it wasn't part of the WL
> /*
> Clear thd->db as it points to something, that will be freed when
> connection is closed. We don't want to accidentally free a wrong
> @@ -673,6 +677,8 @@ static int check_connection(THD *thd)
> #ifdef SIGNAL_WITH_VIO_CLOSE
> thd->set_active_vio(net->vio);
> #endif
> + MYSQL_AUDIT_SECURITY(thd,MYSQL_AUDIT_SECURITY_CONNECT,0,
> + "CONNECTION",0,0,0,0);
it wasn't part of the WL
> if (!thd->main_security_ctx.host) // If TCP/IP connection
> {
> @@ -997,6 +1003,10 @@ bool login_connection(THD *thd)
> void end_connection(THD *thd)
> {
> NET *net= &thd->net;
> +
> + MYSQL_AUDIT_SECURITY(thd,MYSQL_AUDIT_SECURITY_DISCONNECT,0,
> + "CONNECTION",0,0,0,0);
it wasn't part of the WL
> +
> plugin_thdvar_cleanup(thd);
> if (thd->user_connect)
> decrease_user_connections(thd->user_connect);
> @@ -1142,9 +1152,11 @@ pthread_handler_t handle_one_connection(
> !(thd->killed == THD::KILL_CONNECTION))
> {
> net->no_send_error= 0;
> + mysql_audit_release(thd);
> if (do_command(thd))
> break;
> }
> + mysql_audit_release(thd);
same question I've asked in scheduler.cc
> end_connection(thd);
>
> end_thread:
> diff -Nrup a/sql/log.cc b/sql/log.cc
> --- a/sql/log.cc 2007-11-14 02:01:42 -08:00
> +++ b/sql/log.cc 2007-11-29 14:40:16 -08:00
> @@ -4387,6 +4397,33 @@ static void print_buffer_to_file(enum lo
> VOID(pthread_mutex_lock(&LOCK_error_log));
>
> skr= my_time(0);
> +
> + {
> + char user_host_buff[MAX_USER_HOST_SIZE];
> + THD *thd;
> + ulong id= 0;
> + uint user_host_len= 0;
> +
> + if ((thd= current_thd))
> + {
> + Security_context *sctx= thd->security_ctx;
> + id= thd->thread_id;
> + user_host_len= strxnmov(user_host_buff, MAX_USER_HOST_SIZE,
> + sctx->priv_user ? sctx->priv_user : "", "[",
> + sctx->user ? sctx->user : "", "] @ ",
> + sctx->host ? sctx->host : "", " [",
> + sctx->ip ? sctx->ip : "", "]", NullS) -
> + user_host_buff;
> + }
> + MYSQL_AUDIT_GENERAL(thd, MYSQL_AUDIT_GENERAL_CONSOLE, error_code, skr,
> + user_host_buff, user_host_len,
> + (level == ERROR_LEVEL ? "ERROR" :
> + level == WARNING_LEVEL ? "Warning" : "Note"),
> + (level == ERROR_LEVEL ? 5 :
> + level == WARNING_LEVEL ? 7 : 4),
> + buffer, buffer_length,0,0);
> + }
> +
it wasn't part of the WL
> localtime_r(&skr, &tm_tmp);
> start=&tm_tmp;
>
> diff -Nrup a/sql/sql_acl.cc b/sql/sql_acl.cc
> --- a/sql/sql_acl.cc 2007-11-13 03:52:18 -08:00
> +++ b/sql/sql_acl.cc 2007-11-29 14:40:17 -08:00
> @@ -0,0 +1,95 @@
WL (at least what was approved as HLS/LLD) did not need any changes to
sql_acl.cc. Changes to grants are tracked using general audit class.
failed privilege checks - using mysql error class.
> diff -Nrup a/include/mysql/audit.h b/include/mysql/audit.h
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/include/mysql/audit.h 2007-11-29 14:40:19 -08:00
> @@ -0,0 +1,126 @@
> +/* Copyright (C) 2007 MySQL AB
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; version 2 of the License.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program; if not, write to the Free Software
> + Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */
> +
> +#ifndef _my_audit_h
> +#define _my_audit_h
> +
> +/*************************************************************************
> + API for Audit plugin. (MYSQL_AUDIT_PLUGIN)
> +*/
> +
> +#include "plugin.h"
> +
> +#define MYSQL_AUDIT_INTERFACE_VERSION 0x0100
> +
> +
> +struct mysql_event
> +{
> + int event_class;
> +};
> +
> +#ifdef __cplusplus
> +#define DECLARE_AUDIT_EVENT_DATA(name) \
> +struct mysql_event_ ## name :public mysql_event {
> +#else
> +#define DECLARE_AUDIT_EVENT_DATA(name) \
> +struct mysql_event_ ## name { \
> + int event_class;
> +#endif
There's no need to use different code for C and C++
please remove the #ifdef
> +#define END_DECLARE_AUDIT_EVENT_DATA }
I would remove DECLARE_AUDIT_EVENT_DATA and END_DECLARE_AUDIT_EVENT_DATA
macros, using struct ... and } in place, as appropriate.
you don't save or simplify much to justify them.
(unlike your macro helpers related to MYSQL_xxxVAR_yyy variables,
they are useful indeed)
> +
> +
> +/*************************************************************************
> + AUDIT CLASS : GENERAL
> +*/
> +
> +#define MYSQL_AUDIT_GENERAL_CLASS 0
> +#define MYSQL_AUDIT_GENERAL_CLASSMASK1 (1 << MYSQL_AUDIT_GENERAL_CLASS)
> +#define MYSQL_AUDIT_GENERAL_LOG 0
> +#define MYSQL_AUDIT_GENERAL_ERROR 1
> +#define MYSQL_AUDIT_GENERAL_RESULT 2
> +#define MYSQL_AUDIT_GENERAL_CONSOLE 3
only MYSQL_AUDIT_GENERAL_LOG was requested in this WL
> +DECLARE_AUDIT_EVENT_DATA(general)
> + int general_error_code;
> + unsigned long general_thread_id;
> + time_t general_time;
> + const char *general_user;
> + unsigned int general_user_length;
> + const char *general_command;
> + unsigned int general_command_length;
> + const char *general_query;
> + unsigned int general_query_length;
> + struct charset_info_st *general_charset;
> + unsigned long long general_rows;
> +END_DECLARE_AUDIT_EVENT_DATA;
why "general_" prefix ?
> +
> +
> +/*************************************************************************
> + AUDIT CLASS : SECURITY
> +*/
> +
> +#define MYSQL_AUDIT_SECURITY_CLASS 1
> +#define MYSQL_AUDIT_SECURITY_CLASSMASK1 (1 << MYSQL_AUDIT_SECURITY_CLASS)
This wasn't specified in WL. mysql error audit class was, but it's not
implemented
> +#define MYSQL_AUDIT_SECURITY_RELOAD 0
> +#define MYSQL_AUDIT_SECURITY_AUTHENTICATE 1
> +#define MYSQL_AUDIT_SECURITY_CONNECT 2
> +#define MYSQL_AUDIT_SECURITY_DISCONNECT 3
> +#define MYSQL_AUDIT_SECURITY_CHECK_ENTER 4
> +#define MYSQL_AUDIT_SECURITY_CHECK_LEAVE 5
> +#define MYSQL_AUDIT_SECURITY_GRANT 6
> +#define MYSQL_AUDIT_SECURITY_REVOKE 7
> +DECLARE_AUDIT_EVENT_DATA(security)
> + int security_error_code;
> + unsigned long security_thread_id;
> + const char *security_host;
> + unsigned int security_host_length;
> + const char *security_user;
> + unsigned int security_user_length;
> + const char *security_priv_user;
> + unsigned int security_priv_user_length;
> + const char *security_ip;
> + unsigned int security_ip_length;
> + const char *security_host_or_ip;
> + unsigned int security_host_or_ip_length;
> + unsigned long security_global_access;
> + unsigned long security_schema_access;
> +
> + const char *security_type;
> + unsigned int security_type_length;
> + const char *security_schema;
> + unsigned int security_schema_length;
> + const char *security_object;
> + unsigned int security_object_length;
> + const char *security_element;
> + unsigned int security_element_length;
> +
> + unsigned long security_object_rights;
> +END_DECLARE_AUDIT_EVENT_DATA;
> +
> +
> +/*************************************************************************
> + Here we define the descriptor structure, that is referred from
> + st_mysql_plugin.
> +*/
> +
> +struct st_mysql_audit
> +{
> + int interface_version;
> + const unsigned long long *class_mask;
what is class_mask ? how is the mask defined ?
> + void (*release_thd)(MYSQL_THD);
> + void (*event_notify)(MYSQL_THD, const struct mysql_event *);
> +};
> +
no comments at all in this file. It *must* be commented, documenting itself
exhaustively. See ftparser.h for example.
> +
> +#endif
> diff -Nrup a/plugin/audit_null/audit_null.c b/plugin/audit_null/audit_null.c
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/plugin/audit_null/audit_null.c 2007-11-29 14:40:19 -08:00
> @@ -0,0 +1,142 @@
> +/* Copyright (C) 2006-2007 MySQL AB
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; version 2 of the License.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program; if not, write to the Free Software
> + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
> +
> +#include <my_global.h>
> +#include <my_sys.h>
> +#include <my_atomic.h>
don't use internal MySQL include files in the example plugins
(instead of my_atomic you can use a mutex or simply allow your
counter to be approximate, accessing it without a lock -
it's an example after all)
> +#include <mysql/audit.h>
> +
> +#if !defined(__attribute__) && (defined(__cplusplus) || !defined(__GNUC__)
> || __GNUC__ == 2 && __GNUC_MINOR__ < 8)
> +#define __attribute__(A)
> +#endif
> +
> +static volatile uint32 number_of_calls; /* for SHOW STATUS, see below */
> +static my_atomic_rwlock_t rwl;
> +
> +
> +/*
> + Initialize the plugin at server start or plugin installation.
> +
> + SYNOPSIS
> + null_audit_plugin_init()
> +
> + DESCRIPTION
> + Does nothing.
> +
> + RETURN VALUE
> + 0 success
> + 1 failure (cannot happen)
> +*/
> +
> +static int null_audit_plugin_init(void *arg __attribute__((unused)))
> +{
> + my_atomic_rwlock_init(&rwl);
> + number_of_calls= 0;
> + return(0);
> +}
> +
> +
> +/*
> + Terminate the plugin at server shutdown or plugin deinstallation.
> +
> + SYNOPSIS
> + null_audit_plugin_deinit()
> + Does nothing.
> +
> + RETURN VALUE
> + 0 success
> + 1 failure (cannot happen)
> +
> +*/
> +
> +static int null_audit_plugin_deinit(void *arg __attribute__((unused)))
> +{
> + my_atomic_rwlock_destroy(&rwl);
> + printf("audit_null was invoked %lu times\n",
> + (ulong) number_of_calls);
> + return(0);
> +}
> +
> +
> +/*
> + Foo
> +
> + SYNOPSIS
> + null_notify()
> + thd connection context
> +
> + DESCRIPTION
> +*/
> +
> +static void null_notify(MYSQL_THD thd __attribute__((unused)),
> + const struct mysql_event *event
> + __attribute__((unused)))
> +{
> + my_atomic_rwlock_wrlock(&rwl);
> + my_atomic_add32(&number_of_calls, 1);
> + my_atomic_rwlock_wrunlock(&rwl);
> +}
> +
> +
> +static unsigned long long null_audit_mask[]=
> +{
> + (unsigned long long) -1
> +};
> +
> +
> +/*
> + Plugin type-specific descriptor
> +*/
> +
> +static struct st_mysql_audit null_audit_descriptor=
> +{
> + MYSQL_AUDIT_INTERFACE_VERSION, /* interface version */
> + null_audit_mask, /* class mask */
> + NULL, /* release_thd function */
> + null_notify /* notify function */
> +};
> +
> +/*
> + Plugin status variables for SHOW STATUS
> +*/
> +
> +static struct st_mysql_show_var simple_status[]=
> +{
> + {"null_audit_called", (char *)&number_of_calls, SHOW_INT},
> + {0,0,0}
> +};
> +
> +
> +/*
> + Plugin library descriptor
> +*/
> +
> +mysql_declare_plugin(null_audit)
> +{
> + MYSQL_AUDIT_PLUGIN, /* type */
> + &null_audit_descriptor, /* descriptor */
> + "NULL_AUDIT", /* name */
> + "MySQL AB", /* author */
> + "Simple NULL Audit", /* description */
> + PLUGIN_LICENSE_GPL,
> + null_audit_plugin_init, /* init function (when loaded) */
> + null_audit_plugin_deinit, /* deinit function (when unloaded) */
> + 0x0001, /* version */
> + simple_status, /* status variables */
> + NULL, /* system variables */
> + NULL
> +}
> +mysql_declare_plugin_end;
> +
> diff -Nrup a/sql/sql_audit.h b/sql/sql_audit.h
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/sql/sql_audit.h 2007-11-29 14:40:18 -08:00
> @@ -0,0 +1,95 @@
> +/* Copyright (C) 2007 MySQL AB
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; version 2 of the License.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program; if not, write to the Free Software
> + Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */
> +
> +
> +#ifndef EMBEDDED_LIBRARY
> +
> +#include <mysql/audit.h>
> +
> +
> +extern uint64 mysql_global_audit_mask1;
there're no comments in this file either
> +
> +
> +extern void mysql_audit_initialize();
> +extern void mysql_audit_finalize();
> +
> +
> +extern void mysql_audit_init_thd(THD *thd);
> +extern void mysql_audit_free_thd(THD *thd);
> +
> +
> +extern void mysql_audit_notify(THD *thd, uint event_class,
> + uint event_subtype,
> + int error_code, ...);
remove error_code (it should be in the varargs part)
> +extern void mysql_audit_release(THD *thd);
> +
> +
> +
> +
please remove end-of-line blanks and expands tabs in all new files.
> +#define MYSQL_AUDIT_GENERAL(thd,event_subtype,rc,time, \
> + user,userlen,cmd,cmdlen, \
> + query,querylen,clientcs,rows) do { \
> + if (mysql_global_audit_mask1 & MYSQL_AUDIT_GENERAL_CLASSMASK1)\
> + { \
> + time_t audit_time= (time); \
> + const char *audit_user= (user), *audit_cmd= (cmd); \
> + const char *audit_query= (query); \
> + uint audit_user_len= (userlen), audit_cmd_len= (cmdlen); \
> + uint audit_query_len= (querylen); \
> + CHARSET_INFO *audit_client_cs= (clientcs); \
> + ha_rows audit_rows=(rows); \
> + mysql_audit_notify((thd), MYSQL_AUDIT_GENERAL_CLASS, \
> + (event_subtype), (rc), audit_time, \
> + audit_user, audit_user_len, \
> + audit_cmd, audit_cmd_len, \
> + audit_query, audit_query_len, \
> + audit_client_cs,audit_rows); \
> + } \
> +} while (0)
it's C++, use inline functions instead of complex macros
> +
> +
> +
> +#define MYSQL_AUDIT_SECURITY(thd,event_subtype,rc,type,schema, \
> + object,element,rights) do { \
> + if (mysql_global_audit_mask1 & MYSQL_AUDIT_SECURITY_CLASSMASK1)\
> + { \
> + LEX_STRING audit_object_type= {C_STRING_WITH_LEN(type)}; \
> + LEX_STRING *audit_schema= (schema); \
> + LEX_STRING *audit_object= (object); \
> + LEX_STRING *audit_element= (element); \
> + ulong audit_rights= (rights); \
> + mysql_audit_notify((thd), MYSQL_AUDIT_SECURITY_CLASS, \
> + (event_subtype),(rc),&audit_object_type, \
> + &audit_schema,&audit_object, \
> + &audit_element,audit_rights); \
> + } \
> +} while (0)
> +
> +#else /* EMBEDDED_LIBRARY */
> +
> +/* In EMBEDDED_LIBRARY builds, these do nothing */
> +
> +#define MYSQL_AUDIT_GENERAL(thd,event_subtype,rc,time, \
> + user,userlen,cmd,cmdlen, \
> + query,querylen,clientcs,rows) do { \
> +} while (0)
> +#define MYSQL_AUDIT_SECURITY(thd,event_subtype,rc,type,schema, \
> + object,element,rights) do { \
> +} while (0)
> +
> +#define mysql_audit_release(thd) do { } while (0)
> +
> +
> +#endif /* EMBEDDED_LIBRARY */
> diff -Nrup a/sql/sql_audit.cc b/sql/sql_audit.cc
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/sql/sql_audit.cc 2007-11-29 14:40:18 -08:00
> @@ -0,0 +1,502 @@
> +/* Copyright (C) 2007 MySQL AB
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; version 2 of the License.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program; if not, write to the Free Software
> + Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */
> +
> +#include "mysql_priv.h"
> +#include "sql_audit.h"
> +
> +extern int initialize_audit_plugin(st_plugin_int *plugin);
> +extern int finalize_audit_plugin(st_plugin_int *plugin);
> +
> +#ifndef EMBEDDED_LIBRARY
> +
> +uint64 mysql_global_audit_mask1= 0;
> +
> +
> +static void event_class_dispatch(THD *thd, const struct mysql_event *event);
> +
> +typedef void (*audit_handler_t)(THD *thd, uint event_subtype,
> + int error_code, va_list ap);
> +
> +/**
> + MYSQL_AUDIT_GENERAL_CLASS handler
> +
> + @param[in] thd
> + @param[in] event_subtype
> + @param[in] error_code
> + @param[in] ap
> +
> +*/
> +
> +static void general_class_handler(THD *thd, uint event_subtype,
> + int error_code, va_list ap)
> +{
> + mysql_event_general event;
> + event.event_class= MYSQL_AUDIT_GENERAL_CLASS;
> + event.general_error_code= error_code;
> + event.general_thread_id= thd ? thd->thread_id : 0;
> + event.general_time= va_arg(ap, time_t);
> + event.general_user= va_arg(ap, const char *);
> + event.general_user_length= va_arg(ap, unsigned int);
> + event.general_command= va_arg(ap, const char *);
> + event.general_command_length= va_arg(ap, unsigned int);
> + event.general_query= va_arg(ap, const char *);
> + event.general_query_length= va_arg(ap, unsigned int);
> + event.general_charset= va_arg(ap, struct charset_info_st *);
> + event.general_rows= (unsigned long long) va_arg(ap, ha_rows);
> + event_class_dispatch(thd, &event);
> +}
> +
> +
> +/**
> + MYSQL_AUDIT_SECURITY_CLASS handler
> +
> + @param[in] thd
> + @param[in] event_subtype
> + @param[in] error_code
> + @param[in] ap
> +
> +*/
> +
> +static void security_class_handler(THD *thd, uint event_subtype,
> + int error_code, va_list ap)
> +{
> + mysql_event_security event;
> + LEX_STRING *str;
> +
> + bzero(&event, sizeof(event));
> + event.event_class= MYSQL_AUDIT_SECURITY_CLASS;
> + event.security_error_code= error_code;
> +
> + if (thd)
> + {
> + event.security_thread_id= thd->thread_id;
> + Security_context *ctx= thd->security_ctx;
> + event.security_host= ctx->host;
> + event.security_host_length=
> + event.security_host ? strlen(event.security_host) : 0;
> + event.security_user= ctx->user;
> + event.security_user_length=
> + event.security_user ? strlen(event.security_user) : 0;
> + event.security_priv_user= ctx->priv_user;
> + event.security_priv_user_length=
> + event.security_priv_user ? strlen(event.security_priv_user) : 0;
> + event.security_ip= ctx->ip;
> + event.security_ip_length=
> + event.security_ip ? strlen(event.security_ip) : 0;
> + event.security_host_or_ip= ctx->host_or_ip;
> + event.security_host_or_ip_length=
> + event.security_host_or_ip ? strlen(event.security_host_or_ip) : 0;
> + event.security_global_access= ctx->master_access;
> + event.security_schema_access= ctx->db_access;
> + }
> +
> + if ((str= va_arg(ap, LEX_STRING *)))
> + {
> + event.security_type= str->str;
> + event.security_type_length= str->length;
> + }
> + if ((str= va_arg(ap, LEX_STRING *)))
> + {
> + event.security_schema= str->str;
> + event.security_schema_length= str->length;
> + }
> + if ((str= va_arg(ap, LEX_STRING *)))
> + {
> + event.security_object= str->str;
> + event.security_object_length= str->length;
> + }
> + if ((str= va_arg(ap, LEX_STRING *)))
> + {
> + event.security_element= str->str;
> + event.security_element_length= str->length;
> + }
> + event.security_object_rights= va_arg(ap, ulong);
> +
> + event_class_dispatch(thd, &event);
> +}
> +
> +
> +static audit_handler_t audit_handlers[] =
> +{
> + /* MYSQL_AUDIT_GENERAL_CLASS */ general_class_handler,
> + /* MYSQL_AUDIT_SECURITY_CLASS */ security_class_handler,
these two comments inside audit_handlers[] are redundant
it's like
i++; // increment i
> + NULL
it doesn't have to be NULL-terminated. Furthermore it's confusing and
wrong to make it NULL-terminated
> +};
> +
> +static const uint audit_handlers_count=
> + (sizeof(audit_handlers) / sizeof(audit_handler_t)) - 1;
> +
> +
> +/**
> + Acquire and lock any additional audit plugins as required
> +
> + @param[in] thd
> + @param[in] plugin
> + @param[in] arg
> +
> + @retval FALSE Always
> +*/
> +
> +static my_bool acquire_plugins(THD *thd, plugin_ref plugin, void *arg)
> +{
> + uint event_class= *(uint*) arg;
> + uint64 event_class_mask1= (1 << event_class);
> + st_mysql_audit *data= plugin_data(plugin, struct st_mysql_audit *);
> +
> + /* Check if this plugin is interested in the event */
> + if (!(data->class_mask[0] & event_class_mask1))
> + return 0;
So, you preferred to scan the list of plugins...
(in the schema that I suggested it would be not necessary)
okay, I think it shouldn't be a problem for a resonable (read: small)
number of plugins. I don't see us having a couple of thousands
of active audit plugins yet.
> +
> + /* Check if we need to initialize the array of acquired plugins */
> + if (unlikely(!thd->audit_class_plugins.buffer))
> + {
> + /* specify some reasonable initialization defaults */
> + my_init_dynamic_array(&thd->audit_class_plugins,
> + sizeof(plugin_ref), 16, 16);
> + }
> + else
> + {
> + plugin_ref *plugins, *plugins_last;
> +
> + plugins= (plugin_ref*) thd->audit_class_plugins.buffer;
> + plugins_last= plugins + thd->audit_class_plugins.elements;
> + for (; plugins < plugins_last; plugins++)
> + {
> + st_mysql_audit *data2= plugin_data(*plugins, struct st_mysql_audit *);
> +
> + /* Check if we already have acquired this plugin */
> + if (data == data2)
> + return 0;
> + }
you don't need this loop. a plugin can be already acquired only if
this plugin has registered for many audit classes, and it was acquired
for a different audit class. Thus, instead of the loop you can write
if (thd->audit_class_mask1 & data->class_mask[0]) // already acquired
return 0;
there's a boundary case when a plugin was installed after thd has registered
plugins for this class, as in
plugin1 (classA) is installed
thd emits event of the classA
thd acquires plugin1, adds classA to audit_class_mask1
plugin2 (classA + classB) is installed
thd emits event of the classB
in this case my if() will skip plugin2. I believe we can ignore this special
case and optimize for all plugins being installed.
> + }
> +
> + /* lock the plugin and add it to the list */
> + plugin= my_plugin_lock(NULL, &plugin);
> + insert_dynamic(&thd->audit_class_plugins, (uchar*) &plugin);
> +
> + return 0;
> +}
> +
> +
> +/**
> + Notify the audit system of an event
> +
> + @param[in] thd
> + @param[in] event_class
> + @param[in] event_subtype
> + @param[in] error_code
> +
> +*/
> +
> +void mysql_audit_notify(THD *thd, uint event_class, uint event_subtype,
> + int error_code, ...)
> +{
> + va_list ap;
> + audit_handler_t *handlers= audit_handlers + event_class;
> + uint64 event_class_mask1= (1 << event_class);
> +
> + DBUG_ASSERT(event_class < audit_handlers_count);
> +
> + if (thd && thd->killed == THD::KILL_CONNECTION &&
> + !(thd->audit_class_mask1 & event_class_mask1))
> + thd= NULL;
why ? (add a comment)
> +
> + /*
> + Check to see if we have acquired the audit plugins for the
> + required audit event classes.
> + */
> + if (thd && !(thd->audit_class_mask1 & event_class_mask1))
> + {
> + plugin_foreach(thd, acquire_plugins, MYSQL_AUDIT_PLUGIN, &event_class);
> + thd->audit_class_mask1|= event_class_mask1;
> + }
> +
> + va_start(ap, error_code);
> + (*handlers)(thd, event_subtype, error_code, ap);
> + va_end(ap);
> +}
> +
> +
> +/**
> + Release any resources associated with the current thd.
> +
> + @param[in] thd
> +
> +*/
> +
> +void mysql_audit_release(THD *thd)
> +{
> + plugin_ref *plugins, *plugins_last;
> +
> + if (!thd || !(thd->audit_class_plugins.elements))
> + return;
> +
> + plugins= (plugin_ref*) thd->audit_class_plugins.buffer;
> + plugins_last= plugins + thd->audit_class_plugins.elements;
> + for (; plugins < plugins_last; plugins++)
> + {
> + st_mysql_audit *data= plugin_data(*plugins, struct st_mysql_audit *);
> +
> + /* Check to see if the plugin has a release method */
> + if (!(data->release_thd))
> + continue;
> +
> + /* Tell the plugin to release its resources */
> + data->release_thd(thd);
> + }
> +
> + /* Now we actually unlock the plugins */
> + plugin_unlock_list(NULL, (plugin_ref*) thd->audit_class_plugins.buffer,
> + thd->audit_class_plugins.elements);
> +
> + /* Reset the state of thread values */
> + reset_dynamic(&thd->audit_class_plugins);
> + thd->audit_class_mask1= 0;
> +}
> +
> +
> +/**
> + Initialize thd variables used by Audit
> +
> + @param[in] thd
> +
> +*/
> +
> +void mysql_audit_init_thd(THD *thd)
> +{
> + bzero(&thd->audit_class_plugins, sizeof(thd->audit_class_plugins));
> + thd->audit_class_mask1= 0;
> +}
> +
> +
> +/**
> + Free thd variables used by Audit
> +
> + @param[in] thd
> + @param[in] plugin
> + @param[in] arg
> +
> + @retval FALSE Always
> +*/
> +
> +void mysql_audit_free_thd(THD *thd)
> +{
> + mysql_audit_release(thd);
> + DBUG_ASSERT(thd->audit_class_plugins.elements == 0);
> + delete_dynamic(&thd->audit_class_plugins);
> +}
> +
> +
> +static DYNAMIC_ARRAY installed_audit_plugins;
> +static pthread_mutex_t LOCK_audit;
why do you need a second list of all audit plugins ?
it'll be a copy of duplicate plugin_hash[MYSQL_AUDIT_PLUGIN].records
(which you can iterate with plugin_foreach(), as you know)
> +
> +
> +/**
> + Initialize Audit global variables
> +*/
> +
> +void mysql_audit_initialize()
> +{
> + (void) pthread_mutex_init(&LOCK_audit, MY_MUTEX_INIT_FAST);
> + my_init_dynamic_array(&installed_audit_plugins, sizeof(void*), 16, 16);
> +}
> +
> +
> +/**
> + Finalize Audit global variables
> +*/
> +
> +void mysql_audit_finalize()
> +{
> + delete_dynamic(&installed_audit_plugins);
> + (void) pthread_mutex_destroy(&LOCK_audit);
> +}
> +
> +
> +/**
> + Initialize an Audit plug-in
> +
> + @param[in] plugin
> +
> + @retval FALSE OK
> + @retval TRUE There was an error.
> +*/
> +
> +int initialize_audit_plugin(st_plugin_int *plugin)
> +{
> + st_mysql_audit *data= (st_mysql_audit*) plugin->plugin->info;
> + int rc;
> +
> + if (!data->class_mask || !data->event_notify ||
> + !data->class_mask[0])
> + {
> + sql_print_error("Plugin '%s' has invalid data.",
> + plugin->name.str);
> + return 1;
> + }
> +
> + if (plugin->plugin->init && plugin->plugin->init(NULL))
> + {
> + sql_print_error("Plugin '%s' init function returned error.",
> + plugin->name.str);
> + return 1;
> + }
> +
> + /* Make the interface info more easily accessible */
> + plugin->data= plugin->plugin->info;
> +
> + pthread_mutex_lock(&LOCK_audit);
> +
> + rc= insert_dynamic(&installed_audit_plugins, (uchar*) &plugin->data);
> +
> + /* Add the bits the plugin is interested in to the global mask */
> + mysql_global_audit_mask1|= data->class_mask[0];
> +
> + pthread_mutex_unlock(&LOCK_audit);
> +
> + return rc;
> +}
> +
> +
> +/**
> + Finalize an Audit plug-in
> +
> + @param[in] plugin
> +
> + @retval FALSE OK
> + @retval TRUE There was an error.
> +*/
> +int finalize_audit_plugin(st_plugin_int *plugin)
> +{
> + uint64 event_class_mask1= 0;
> + st_mysql_audit **data, **data_last;
> + uint index, index_to_delete= (uint) -1;
> +
> + if (plugin->plugin->deinit && plugin->plugin->deinit(NULL))
> + {
> + DBUG_PRINT("warning", ("Plugin '%s' deinit function returned error.",
> + plugin->name.str));
> + DBUG_EXECUTE("finalize_audit_plugin", return 1; );
> + }
> +
> + pthread_mutex_lock(&LOCK_audit);
> +
> + /* Iterate through all the installed plugins to create new mask */
> + data= (st_mysql_audit**) installed_audit_plugins.buffer;
> + data_last= data + installed_audit_plugins.elements;
> + for (index= 0; data < data_last; data++, index++)
> + {
> + if (plugin->data == *data)
> + index_to_delete= index;
> + else
> + event_class_mask1|= (*data)->class_mask[0];
> + }
> +
> + /* Set the global audit mask */
> + mysql_global_audit_mask1= event_class_mask1;
> +
> + /* Remove this plugin from the list */
> + if (likely(index_to_delete != (uint) -1))
> + delete_dynamic_element(&installed_audit_plugins, index_to_delete);
> + else
> + {
> + sql_print_error("Plugin '%s' was not found in installed_audit_plugins",
> + plugin->name.str);
> + }
> +
> + pthread_mutex_unlock(&LOCK_audit);
> +
> + return 0;
> +}
> +
> +
> +/**
> + Distributes an audit event to plug-ins
> +
> + @param[in] thd
> + @param[in] event
> +*/
> +
> +static void event_class_dispatch(THD *thd, const struct mysql_event *event)
> +{
> + uint64 event_class_mask1= (1 << event->event_class);
> +
> + if (thd)
> + {
> + plugin_ref *plugins, *plugins_last;
> +
> + /* Use the cached set of audit plugins */
> + plugins= (plugin_ref*) thd->audit_class_plugins.buffer;
> + plugins_last= plugins + thd->audit_class_plugins.elements;
> + for (; plugins < plugins_last; plugins++)
> + {
> + st_mysql_audit *data= plugin_data(*plugins, struct st_mysql_audit *);
> +
> + /* Check to see if the plugin is interested in this event */
> + if (!(data->class_mask[0] & event_class_mask1))
> + continue;
> +
> + /* Actually notify the plugin */
> + data->event_notify(thd, event);
> + }
> + }
> + else
> + {
> + st_mysql_audit **data, **data_last;
> +
> + pthread_mutex_lock(&LOCK_audit);
> +
> + /* Use our own list of all audit plugins */
> + data= (st_mysql_audit**) installed_audit_plugins.buffer;
> + data_last= data + installed_audit_plugins.elements;
> + for (; data < data_last; data++)
> + {
> + if (!((*data)->class_mask[0] & event_class_mask1))
> + continue;
> +
> + /* Actually notify the plugin */
> + (*data)->event_notify(0, event);
> + }
> +
> + pthread_mutex_unlock(&LOCK_audit);
> + }
> +}
> +
> +
> +
> +
> +#else /* EMBEDDED_LIBRARY */
> +
> +void mysql_audit_initialize()
> +{
> +}
> +
> +
> +void mysql_audit_finalize()
> +{
> +}
> +
> +
> +int initialize_audit_plugin(st_plugin_int *plugin)
> +{
> + return 1;
> +}
> +
> +int finalize_audit_plugin(st_plugin_int *plugin)
> +{
> + return 0;
> +}
> +
> +#endif /* EMBEDDED_LIBRARY */
> diff -Nrup a/sql/sql_plugin.cc b/sql/sql_plugin.cc
> --- a/sql/sql_plugin.cc 2007-11-21 12:19:04 -08:00
> +++ b/sql/sql_plugin.cc 2007-11-29 14:40:18 -08:00
> @@ -174,7 +186,8 @@ public:
> sys_var_pluginvar *cast_pluginvar() { return this; }
> bool is_readonly() const { return plugin_var->flags & PLUGIN_VAR_READONLY;
> }
> bool check_type(enum_var_type type)
> - { return !(plugin_var->flags & PLUGIN_VAR_THDLOCAL) && type !=
> OPT_GLOBAL; }
> + { return !(plugin_var->flags & PLUGIN_VAR_THDLOCAL) ? (type != OPT_GLOBAL)
> + : ((type == OPT_GLOBAL) && (plugin_var->flags &
> PLUGIN_VAR_NOGLOBAL)); }
no PLUGIN_VAR_NOGLOBAL
> bool check_update_type(Item_result type);
> SHOW_TYPE show_type();
> uchar* real_value_ptr(THD *thd, enum_var_type type);
> @@ -330,6 +343,11 @@ static inline void free_plugin_mem(struc
> my_free((uchar*)p->plugins, MYF(MY_ALLOW_ZERO_PTR));
> }
>
> +extern "C" char *mysqld_plugin_path(char *tgt, unsigned int tgt_size,
> + const char *name)
> +{
> + return strxnmov(tgt, tgt_size - 1, opt_plugin_dir, "/", name, NullS);
> +}
What is that ?
> static st_plugin_dl *plugin_dl_add(const LEX_STRING *dl, int report)
> {
> @@ -346,7 +364,8 @@ static st_plugin_dl *plugin_dl_add(const
> plugin directory are used (to make this even remotely secure).
> */
> if (my_strchr(files_charset_info, dl->str, dl->str + dl->length,
> FN_LIBCHAR) ||
> - check_identifier_name((LEX_STRING *) dl) ||
> + check_string_char_length((LEX_STRING *) dl, "", NAME_CHAR_LEN,
indeed
> + system_charset_info, 1) ||
> plugin_dir_len + dl->length + 1 >= FN_REFLEN)
> {
> if (report & REPORT_TO_USER)
> @@ -1179,9 +1198,7 @@ int plugin_init(int *argc, char **argv,
> /* Register all dynamic plugins */
> if (!(flags & PLUGIN_INIT_SKIP_DYNAMIC_LOADING))
> {
> - if (opt_plugin_load &&
> - plugin_load_list(&tmp_root, argc, argv, opt_plugin_load))
> - goto err;
> + plugin_load_list(&tmp_root, argc, argv, opt_plugin_load);
what was wrong with plugin_load_list ?
> if (!(flags & PLUGIN_INIT_SKIP_PLUGIN_TABLE))
> plugin_load(&tmp_root, argc, argv);
> }
> @@ -1310,27 +1327,23 @@ end:
> */
> static void plugin_load(MEM_ROOT *tmp_root, int *argc, char **argv)
> {
> + THD thd;
> TABLE_LIST tables;
> TABLE *table;
> READ_RECORD read_record_info;
> int error;
> - THD *new_thd;
> + THD *new_thd= &thd;
why ?
> #ifdef EMBEDDED_LIBRARY
> bool table_exists;
> #endif /* EMBEDDED_LIBRARY */
> DBUG_ENTER("plugin_load");
>
> - if (!(new_thd= new THD))
> - {
> - sql_print_error("Can't allocate memory for plugin structures");
> - delete new_thd;
> - DBUG_VOID_RETURN;
> - }
> new_thd->thread_stack= (char*) &tables;
> new_thd->store_globals();
> lex_start(new_thd);
> new_thd->db= my_strdup("mysql", MYF(0));
> new_thd->db_length= 5;
> + bzero((char*) &thd.net, sizeof(thd.net));
> bzero((uchar*)&tables, sizeof(tables));
> tables.alias= tables.table_name= (char*)"plugin";
> tables.lock_type= TL_READ;
> @@ -1406,11 +1418,15 @@ static bool plugin_load_list(MEM_ROOT *t
> struct st_plugin_dl *plugin_dl;
by the way, please remove argument names from the forward declaration of
plugin_load_list:
-static bool plugin_load_list(MEM_ROOT *tmp_root, int *argc, char **argv,
- const char *list);
+static bool plugin_load_list(MEM_ROOT *, int *, char **, const char *);
> struct st_mysql_plugin *plugin;
> char *p= buffer;
> + bool result= FALSE;
> DBUG_ENTER("plugin_load_list");
> while (list)
> {
> if (p == buffer + sizeof(buffer) - 1)
> + {
> + sql_print_warning("plugin-load argument too long");
> break;
> + }
> switch ((*(p++)= *(list++))) {
> case '\0':
> list= NULL; /* terminate the loop */
> @@ -1420,6 +1436,7 @@ static bool plugin_load_list(MEM_ROOT *t
> #endif
> case ';':
> name.str[name.length]= '\0';
> + pthread_mutex_lock(&LOCK_plugin);
why ? safe_mutex_assert_owner()'s ?
why not to lock once in the beginning on the function and unlock on return ?
some other code that expects the mutex to be NOT locked ?
> if (str != &dl) // load all plugins in named module
> {
> dl= name;
> @@ -1459,12 +1483,12 @@ static bool plugin_load_list(MEM_ROOT *t
> str->length++;
> continue;
> }
> - }
> - DBUG_RETURN(FALSE);
> error:
> - sql_print_error("Couldn't load plugin named '%s' with soname '%s'.",
> - name.str, dl.str);
> - DBUG_RETURN(TRUE);
> + sql_print_warning("Couldn't load plugin named '%s' with soname '%s'.",
> + name.str, dl.str);
> + result= TRUE;
> + }
> + DBUG_RETURN(result);
why do you return a value if you ignore it in the caller ?
> }
>
>
> @@ -1625,16 +1649,28 @@ bool mysql_install_plugin(THD *thd, cons
> DBUG_RETURN(TRUE);
>
> pthread_mutex_lock(&LOCK_plugin);
> - rw_wrlock(&LOCK_system_variables_hash);
> +
> + table->use_all_columns();
> + restore_record(table, s->default_values);
> + table->field[0]->store(name->str, name->length, system_charset_info);
> + table->field[1]->store(dl->str, dl->length, files_charset_info);
> + error= table->file->ha_write_row(table->record[0]);
> + if (error)
> + {
> + table->file->print_error(error, MYF(0));
> + goto err;
> + }
why ?
> /* handle_options() assumes arg0 (program name) always exists */
> argv[0]= const_cast<char*>(""); // without a cast gcc emits a warning
> argv[1]= 0;
> argc= 1;
> + rw_wrlock(&LOCK_system_variables_hash);
> error= plugin_add(thd->mem_root, name, dl, &argc, argv, REPORT_TO_USER);
> rw_unlock(&LOCK_system_variables_hash);
>
> if (error || !(tmp= plugin_find_internal(name, MYSQL_ANY_PLUGIN)))
> - goto err;
> + goto deinit;
>
> if (plugin_initialize(tmp))
> {
>
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
