List:Commits« Previous MessageNext Message »
From:ingo Date:February 16 2006 6:46pm
Subject:bk commit into 4.1 tree (ingo:1.2463) BUG#11527
View as plain text  
Below is the list of changes that have just been committed into a local
4.1 repository of mydev. When mydev 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
  1.2463 06/02/16 19:45:57 ingo@stripped +1 -0
  Bug#11527 - Setting myisam_repair_threads to >1 leads to corruption
  A wrong cast led to numeric overflow for data files
  greater than 4GB. The parallel repair assumed end of
  file after reading the amount of data that the file
  was bigger than 4GB. It truncated the data file and
  noted the number of records it found so far in the
  index file header as the number of rows in the table.
  Removing the cast fixed the problem.
  I added some cosmetic changes too.
  
  The normal repair worked because it uses a different
  function to read from the data file.

  mysys/mf_iocache.c
    1.48 06/02/16 19:45:54 ingo@stripped +75 -28
    Bug#11527 - Setting myisam_repair_threads to >1 leads to corruption
    The pure fix was to remove a cast from a file offset difference.
    Supplemented this with warnings in function comments,
    a change from == to <= to be slightly more safe,
    a renaming from "read_len" to "left_length" to make the
    partial code duplication between _my_b_read() and _my_b_read_r()
    more obvious and easier to compare the functions,
    removed another unnecessary (but harmless) cast,
    and fixed coding sytle around the "left_length" changes.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	ingo
# Host:	chilla.local
# Root:	/home/mydev/mysql-4.1-bug11527

--- 1.47/mysys/mf_iocache.c	2005-01-15 02:49:06 +01:00
+++ 1.48/mysys/mf_iocache.c	2006-02-16 19:45:54 +01:00
@@ -397,11 +397,31 @@
 
 
 /*
-  Read buffered. Returns 1 if can't read requested characters
-  This function is only called from the my_b_read() macro
-  when there isn't enough characters in the buffer to
-  satisfy the request.
-  Returns 0 we succeeded in reading all data
+  Read buffered.
+
+  SYNOPSIS
+    _my_b_read()
+      info                      IO_CACHE pointer
+      Buffer                    Buffer to retrieve count bytes from file
+      Count                     Number of bytes to read into Buffer
+
+  NOTE
+    This function is only called from the my_b_read() macro when there
+    isn't enough characters in the buffer to satisfy the request.
+
+  WARNING
+
+    When changing this function, be careful with handling file offsets
+    (end-of_file, pos_in_file). Do not cast them to possibly smaller
+    types than my_off_t unless you can be sure that their value fits.
+    Same applies to differences of file offsets.
+
+    When changing this function, check _my_b_read_r(). It might need the
+    same change.
+
+  RETURN
+    0      we succeeded in reading all data
+    1      Error: can't read requested characters
 */
 
 int _my_b_read(register IO_CACHE *info, byte *Buffer, uint Count)
@@ -429,7 +449,7 @@
   if (Count >= (uint) (IO_SIZE+(IO_SIZE-diff_length)))
   {					/* Fill first intern buffer */
     uint read_length;
-    if (info->end_of_file == pos_in_file)
+    if (info->end_of_file <= pos_in_file)
     {					/* End of file */
       info->error=(int) left_length;
       DBUG_RETURN(1);
@@ -544,43 +564,70 @@
 
 /*
   Read from IO_CACHE when it is shared between several threads.
-  It works as follows: when a thread tries to read from a file
-  (that is, after using all the data from the (shared) buffer),
-  it just hangs on lock_io_cache(), wating for other threads.
-  When the very last thread attempts a read, lock_io_cache()
-  returns 1, the thread does actual IO and unlock_io_cache(),
-  which signals all the waiting threads that data is in the buffer.
+
+  SYNOPSIS
+    _my_b_read_r()
+      info                      IO_CACHE pointer
+      Buffer                    Buffer to retrieve count bytes from file
+      Count                     Number of bytes to read into Buffer
+
+  NOTE
+    This function is only called from the my_b_read() macro when there
+    isn't enough characters in the buffer to satisfy the request.
+
+  IMPLEMENTATION
+
+    It works as follows: when a thread tries to read from a file (that
+    is, after using all the data from the (shared) buffer), it just
+    hangs on lock_io_cache(), wating for other threads. When the very
+    last thread attempts a read, lock_io_cache() returns 1, the thread
+    does actual IO and unlock_io_cache(), which signals all the waiting
+    threads that data is in the buffer.
+
+  WARNING
+
+    When changing this function, be careful with handling file offsets
+    (end-of_file, pos_in_file). Do not cast them to possibly smaller
+    types than my_off_t unless you can be sure that their value fits.
+    Same applies to differences of file offsets. (Bug #11527)
+
+    When changing this function, check _my_b_read(). It might need the
+    same change.
+
+  RETURN
+    0      we succeeded in reading all data
+    1      Error: can't read requested characters
 */
 
 int _my_b_read_r(register IO_CACHE *info, byte *Buffer, uint Count)
 {
   my_off_t pos_in_file;
-  uint length,diff_length,read_len;
+  uint length, diff_length, left_length;
   DBUG_ENTER("_my_b_read_r");
 
-  if ((read_len=(uint) (info->read_end-info->read_pos)))
+  if ((left_length= (uint) (info->read_end - info->read_pos)))
   {
-    DBUG_ASSERT(Count >= read_len);	/* User is not using my_b_read() */
-    memcpy(Buffer,info->read_pos, (size_t) (read_len));
-    Buffer+=read_len;
-    Count-=read_len;
+    DBUG_ASSERT(Count >= left_length);	/* User is not using my_b_read() */
+    memcpy(Buffer, info->read_pos, (size_t) (left_length));
+    Buffer+= left_length;
+    Count-= left_length;
   }
   while (Count)
   {
     int cnt, len;
 
-    pos_in_file= info->pos_in_file + (uint)(info->read_end - info->buffer);
+    pos_in_file= info->pos_in_file + (info->read_end - info->buffer);
     diff_length= (uint) (pos_in_file & (IO_SIZE-1));
     length=IO_ROUND_UP(Count+diff_length)-diff_length;
     length=(length <= info->read_length) ?
                    length + IO_ROUND_DN(info->read_length - length) :
                    length - IO_ROUND_UP(length - info->read_length) ;
     if (info->type != READ_FIFO &&
-	(length > (uint) (info->end_of_file - pos_in_file)))
+	(length > (info->end_of_file - pos_in_file)))
       length= (uint) (info->end_of_file - pos_in_file);
     if (length == 0)
     {
-      info->error=(int) read_len;
+      info->error= (int) left_length;
       DBUG_RETURN(1);
     }
     if (lock_io_cache(info, pos_in_file))
@@ -605,15 +652,15 @@
     info->seek_not_done=0;
     if (len <= 0)
     {
-      info->error=(int) read_len;
+      info->error= (int) left_length;
       DBUG_RETURN(1);
     }
-    cnt=((uint) len > Count) ? (int) Count : len;
-    memcpy(Buffer,info->read_pos, (size_t)cnt);
-    Count -=cnt;
-    Buffer+=cnt;
-    read_len+=cnt;
-    info->read_pos+=cnt;
+    cnt= ((uint) len > Count) ? (int) Count : len;
+    memcpy(Buffer, info->read_pos, (size_t) cnt);
+    Count -= cnt;
+    Buffer+= cnt;
+    left_length+= cnt;
+    info->read_pos+= cnt;
   }
   DBUG_RETURN(0);
 }
Thread
bk commit into 4.1 tree (ingo:1.2463) BUG#11527ingo16 Feb