maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Holyfoot!

Mostly ok.
There are few comments, see below:

On Sep 03, holyfoot@xxxxxxxxxxxx wrote:
> ------------------------------------------------------------
> revno: 3876
> revision-id: holyfoot@xxxxxxxxxxxx-20130903082521-xpia2wond6oggpsk
> parent: igor@xxxxxxxxxxxx-20130831163309-vqtls4oubhvz9zcz
> committer: Alexey Botchkov <holyfoot@xxxxxxxxxxxx>
> branch nick: 55-4472
> timestamp: Tue 2013-09-03 13:25:21 +0500
> message:
>   MDEV-4472 Audit-plugin.  Server-related part of the task.
>           file_logger became the service.
>           Data like query_id now are sent to the audit plugin.
>           Fix for MDEV-4770 ported from 10.0.
>           Fix added for the read_maria_plugin_info().

> === modified file 'include/mysql/plugin_audit.h'
> --- a/include/mysql/plugin_audit.h	2013-04-19 10:50:16 +0000
> +++ b/include/mysql/plugin_audit.h	2013-09-03 08:25:21 +0000
> @@ -25,7 +25,7 @@
>  
>  #define MYSQL_AUDIT_CLASS_MASK_SIZE 1
>  
> -#define MYSQL_AUDIT_INTERFACE_VERSION 0x0301
> +#define MYSQL_AUDIT_INTERFACE_VERSION 0x0302
>  
>  
>  /*************************************************************************
> @@ -59,6 +59,10 @@ struct mysql_event_general
>    struct charset_info_st *general_charset;
>    unsigned long long general_time;
>    unsigned long long general_rows;
> +  /* Added for 0x302 */

A bit confusing. Better "Added in version 0x0302"

> +  long long query_id;

should it rather be unsigned?

> +  const char *database;
> +  unsigned int database_length;
>  };
>  
>  
> @@ -140,6 +144,8 @@ struct mysql_event_table
>    unsigned int new_database_length;
>    const char *new_table;
>    unsigned int new_table_length;
> +  /* Added for 0x302 */
> +  long long query_id;
>  };
>  
>  /*************************************************************************
> 
> === renamed file 'plugin/sql_errlog/sql_logger.cc' => 'mysys/file_logger.c'
> --- a/plugin/sql_errlog/sql_logger.cc	2013-06-22 12:02:03 +0000
> +++ b/mysys/file_logger.c	2013-09-03 08:25:21 +0000
> @@ -51,6 +53,12 @@ LOGGER_HANDLE *logger_open(const char *p
>                             unsigned int rotations)
>  {
>    LOGGER_HANDLE new_log, *l_perm;
> +  char *local_mysql_data_home;
> +#ifdef _WIN32
> +  local_mysql_data_home= (char *)GetProcAddress(0, "mysql_data_home");
> +#else
> +  local_mysql_data_home= mysql_data_home;
> +#endif /*_WIN32*/

You shouldn't need it, file_logger.c is part of the server, not a
dynamically loaded plugin.

>  
>    /*
>      I don't think we ever need more rotations,
> === modified file 'sql/sql_audit.h'
> --- a/sql/sql_audit.h	2013-04-19 10:50:16 +0000
> +++ b/sql/sql_audit.h	2013-09-03 08:25:21 +0000
> @@ -93,7 +93,8 @@ void mysql_audit_general_log(THD *thd, t
>  
>      mysql_audit_notify(thd, MYSQL_AUDIT_GENERAL_CLASS, MYSQL_AUDIT_GENERAL_LOG,
>                         0, time, user, userlen, cmd, cmdlen,
> -                       query, querylen, clientcs, 0);
> +                       query, querylen, clientcs, (ha_rows) 0,
> +                       thd->db, thd->db_length);

no query_id?

>    }
>  }
>  
> @@ -139,7 +140,8 @@ void mysql_audit_general(THD *thd, uint 
>  
>      mysql_audit_notify(thd, MYSQL_AUDIT_GENERAL_CLASS, event_subtype,
>                         error_code, time, user, userlen, msg, msglen,
> -                       query.str(), query.length(), query.charset(), rows);
> +                       query.str(), query.length(), query.charset(), rows,
> +                       thd->db, thd->db_length);

no query_id?

>    }
>  }
>  
> === modified file 'sql/sql_plugin.cc'
> --- a/sql/sql_plugin.cc	2013-06-13 18:19:11 +0000
> +++ b/sql/sql_plugin.cc	2013-09-03 08:25:21 +0000
> @@ -2645,13 +2647,16 @@ static void update_func_longlong(THD *th
>  static void update_func_str(THD *thd, struct st_mysql_sys_var *var,
>                               void *tgt, const void *save)
>  {
> -  char *old= *(char **) tgt;
> -  *(char **)tgt= *(char **) save;
> +  char *value= *(char**) save;
>    if (var->flags & PLUGIN_VAR_MEMALLOC)
>    {
> -    *(char **)tgt= my_strdup(*(char **) save, MYF(0));
> +    char *old= *(char**) tgt;
> +    if (value)
> +      *(char**) tgt= my_strdup(value, MYF(0));

add

       else
         *(char**) tgt= 0;

>      my_free(old);
>    }
> +  else
> +    *(char**) tgt= value;
>  }
>  
Regards,
Sergei