bk commit into 5.1 tree (mats:1.2371) BUG#24533

[Mats Kindahl]

Below is the list of changes that have just been committed into a local
5.1 repository of mats. When mats 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, 2006-11-23 10:15:02+01:00, mats@romeo.(none) +1 -0
  BUG#24533 (uninitialized null but of m_after_image):
  Patch to fix valgrind warning that uninitialized bytes were written. 
  The warning was caused by set_default(), which assumes that the field pointer is
pointing
  into record[0], while all other functions honors the move_field_offset().

  sql/log_event.cc@stripped, 2006-11-23 10:14:59+01:00, mats@romeo.(none) +36 -29
    Changing unpack_row() to always unpack into table->record[0] since the Field class
hierarchy
    contain too many dependencies on the record to operate on being in
table->record[0].
    Changing code to use the new unpack_row(), which requires some juggling of records in
    one case.

# 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:	mats
# Host:	romeo.(none)
# Root:	/home/bk/memcheck-mysql-5.1

--- 1.254/sql/log_event.cc	2006-11-23 10:15:10 +01:00
+++ 1.255/sql/log_event.cc	2006-11-23 10:15:10 +01:00
@@ -5467,14 +5467,13 @@
 
 #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
 /*
-  Unpack a row into a record.
+  Unpack a row into table->record[0].
   
   SYNOPSIS
     unpack_row()
     rli     Relay log info
     table   Table to unpack into
     colcnt  Number of columns to read from record
-    record  Record where the data should be unpacked
     row     Packed row data
     cols    Pointer to columns data to fill in
     row_end Pointer to variable that will hold the value of the
@@ -5487,6 +5486,11 @@
 
   DESCRIPTION
 
+      The function will always unpack into the table->record[0]
+      record.  This is because there are too many dependencies on
+      where the various member functions of Field and subclasses
+      expect to write.
+
       The row is assumed to only consist of the fields for which the
       bitset represented by 'arr' and 'bits'; the other parts of the
       record are left alone.
@@ -5505,15 +5509,15 @@
  */
 static int
 unpack_row(RELAY_LOG_INFO *rli,
-           TABLE *table, uint const colcnt, byte *record,
+           TABLE *table, uint const colcnt,
            char const *row, MY_BITMAP const *cols,
            char const **row_end, ulong *master_reclength,
            MY_BITMAP* const rw_set, Log_event_type const event_type)
 {
+  byte *const record= table->record[0];
   DBUG_ENTER("unpack_row");
   DBUG_ASSERT(record && row);
-  DBUG_PRINT("enter", ("row=0x%lx; record=0x%lx", row, record));
-  my_ptrdiff_t const offset= record - (byte*) table->record[0];
+  DBUG_PRINT("enter", ("row=0x%lx; table->record[0]=0x%lx", row, record));
   my_size_t master_null_bytes= table->s->null_bytes;
 
   if (colcnt != table->s->fields)
@@ -5555,11 +5559,9 @@
     {
       DBUG_ASSERT(table->record[0] <= f->ptr);
       DBUG_ASSERT(f->ptr < table->record[0] + table->s->reclength +
(f->pack_length_in_rec() == 0));
-      f->move_field_offset(offset);
 
       DBUG_PRINT("info", ("unpacking column '%s' to 0x%lx", f->field_name,
f->ptr));
       ptr= f->unpack(f->ptr, ptr);
-      f->move_field_offset(-offset);
       /* Field...::unpack() cannot return 0 */
       DBUG_ASSERT(ptr != NULL);
     }
@@ -5590,9 +5592,10 @@
   for ( ; *field_ptr ; ++field_ptr)
   {
     uint32 const mask= NOT_NULL_FLAG | NO_DEFAULT_VALUE_FLAG;
+    Field *const f= *field_ptr;
 
-    if (event_type == WRITE_ROWS_EVENT &&
-        ((*field_ptr)->flags & mask) == mask)
+    DBUG_PRINT("info", ("processing column '%s' @ 0x%lx", f->field_name, f->ptr));
+    if (event_type == WRITE_ROWS_EVENT && (f->flags & mask) == mask)
     {
       slave_print_msg(ERROR_LEVEL, rli, ER_NO_DEFAULT_FOR_FIELD,
                       "Field `%s` of table `%s`.`%s` "
@@ -5602,7 +5605,7 @@
       error = ER_NO_DEFAULT_FOR_FIELD;
     }
     else
-      (*field_ptr)->set_default();
+      f->set_default();
   }
 
   DBUG_RETURN(error);
@@ -6458,10 +6461,8 @@
   DBUG_ASSERT(row_start && row_end);
 
   int error;
-  error= unpack_row(rli,
-                    table, m_width, table->record[0],
-                    row_start, &m_cols, row_end, &m_master_reclength,
-                    table->write_set, WRITE_ROWS_EVENT);
+  error= unpack_row(rli, table, m_width, row_start, &m_cols, row_end,
+                    &m_master_reclength, table->write_set, WRITE_ROWS_EVENT);
   bitmap_copy(table->read_set, table->write_set);
   return error;
 }
@@ -6682,7 +6683,7 @@
        present on the master from table->record[1], if there are any.
     */
     copy_extra_record_fields(table, master_reclength, master_fields);
-    
+
     /*
        REPLACE is defined as either INSERT or DELETE + INSERT.  If
        possible, we can replace it with an UPDATE, but that will not
@@ -7063,10 +7064,8 @@
   */
   DBUG_ASSERT(table->s->fields >= m_width);
 
-  error= unpack_row(rli,
-                    table, m_width, table->record[0], 
-                    row_start, &m_cols, row_end, &m_master_reclength,
-                    table->read_set, DELETE_ROWS_EVENT);
+  error= unpack_row(rli, table, m_width, row_start, &m_cols, row_end,
+                    &m_master_reclength, table->read_set, DELETE_ROWS_EVENT);
   /*
     If we will access rows using the random access method, m_key will
     be set to NULL, so we do not need to make a key copy in that case.
@@ -7200,25 +7199,30 @@
   */
   DBUG_ASSERT(table->s->fields >= m_width);
 
+  /*
+    We need to perform some juggling below since unpack_row() always
+    unpacks into table->record[0]. For more information, see the
+    comments for unpack_row().
+  */
+
   /* record[0] is the before image for the update */
-  error= unpack_row(rli,
-                    table, m_width, table->record[0],
-                    row_start, &m_cols, row_end, &m_master_reclength,
-                    table->read_set, UPDATE_ROWS_EVENT);
+  error= unpack_row(rli, table, m_width, row_start, &m_cols, row_end,
+                    &m_master_reclength, table->read_set, UPDATE_ROWS_EVENT);
+  store_record(table, record[1]);
   char const *next_start = *row_end;
   /* m_after_image is the after image for the update */
-  error= unpack_row(rli,
-                    table, m_width, m_after_image,
-                    next_start, &m_cols, row_end, &m_master_reclength,
-                    table->write_set, UPDATE_ROWS_EVENT);
+  error= unpack_row(rli, table, m_width, next_start, &m_cols, row_end,
+                    &m_master_reclength, table->write_set, UPDATE_ROWS_EVENT);
+  bmove_align(m_after_image, table->record[0], table->s->reclength);
+  restore_record(table, record[1]);
 
   /*
     Don't print debug messages when running valgrind since they can
     trigger false warnings.
    */
 #ifndef HAVE_purify
-  DBUG_DUMP("record[0]", (const char *)table->record[0], m_master_reclength);
-  DBUG_DUMP("m_after_image", (const char *)m_after_image, m_master_reclength);
+  DBUG_DUMP("record[0]", (const char *)table->record[0], table->s->reclength);
+  DBUG_DUMP("m_after_image", (const char *)m_after_image, table->s->reclength);
 #endif
 
   /*
@@ -7258,6 +7262,9 @@
   */
   bmove_align(table->record[0], m_after_image, table->s->reclength);
   copy_extra_record_fields(table, m_master_reclength, m_width);
+
+  valgrind_check_mem(table->record[0], table->s->reclength);
+  valgrind_check_mem(table->record[1], table->s->reclength);
 
   /*
     Now we have the right row to update.  The old row (the one we're