bk commit into 5.1 tree (cbell:1.2570) BUG#30790

[cbell]

Below is the list of changes that have just been committed into a local
5.1 repository of cbell. When cbell 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-09-10 16:05:02-04:00, cbell@mysql_cab_desk. +4 -0
  BUG#30790 : Suspicious code in rpl_utility.cc
  
  This patch clarifies some of the coding choices with documentationa and
  removes a limitation in the code for future expansion of the CHAR and
  BINARY fields to length > 255.

  sql/field.cc@stripped, 2007-09-10 16:04:58-04:00, cbell@mysql_cab_desk. +1 -0
    BUG#30790 : Suspicious code in rpl_utility.cc
    
    This patch adds an assertion to ensure we are not attempting to encode
    negative values.

  sql/log_event.cc@stripped, 2007-09-10 16:04:58-04:00, cbell@mysql_cab_desk. +10 -0
    BUG#30790 : Suspicious code in rpl_utility.cc
    
    This patch adds comments to help explain the choice of variable types.

  sql/rpl_utility.cc@stripped, 2007-09-10 16:04:59-04:00, cbell@mysql_cab_desk. +22 -12
    BUG#30790 : Suspicious code in rpl_utility.cc
    
    This patch removes code from the calc_field_size that is not needed and
    was ambiguous. Originally intended to future expansion, the code was
    not needed.
    
    Also added are comments to help explain some portions of the code.
    
    A change was made to the korr method to use the unsigned version to 
    avoid extended sign problems.

  sql/rpl_utility.h@stripped, 2007-09-10 16:04:59-04:00, cbell@mysql_cab_desk. +5 -5
    BUG#30790 : Suspicious code in rpl_utility.cc
    
    This patch corrects some type discrepencies and removes an extra cast.

diff -Nrup a/sql/field.cc b/sql/field.cc
--- a/sql/field.cc	2007-08-27 07:43:57 -04:00
+++ b/sql/field.cc	2007-09-10 16:04:58 -04:00
@@ -6732,6 +6732,7 @@ const uint Field_varstring::MAX_SIZE= UI
 int Field_varstring::do_save_field_metadata(uchar *metadata_ptr)
 {
   char *ptr= (char *)metadata_ptr;
+  DBUG_ASSERT(field_length <= 65535);
   int2store(ptr, field_length);
   return 2;
 }
diff -Nrup a/sql/log_event.cc b/sql/log_event.cc
--- a/sql/log_event.cc	2007-08-27 14:27:42 -04:00
+++ b/sql/log_event.cc	2007-09-10 16:04:58 -04:00
@@ -6469,6 +6469,16 @@ void Rows_log_event::print_helper(FILE *
   data) in the table map are initialized as zero (0). The array size is the 
   same as the columns for the table on the slave.
 
