Re: bzr commit into mysql-6.0-rpl branch (alfranio.correia:2847) WL#4828

[Davi Arnaut]

Hi Alfranio,

Looks good! A few minor comments below...

On 5/8/09 3:43 PM, Alfranio Correia wrote:
> #At
> file:///home/acorreia/workspace.sun/repository.mysql/bzrwork/wl-4828/mysql-6.0-rpl/ based
> on revid:zhenxing.he@stripped
>
>   2847 Alfranio Correia	2009-05-08
>        WL#4828
>
>      modified:
>        dbug/dbug.c
>        include/my_dbug.h
>        sql/mysql_priv.h
>        sql/sql_class.cc
>        sql/sql_class.h
>        sql/sql_partition.cc
> === modified file 'dbug/dbug.c'
> --- a/dbug/dbug.c	2009-03-25 16:51:28 +0000
> +++ b/dbug/dbug.c	2009-05-08 18:43:13 +0000
> @@ -1836,6 +1836,38 @@ BOOLEAN _db_keyword_(CODE_STATE *cs, con
>   /*
>    *  FUNCTION
>    *
> + *      _db_keywords_    test keyword formed by a set of strings for member
> + *                       of keyword list
> + *
> + *  DESCRIPTION
> + *
> + *      This function is similar to _db_keyword but receives a set of strings to
> + *      be concatenated in order to make the keyword to be compared.
> + */
> +
> +BOOLEAN _db_keywords_(CODE_STATE *cs, int strict, ...)
> +{
> +  char *src;
> +  va_list ap;
> +  int pos= 0;
> +  int size= 0;
> +  char dest[_MAX_FUNC_NAME_];
> +
> +  va_start(ap, strict);
> +  while ((src = va_arg(ap, char *))&&  size<  _MAX_FUNC_NAME_)
> +  {
> +    for (pos= 0; (size + pos)<  _MAX_FUNC_NAME_&&  src[pos] != '\0' ;
> pos++)
> +      dest[size + pos] = src[pos];
> +    size= size + pos;
> +  }
> +  dest[size + pos] = '\0';
> +  va_end(ap);

Use strxnmov or similar. strings/libmystrings gets linked to dbug.

> +  return _db_keyword_(cs, dest, strict);
> +}
> +
> +/*
> + *  FUNCTION
> + *
>    *      Indent    indent a line to the given indentation level
>    *
>    *  SYNOPSIS
> @@ -2460,6 +2492,13 @@ void _db_unlock_file_()
>     pthread_mutex_unlock(&THR_LOCK_dbug);
>   }
>
> +const char* _db_get_func_(void)
> +{
> +  CODE_STATE *cs;
> +  get_code_state_or_return(0);

get_code_state_or_return NULL;

> +  return cs->func;
> +}
> +
>   /*
>    * Here we need the definitions of the clock routine.  Add your
>    * own for whatever system that you have.
>
> === modified file 'include/my_dbug.h'
> --- a/include/my_dbug.h	2008-12-10 17:04:38 +0000
> +++ b/include/my_dbug.h	2009-05-08 18:43:13 +0000
> @@ -31,6 +31,7 @@ struct _db_stack_frame_ {
>   struct  _db_code_state_;
>   extern  my_bool _dbug_on_;
>   extern  my_bool _db_keyword_(struct _db_code_state_ *, const char *, int);
> +extern  my_bool _VARARGS(_db_keywords_(struct _db_code_state_ *, int, ...));
>   extern  int _db_explain_(struct _db_code_state_ *cs, char *buf, size_t len);
>   extern  int _db_explain_init_(char *buf, size_t len);
>   extern	int _db_is_pushed_(void);
> @@ -54,6 +55,7 @@ extern  void _db_lock_file_(void);
>   extern  void _db_unlock_file_(void);
>   extern  FILE *_db_fp_(void);
>   extern  void _db_flush_();
> +extern  const char* _db_get_func_(void);
>
>   #define DBUG_ENTER(a) struct _db_stack_frame_ _db_stack_frame_; \
>           _db_enter_ (a,__FILE__,__LINE__,&_db_stack_frame_)
> @@ -101,6 +103,20 @@ extern  void _db_flush_();
>                        (void)_CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDERR),\
>                        _exit(3))
>   #endif
> +#define _MAX_FUNC_NAME_ 255
> +#define CHECK_CRASH(func, op) \

Prefix it with DBUG_

> +        do { \
> +          if (_db_keywords_(0, 0, (func), (op), 0)) \
> +            { (_db_flush_(), abort()); } \
> +        } while (0)
> +#define DBUG_CRASH_ENTER(func) \
> +  DBUG_ENTER(func); CHECK_CRASH(func, "_crash_enter")
> +#define DBUG_CRASH_RETURN(val) \
> +  do {CHECK_CRASH (_db_get_func_(), "_crash_return"); \
> +    DBUG_RETURN(val);} while(0)
> +#define DBUG_CRASH_VOID_RETURN \
> +  do {CHECK_CRASH (_db_get_func_(), "_crash_return"); \
> +    DBUG_VOID_RETURN;} while(0)
>
>   #else                                           /* No debugger */
>
> @@ -132,6 +148,9 @@ extern  void _db_flush_();
>   #define DEBUGGER_ON                     do { } while(0)
>   #define IF_DBUG(A)
>   #define DBUG_ABORT()                    abort()
> +#define DBUG_CRASH_ENTER(func)
> +#define DBUG_CRASH_RETURN(val)          do { return(val); } while(0)
> +#define DBUG_CRASH_VOID_RETURN          do { return; } while(0)
>
>   #endif
>   #ifdef  __cplusplus
>
> === modified file 'sql/mysql_priv.h'
> --- a/sql/mysql_priv.h	2009-04-30 14:35:36 +0000
> +++ b/sql/mysql_priv.h	2009-05-08 18:43:13 +0000
> @@ -977,100 +977,6 @@ struct Query_cache_query_flags
>   #define query_cache_is_cacheable_query(L) 0
>   #endif /*HAVE_QUERY_CACHE*/
>
> -/*
> -  Error injector Macros to enable easy testing of recovery after failures
> -  in various error cases.
> -*/
> -#ifndef ERROR_INJECT_SUPPORT
> -
> -#define ERROR_INJECT(x) 0
> -#define ERROR_INJECT_ACTION(x,action) 0
> -#define ERROR_INJECT_CRASH(x) 0
> -#define ERROR_INJECT_VALUE(x) 0
> -#define ERROR_INJECT_VALUE_ACTION(x,action) 0
> -#define ERROR_INJECT_VALUE_CRASH(x) 0
> -#define SET_ERROR_INJECT_VALUE(x)
> -
> -#else
> -
> -inline bool check_and_unset_keyword(const char *dbug_str)
> -{
> -  const char *extra_str= "-d,";
> -  char total_str[200];
> -  if (_db_keyword_ (0, dbug_str, 1))
> -  {
> -    strxmov(total_str, extra_str, dbug_str, NullS);
> -    DBUG_SET(total_str);
> -    return 1;
> -  }
> -  return 0;
> -}
> -
> -
> -inline bool
> -check_and_unset_inject_value(int value)
> -{
> -  THD *thd= current_thd;
> -  if (thd->error_inject_value == (uint)value)
> -  {
> -    thd->error_inject_value= 0;
> -    return 1;
> -  }
> -  return 0;
> -}
> -
> -/*
> -  ERROR INJECT MODULE:
> -  --------------------
> -  These macros are used to insert macros from the application code.
> -  The event that activates those error injections can be activated
> -  from SQL by using:
> -  SET SESSION dbug=+d,code;
> -
> -  After the error has been injected, the macros will automatically
> -  remove the debug code, thus similar to using:
> -  SET SESSION dbug=-d,code
> -  from SQL.
> -
> -  ERROR_INJECT_CRASH will inject a crash of the MySQL Server if code
> -  is set when macro is called. ERROR_INJECT_CRASH can be used in
> -  if-statements, it will always return FALSE unless of course it
> -  crashes in which case it doesn't return at all.
> -
> -  ERROR_INJECT_ACTION will inject the action specified in the action
> -  parameter of the macro, before performing the action the code will
> -  be removed such that no more events occur. ERROR_INJECT_ACTION
> -  can also be used in if-statements and always returns FALSE.
> -  ERROR_INJECT can be used in a normal if-statement, where the action
> -  part is performed in the if-block. The macro returns TRUE if the
> -  error was activated and otherwise returns FALSE. If activated the
> -  code is removed.
> -
> -  Sometimes it is necessary to perform error inject actions as a serie
> -  of events. In this case one can use one variable on the THD object.
> -  Thus one sets this value by using e.g. SET_ERROR_INJECT_VALUE(100).
> -  Then one can later test for it by using ERROR_INJECT_CRASH_VALUE,
> -  ERROR_INJECT_ACTION_VALUE and ERROR_INJECT_VALUE. This have the same
> -  behaviour as the above described macros except that they use the
> -  error inject value instead of a code used by DBUG macros.
> -*/
> -#define SET_ERROR_INJECT_VALUE(x) \
> -  current_thd->error_inject_value= (x)
> -#define ERROR_INJECT_CRASH(code) \
> -  DBUG_EVALUATE_IF(code, (DBUG_ABORT(), 0), 0)
> -#define ERROR_INJECT_ACTION(code, action) \
> -  (check_and_unset_keyword(code) ? ((action), 0) : 0)
> -#define ERROR_INJECT(code) \
> -  check_and_unset_keyword(code)
> -#define ERROR_INJECT_VALUE(value) \
> -  check_and_unset_inject_value(value)
> -#define ERROR_INJECT_VALUE_ACTION(value,action) \
> -  (check_and_unset_inject_value(value) ? (action) : 0)
> -#define ERROR_INJECT_VALUE_CRASH(value) \
> -  ERROR_INJECT_VALUE_ACTION(value, (DBUG_ABORT(), 0))
> -
> -#endif
> -
>   void write_bin_log(THD *thd, bool clear_error,
>                      char const *query, ulong query_length);
>
>
> === modified file 'sql/sql_class.cc'
> --- a/sql/sql_class.cc	2009-05-07 11:44:40 +0000
> +++ b/sql/sql_class.cc	2009-05-08 18:43:13 +0000
> @@ -493,9 +493,6 @@ THD::THD()
>     limit_found_rows= 0;
>     row_count_func= -1;
>     statement_id_counter= 0UL;
> -#ifdef ERROR_INJECT_SUPPORT
> -  error_inject_value= 0UL;
> -#endif
>     // Must be reset to handle error with THD's created for init of mysqld
>     lex->current_select= 0;
>     start_time=(time_t) 0;
>
> === modified file 'sql/sql_class.h'
> --- a/sql/sql_class.h	2009-05-08 07:49:21 +0000
> +++ b/sql/sql_class.h	2009-05-08 18:43:13 +0000
> @@ -1696,9 +1696,6 @@ public:
>     query_id_t query_id;
>     ulong      col_access;
>
> -#ifdef ERROR_INJECT_SUPPORT
> -  ulong      error_inject_value;
> -#endif
>     /* Statement id is thread-wide. This counter is used to generate ids */
>     ulong      statement_id_counter;
>     ulong	     rand_saved_seed1, rand_saved_seed2;
>
> === modified file 'sql/sql_partition.cc'
> --- a/sql/sql_partition.cc	2009-04-01 09:15:50 +0000
> +++ b/sql/sql_partition.cc	2009-05-08 18:43:13 +0000
> @@ -6140,28 +6140,28 @@ uint fast_alter_partition_table(THD *thd
>         to test if recovery is properly done.
>       */
>       if (write_log_drop_shadow_frm(lpt) ||
> -        ERROR_INJECT_CRASH("crash_drop_partition_1") ||
> +        DBUG_EVALUATE_IF("crash_drop_partition_1", (DBUG_ABORT(), 0), 0) ||

It might be better to keep the define local to this file. Something like:

#define INJECT_DBUG_CRASH(str) DBUG_EVALUATE_IF(str, (DBUG_ABORT(), 0), 0)

[..]

Regards,

-- Davi Arnaut