bk commit into 4.1 tree (svoj:1.2688) BUG#31277

[Sergey Vojtovich]

Below is the list of changes that have just been committed into a local
4.1 repository of svoj. When svoj does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-10-30 14:36:37+04:00, svoj@stripped +4 -0
  BUG#31277 - myisamchk --unpack corrupts a table
  
  With certain data sets (when compressed record length gets bigger than
  uncompressed) myisamchk --unpack may corrupt data file.
  
  Fixed that record length was wrongly restored from compressed table.
  
  No test case for this fix, as currently our test suite doesn't support
  execution of myisampack tool.

  myisam/mi_check.c@stripped, 2007-10-30 14:36:35+04:00, svoj@stripped +21 -18
    With compressed tables compressed record length may be bigger than
    pack_reclength, thus we may allocate insufficient memory for record
    buffer.
    
    Let single function allocate record buffer, performing needed record
    length calculations.
    
    Still, it is not doable with parallel repair, as it allocates needed
    record buffers at once. For parellel repair added better record length
    calculation.

  myisam/mi_open.c@stripped, 2007-10-30 14:36:35+04:00, svoj@stripped +5 -2
    When calculating record buffer size, take into account that compressed
    record length may be bigger than uncompressed.

  myisam/mi_packrec.c@stripped, 2007-10-30 14:36:35+04:00, svoj@stripped +0 -1
    With certain data set share->max_pack_length (compressed record length)
    may be bigger than share->base.pack_reclength (packed record length).
    
    set_if_bigger(pack_reclength, max_pack_length) in this case causes
    myisamchk --unpack to write extra garbage, whereas pack_reclength
    remains the same in new index file. As a result we get unreadable
    table.

  myisam/myisamchk.c@stripped, 2007-10-30 14:36:35+04:00, svoj@stripped +4 -3
    With compressed tables compressed record length may be bigger than
    pack_reclength, thus we may allocate insufficient memory for record
    buffer.
    
    Let single function allocate record buffer, performing needed record
    length calculations.

