Bug #26981 param.op_name not updated?
Submitted: 9 Mar 2007 1:10 Modified: 12 Dec 2007 14:32
Reporter: Chongfeng Hu Email Updates:
Status: Not a Bug Impact on me:
None 
Category:MySQL Server: MyISAM storage engine Severity:S2 (Serious)
Version:5.1.17-BK, 5.2.0 falcon-alpha OS:Any (any)
Assigned to: Sergey Vojtovich CPU Architecture:Any
Tags: MI_CHECK, op_name

[9 Mar 2007 1:10] Chongfeng Hu
Description:
In file storage/myisam/ha_myisam.cc, there are many functions for different operations, for example, check(), restore(), analyze(), backup(), optimize() and repair(). If they are to modify a variable param with type MI_CHECK, they will all assign param.op_name the corresponding operation name string ("check",..), but repair() is the exception. In function int ha_myisam::repair(THD *thd, MI_CHECK &param, bool optimize), param.op_name is not updated. Because of the unification, I suspect that it should also be updated, otherwise it will be a random char * pointer, and can cause serious problems when dereferenced.

  param.db_name=    table->s->db.str;
  param.table_name= table->alias;
  param.tmpfile_createflag = O_RDWR | O_TRUNC;
  param.using_global_keycache = 1;
  param.thd= thd;
  param.tmpdir= &mysql_tmpdir_list;
  param.out_flag= 0;
  strmov(fixed_name,file->filename);

How to repeat:
I found this suspicious spot with the help of a code-checking tool.

Suggested fix:
add:

param.op_name = "repair";
[9 Mar 2007 11:54] Valeriy Kravchuk
Thank you for a problem report. Please, send URL you had used to get that source code. In both latest 5.1.17-BK and 5.1-flacon trees we have:

int ha_myisam::repair(THD* thd, HA_CHECK_OPT *check_opt)
{
  int error;
  MI_CHECK param;
  ha_rows start_records;

  if (!file) return HA_ADMIN_INTERNAL_ERROR;

  myisamchk_init(&param);
  param.thd = thd;
  param.op_name=  "repair";
  param.testflag= ((check_opt->flags & ~(T_EXTEND)) |
                   T_SILENT | T_FORCE_CREATE | T_CALC_CHECKSUM |
                   (check_opt->flags & T_EXTEND ? T_REP : T_REP_BY_SORT));
...

So, param.op_name is initiated as in other ha_mysiam methods.
[10 Mar 2007 22:37] Chongfeng Hu
I checked code in my local directory. Actually, in storage/myisam/ha_myisam.cc, there are two repair()s, one is defined as:

int ha_myisam::repair(THD *thd, MI_CHECK &param, bool optimize)

the other:

int ha_myisam::repair(THD* thd, HA_CHECK_OPT *check_opt)

the suspicious spot is in the first function. Please check that.
[12 Mar 2007 11:14] Valeriy Kravchuk
Sorry, I missed correct overloaded function to check. Verified just as described on latest 5.1.17-BK:

int ha_myisam::repair(THD *thd, MI_CHECK &param, bool do_optimize)
{
  int error=0;
  uint local_testflag=param.testflag;
  bool optimize_done= !do_optimize, statistics_done=0;
  const char *old_proc_info=thd->proc_info;
  char fixed_name[FN_REFLEN];
  MYISAM_SHARE* share = file->s;
  ha_rows rows= file->state->records;
  DBUG_ENTER("ha_myisam::repair");

  param.db_name=    table->s->db.str;
  param.table_name= table->alias;
  param.tmpfile_createflag = O_RDWR | O_TRUNC;
  param.using_global_keycache = 1;
  param.thd= thd;
  param.tmpdir= &mysql_tmpdir_list;
  param.out_flag= 0;
  strmov(fixed_name,file->filename);
...
[16 Nov 2007 5:55] terry tao
ha_myisam::repair(THD *thd, MI_CHECK &param, bool do_optimize) is called by other function,
ha_myisam::repair(THD* thd, HA_CHECK_OPT *check_opt)
ha_myisam::optimize(THD* thd, HA_CHECK_OPT *check_opt)
ha_myisam::enable_indexes(uint mode)
in each of these function the param.op_name will be assign  "repair","optimize","recreating_index",so it is not to assign at this time?
[12 Dec 2007 14:32] Sergey Vojtovich
"int ha_myisam::repair(THD *thd, MI_CHECK &param, bool optimize)" is a private MyISAM function. A caller function must set param.op_name.

Closing as "Not a bug".