+  Additionally, values saved for field metadata on the master are saved as a 
+  string of bytes (uchar) in the binlog. A field may require 1 or more bytes
+  to store the information. In cases where values require multiple bytes 
+  (e.g. values > 255), the endian-safe methods are used to properly encode 
+  the values on the master and decode them on the slave. When the field
+  metadata values are captured on the slave, they are stored in an array of
+  type uint16. This allows the least number of casts to prevent casting bugs
+  when the field metadata is used in comparisons of field attributes. When
+  the field metadata is used for calculating addresses in pointer math, the
+  type used is uint32. 
 */
 
 /**
diff -Nrup a/sql/rpl_utility.cc b/sql/rpl_utility.cc
--- a/sql/rpl_utility.cc	2007-08-27 07:43:58 -04:00
+++ b/sql/rpl_utility.cc	2007-09-10 16:04:59 -04:00
@@ -38,24 +38,27 @@ uint32 table_def::calc_field_size(uint c
   case MYSQL_TYPE_DOUBLE:
     length= m_field_metadata[col];
     break;
+  /*
+    The cases for SET and ENUM are include for completeness, however
+    both are mapped to type MYSQL_TYPE_STRING and their real types
+    are encoded in the field metadata.
+  */
   case MYSQL_TYPE_SET:
   case MYSQL_TYPE_ENUM:
   case MYSQL_TYPE_STRING:
   {
-    if (((m_field_metadata[col] & 0xff00) == (MYSQL_TYPE_SET << 8)) ||
-        ((m_field_metadata[col] & 0xff00) == (MYSQL_TYPE_ENUM << 8)))
+    uchar type= m_field_metadata[col] >> 8U;
+    if ((type == MYSQL_TYPE_SET) || (type == MYSQL_TYPE_ENUM))
       length= m_field_metadata[col] & 0x00ff;
     else
     {
-      length= m_field_metadata[col] & 0x00ff;
-      DBUG_ASSERT(length > 0);
-      if (length > 255)
-      {
-        DBUG_ASSERT(uint2korr(master_data) > 0);
-        length= uint2korr(master_data) + 2;
-      }
-      else
-        length= (uint) *master_data + 1;
+      /*
+        We are reading the actual size from the master_data record
+        because this field has the actual lengh stored in the first
+        byte.
+      */
+      length= (uint) *master_data + 1;
+      DBUG_ASSERT(length != 0);
     }
     break;
   }
@@ -95,6 +98,13 @@ uint32 table_def::calc_field_size(uint c
     break;
   case MYSQL_TYPE_BIT:
   {
+    /*
+      Decode the size of the bit field from the master.
+        from_len is the length in bytes from the master
+        from_bit_len is the number of extra bits stored in the master record
+      If from_bit_len is not 0, add 1 to the length to account for accurate
+      number of bytes needed.
+    */
     uint from_len= (m_field_metadata[col] >> 8U) & 0x00ff;
     uint from_bit_len= m_field_metadata[col] & 0x00ff;
     DBUG_ASSERT(from_bit_len <= 7);
@@ -136,7 +146,7 @@ uint32 table_def::calc_field_size(uint c
       length= *master_data;
       break;
     case 2:
-      length= sint2korr(master_data);
+      length= uint2korr(master_data);
       break;
     case 3:
       length= uint3korr(master_data);
diff -Nrup a/sql/rpl_utility.h b/sql/rpl_utility.h
--- a/sql/rpl_utility.h	2007-08-27 07:43:58 -04:00
+++ b/sql/rpl_utility.h	2007-09-10 16:04:59 -04:00
@@ -99,7 +99,7 @@ public:
           /*
             These types store a single byte.
           */
-          m_field_metadata[i]= (uchar)field_metadata[index];
+          m_field_metadata[i]= field_metadata[index];
           index++;
           break;
         }
@@ -107,14 +107,14 @@ public:
         case MYSQL_TYPE_ENUM:
         case MYSQL_TYPE_STRING:
         {
-          short int x= field_metadata[index++] << 8U; // real_type
+          uint16 x= field_metadata[index++] << 8U; // real_type
           x = x + field_metadata[index++];            // pack or field length
           m_field_metadata[i]= x;
           break;
         }
         case MYSQL_TYPE_BIT:
         {
-          short int x= field_metadata[index++]; 
+          uint16 x= field_metadata[index++]; 
           x = x + (field_metadata[index++] << 8U);
           m_field_metadata[i]= x;
           break;
@@ -125,13 +125,13 @@ public:
             These types store two bytes.
           */
           char *ptr= (char *)&field_metadata[index];
-          m_field_metadata[i]= sint2korr(ptr);
+          m_field_metadata[i]= uint2korr(ptr);
           index= index + 2;
           break;
         }
         case MYSQL_TYPE_NEWDECIMAL:
         {
-          short int x= field_metadata[index++] << 8U; // precision
+          uint16 x= field_metadata[index++] << 8U; // precision
           x = x + field_metadata[index++];            // decimals
           m_field_metadata[i]= x;
           break;