diff -Nrup a/myisam/mi_check.c b/myisam/mi_check.c
--- a/myisam/mi_check.c	2007-05-16 23:42:31 +05:00
+++ b/myisam/mi_check.c	2007-10-30 14:36:35 +04:00
@@ -940,7 +940,7 @@ int chk_data_link(MI_CHECK *param, MI_IN
   ha_rows records,del_blocks;
   my_off_t used,empty,pos,splits,start_recpos,
 	   del_length,link_used,start_block;
-  byte	*record,*to;
+  byte	*record= 0, *to;
   char llbuff[22],llbuff2[22],llbuff3[22];
   ha_checksum intern_record_checksum;
   ha_checksum key_checksum[MI_MAX_POSSIBLE_KEY];
@@ -957,7 +957,7 @@ int chk_data_link(MI_CHECK *param, MI_IN
       puts("- check record links");
   }
 
-  if (!(record= (byte*) my_malloc(info->s->base.pack_reclength,MYF(0))))
+  if (!mi_alloc_rec_buff(info, -1, &record))
   {
     mi_check_print_error(param,"Not enough memory for record");
     DBUG_RETURN(-1);
@@ -1364,12 +1364,12 @@ int chk_data_link(MI_CHECK *param, MI_IN
     printf("Lost space:   %12s    Linkdata:     %10s\n",
 	   llstr(empty,llbuff),llstr(link_used,llbuff2));
   }
-  my_free((gptr) record,MYF(0));
+  my_free(mi_get_rec_buff_ptr(info, record), MYF(0));
   DBUG_RETURN (error);
  err:
   mi_check_print_error(param,"got error: %d when reading datafile at record:
%s",my_errno, llstr(records,llbuff));
  err2:
-  my_free((gptr) record,MYF(0));
+  my_free(mi_get_rec_buff_ptr(info, record), MYF(0));
   param->testflag|=T_RETRY_WITHOUT_QUICK;
   DBUG_RETURN(1);
 } /* chk_data_link */
@@ -1428,8 +1428,7 @@ int mi_repair(MI_CHECK *param, register 
 		      MYF(MY_WME | MY_WAIT_IF_FULL)))
       goto err;
   info->opt_flag|=WRITE_CACHE_USED;
-  if (!(sort_param.record=(byte*) my_malloc((uint) share->base.pack_reclength,
-					   MYF(0))) ||
+  if (!mi_alloc_rec_buff(info, -1, &sort_param.record) ||
       !mi_alloc_rec_buff(info, -1, &sort_param.rec_buff))
   {
     mi_check_print_error(param, "Not enough memory for extra record");
@@ -1631,7 +1630,8 @@ err:
   }
   my_free(mi_get_rec_buff_ptr(info, sort_param.rec_buff),
                             MYF(MY_ALLOW_ZERO_PTR));
-  my_free(sort_param.record,MYF(MY_ALLOW_ZERO_PTR));
+  my_free(mi_get_rec_buff_ptr(info, sort_param.record),
+          MYF(MY_ALLOW_ZERO_PTR));
   my_free(sort_info.buff,MYF(MY_ALLOW_ZERO_PTR));
   VOID(end_io_cache(&param->read_cache));
   info->opt_flag&= ~(READ_CACHE_USED | WRITE_CACHE_USED);
@@ -2129,8 +2129,7 @@ int mi_repair_by_sort(MI_CHECK *param, r
   info->opt_flag|=WRITE_CACHE_USED;
   info->rec_cache.file=info->dfile;		/* for sort_delete_record */
 
-  if (!(sort_param.record=(byte*) my_malloc((uint) share->base.pack_reclength,
-					   MYF(0))) ||
+  if (!mi_alloc_rec_buff(info, -1, &sort_param.record) ||
       !mi_alloc_rec_buff(info, -1, &sort_param.rec_buff))
   {
     mi_check_print_error(param, "Not enough memory for extra record");
@@ -2415,7 +2414,8 @@ err:
 
   my_free(mi_get_rec_buff_ptr(info, sort_param.rec_buff),
                             MYF(MY_ALLOW_ZERO_PTR));
-  my_free(sort_param.record,MYF(MY_ALLOW_ZERO_PTR));
+  my_free(mi_get_rec_buff_ptr(info, sort_param.record),
+          MYF(MY_ALLOW_ZERO_PTR));
   my_free((gptr) sort_info.key_block,MYF(MY_ALLOW_ZERO_PTR));
   my_free((gptr) sort_info.ft_buf, MYF(MY_ALLOW_ZERO_PTR));
   my_free(sort_info.buff,MYF(MY_ALLOW_ZERO_PTR));
@@ -2493,6 +2493,7 @@ int mi_repair_parallel(MI_CHECK *param, 
   SORT_INFO sort_info;
   ulonglong key_map=share->state.key_map;
   pthread_attr_t thr_attr;
+  ulong max_pack_reclength;
   DBUG_ENTER("mi_repair_parallel");
 
   start_records=info->state->records;
@@ -2649,10 +2650,13 @@ int mi_repair_parallel(MI_CHECK *param, 
 
   del=info->state->del;
   param->glob_crc=0;
-
+  /* for compressed tables */
+  max_pack_reclength= share->base.pack_reclength;
+  if (share->options & HA_OPTION_COMPRESS_RECORD)
+    set_if_bigger(max_pack_reclength, share->max_pack_length);
   if (!(sort_param=(MI_SORT_PARAM *)
         my_malloc((uint) share->base.keys *
-		  (sizeof(MI_SORT_PARAM) + share->base.pack_reclength),
+		  (sizeof(MI_SORT_PARAM) + max_pack_reclength),
 		  MYF(MY_ZEROFILL))))
   {
     mi_check_print_error(param,"Not enough memory for key!");
@@ -2704,7 +2708,7 @@ int mi_repair_parallel(MI_CHECK *param, 
     sort_param[i].max_pos=sort_param[i].pos=share->pack.header_length;
 
     sort_param[i].record= (((char *)(sort_param+share->base.keys))+
-			   (share->base.pack_reclength * i));
+			   (max_pack_reclength * i));
     if (!mi_alloc_rec_buff(info, -1, &sort_param[i].rec_buff))
     {
       mi_check_print_error(param,"Not enough memory!");
@@ -4320,7 +4324,7 @@ err:
 void update_auto_increment_key(MI_CHECK *param, MI_INFO *info,
 			       my_bool repair_only)
 {
-  byte *record;
+  byte *record= 0;
   DBUG_ENTER("update_auto_increment_key");
 
   if (!info->s->base.auto_key ||
@@ -4340,8 +4344,7 @@ void update_auto_increment_key(MI_CHECK 
     We have to use an allocated buffer instead of info->rec_buff as 
     _mi_put_key_in_record() may use info->rec_buff
   */
-  if (!(record= (byte*) my_malloc((uint) info->s->base.pack_reclength,
-				  MYF(0))))
+  if (!mi_alloc_rec_buff(info, -1, &record))
   {
     mi_check_print_error(param,"Not enough memory for extra record");
     DBUG_VOID_RETURN;
@@ -4353,7 +4356,7 @@ void update_auto_increment_key(MI_CHECK 
     if (my_errno != HA_ERR_END_OF_FILE)
     {
       mi_extra(info,HA_EXTRA_NO_KEYREAD,0);
-      my_free((char*) record, MYF(0));
+      my_free(mi_get_rec_buff_ptr(info, record), MYF(0));
       mi_check_print_error(param,"%d when reading last record",my_errno);
       DBUG_VOID_RETURN;
     }
@@ -4369,7 +4372,7 @@ void update_auto_increment_key(MI_CHECK 
     set_if_bigger(info->s->state.auto_increment,auto_increment);
   }
   mi_extra(info,HA_EXTRA_NO_KEYREAD,0);
-  my_free((char*) record, MYF(0));
+  my_free(mi_get_rec_buff_ptr(info, record), MYF(0));
   update_state_info(param, info, UPDATE_AUTO_INC);
   DBUG_VOID_RETURN;
 }
diff -Nrup a/myisam/mi_open.c b/myisam/mi_open.c
--- a/myisam/mi_open.c	2007-04-02 13:39:23 +05:00
+++ b/myisam/mi_open.c	2007-10-30 14:36:35 +04:00
@@ -659,8 +659,11 @@ byte *mi_alloc_rec_buff(MI_INFO *info, u
     /* to simplify initial init of info->rec_buf in mi_open and mi_extra */
     if (length == (ulong) -1)
     {
-      length= max(info->s->base.pack_reclength,
-                  info->s->base.max_key_length);
+      if (info->s->options & HA_OPTION_COMPRESS_RECORD)
+        length= max(info->s->base.pack_reclength, info->s->max_pack_length);
+      else
+        length= info->s->base.pack_reclength;
+      length= max(length, info->s->base.max_key_length);
       /* Avoid unnecessary realloc */
       if (newptr && length == old_length)
 	return newptr;
diff -Nrup a/myisam/mi_packrec.c b/myisam/mi_packrec.c
--- a/myisam/mi_packrec.c	2007-01-17 14:51:51 +04:00
+++ b/myisam/mi_packrec.c	2007-10-30 14:36:35 +04:00
@@ -164,7 +164,6 @@ my_bool _mi_read_pack_info(MI_INFO *info
   share->pack.header_length=	uint4korr(header+4);
   share->min_pack_length=(uint) uint4korr(header+8);
   share->max_pack_length=(uint) uint4korr(header+12);
-  set_if_bigger(share->base.pack_reclength,share->max_pack_length);
   elements=uint4korr(header+16);
   intervall_length=uint4korr(header+20);
   trees=uint2korr(header+24);
diff -Nrup a/myisam/myisamchk.c b/myisam/myisamchk.c
--- a/myisam/myisamchk.c	2007-08-02 21:50:16 +05:00
+++ b/myisam/myisamchk.c	2007-10-30 14:36:35 +04:00
@@ -1543,8 +1543,8 @@ static int mi_sort_records(MI_CHECK *par
     mi_check_print_error(param,"Not enough memory for key block");
     goto err;
   }
-  if (!(sort_param.record=(byte*) my_malloc((uint) share->base.pack_reclength,
-					   MYF(0))))
+
+  if (!mi_alloc_rec_buff(info, -1, &sort_param.record))
   {
     mi_check_print_error(param,"Not enough memory for record");
     goto err;
@@ -1639,7 +1639,8 @@ err:
   {
     my_afree((gptr) temp_buff);
   }
-  my_free(sort_param.record,MYF(MY_ALLOW_ZERO_PTR));
+  my_free(mi_get_rec_buff_ptr(info, sort_param.record),
+          MYF(MY_ALLOW_ZERO_PTR));
   info->opt_flag&= ~(READ_CACHE_USED | WRITE_CACHE_USED);
   VOID(end_io_cache(&info->rec_cache));
   my_free(sort_info.buff,MYF(MY_ALLOW_ZERO_PTR));