List:Internals« Previous MessageNext Message »
From:Mats Kindahl Date:May 4 2005 2:00pm
Subject:bk commit into 5.1 tree (mats:1.1818)
View as plain text  
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
  1.1818 05/05/04 15:59:57 mats@stripped +3 -0
  WL#1012: Fixing an error in the table map implementation causing a memory overrun.

  sql/rpl_tblmap.h
    1.4 05/05/04 15:59:53 mats@stripped +6 -0
    Introducing error codes.

  sql/rpl_tblmap.cc
    1.4 05/05/04 15:59:53 mats@stripped +34 -22
    No need to fill an array.
    my_free() can be called with a zero pointer.
    Adding assertions.
    Introducing error codes.
    Bug when a new table is added.

  sql/log_event.cc
    1.188 05/05/04 15:59:53 mats@stripped +14 -9
    Removed some warnings.
    my_free() can be called on zero pointer.

# 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.kindahl.net
# Root:	/home/bk/w2325-mysql-5.1

--- 1.187/sql/log_event.cc	2005-04-30 13:37:36 +02:00
+++ 1.188/sql/log_event.cc	2005-05-04 15:59:53 +02:00
@@ -4901,7 +4901,8 @@
   size_t const data_size = event_len - (ptr_rows_data - buf);
   DBUG_PRINT("info",("data_size=%lu", data_size));
 
-  if ((m_rows_buf = my_malloc(data_size, MYF(0)))) 
+  m_rows_buf = my_malloc(data_size, MYF(0));
+  if (m_rows_buf) 
   {
     // Construct a new bitvector of the right size with the data and swap the
     // guts of it with the member variable.
@@ -4923,7 +4924,7 @@
 Rows_log_event::
 ~Rows_log_event()
 {
-  my_free(m_rows_buf, MYF(MY_WME));
+  my_free(m_rows_buf, MYF(MY_ALLOW_ZERO_PTR));
   m_rows_buf = m_rows_cur = m_rows_end = NULL;
   m_dbnam = m_tblnam = NULL; 
 }
@@ -5286,7 +5287,7 @@
 Table_map_log_event::
 ~Table_map_log_event()
 {
-  my_free(m_memory, MYF(0));
+  my_free(m_memory, MYF(MY_ALLOW_ZERO_PTR));
   m_memory = NULL;
 }
 
@@ -5694,6 +5695,11 @@
 Delete_rows_log_event::
 ~Delete_rows_log_event() 
 {
+#ifdef HAVE_REPLICATION
+  // This free is just a precaution, the memory should be released.
+  my_free(m_memory, MYF(MY_ALLOW_ZERO_PTR));
+  m_memory= 0;
+#endif
 }
 #endif
 
@@ -5731,15 +5737,14 @@
 		    NULL);
   if (!m_memory)
     DBUG_RETURN(HA_ERR_OUT_OF_MEM);
+
   if (table->s->keys > 0) 
-  {
-    // We have a key: search the table using the index
+  {			    // We have a key: search the table using the index
     if (!table->file->inited)
       table->file->ha_index_init(0);
   } 
   else 
-  {
-    // We doesn't have a key: search the table using rnd_next()
+  {		   // We doesn't have a key: search the table using rnd_next()
     table->file->ha_rnd_init(1);
   }
 
@@ -5752,7 +5757,7 @@
   DBUG_ENTER("Delete_rows_log_event::do_after_row_operations(TABLE*)");
 
   table->file->ha_index_or_rnd_end();
-  my_free(m_memory, MYF(MY_WME)); // Free for multi_malloc
+  my_free(m_memory, MYF(MY_ALLOW_ZERO_PTR)); // Free for multi_malloc
   m_memory = m_search_record = m_key = NULL;
 
   DBUG_VOID_RETURN;
@@ -5884,7 +5889,7 @@
   DBUG_ENTER("Update_rows_log_event::do_after_row_operations(TABLE*)");
 
   table->file->ha_index_or_rnd_end();
-  my_free(m_memory, MYF(MY_WME));
+  my_free(m_memory, MYF(MY_ALLOW_ZERO_PTR));
   m_memory = m_search_record = m_key = NULL;
 
   DBUG_VOID_RETURN;

--- 1.3/sql/rpl_tblmap.cc	2005-04-28 14:27:29 +02:00
+++ 1.4/sql/rpl_tblmap.cc	2005-05-04 15:59:53 +02:00
@@ -8,11 +8,11 @@
   : m_array(0), m_count(0), m_reserve(16)
 {
   pthread_mutex_init(&m_mutex, NULL);
-
   sentry sentry(&m_mutex);
 
-  m_array = reinterpret_cast<entry*>(my_malloc(m_reserve * sizeof(*m_array), MYF(MY_WME)));
-  memset(m_array, m_reserve * sizeof(*m_array), 0);
+  // TODO: my_malloc() can fail
+  m_array = reinterpret_cast<entry*>(my_malloc(m_reserve * sizeof(*m_array), 
+					       MYF(0)));
 }
 
 table_mapping::
@@ -20,20 +20,20 @@
 {
   {
     sentry sentry(&m_mutex);
-    my_free(reinterpret_cast<char*>(m_array), MYF(MY_WME));
+    my_free(reinterpret_cast<char*>(m_array), MYF(MY_ALLOW_ZERO_PTR));
     m_array = NULL;
   }
   pthread_mutex_destroy(&m_mutex);
 }
 
-
-
 ulong table_mapping::
 find_pos(ulong table_id) const
 {
-  // !!! This is a linear search, it will be infeasible to use for larger 
-  // !!! number of tables in the air at the same time. Switch to a binary
-  // !!! search later.
+  DBUG_ASSERT(m_count <= m_reserve);
+  DBUG_ASSERT(m_array != NULL);
+  // N.B.: This is a linear search, it will be infeasible to use for larger
+  // number of tables in the air at the same time. Switch to a binary search
+  // later.
   for (ulong i = 0 ; i < m_count ; ++i) {
     if (m_array[i].table_id == table_id)
       return i;
@@ -44,9 +44,11 @@
 ulong table_mapping::
 find_pos(st_table* table) const
 {
-  // !!! This is a linear search, it will be infeasible to use for larger 
-  // !!! number of tables in the air at the same time. Switch to a binary
-  // !!! search later.
+  DBUG_ASSERT(m_count <= m_reserve);
+  DBUG_ASSERT(m_array != NULL);
+  // N.B.: This is a linear search, it will be infeasible to use for larger
+  // number of tables in the air at the same time. Switch to a binary search
+  // later.
   for (ulong i = 0 ; i < m_count ; ++i) {
     if (m_array[i].table == table)
       return i;
@@ -57,12 +59,13 @@
 st_table* table_mapping::
 get_table(ulong table_id)
 {
-  sentry sentry(&m_mutex);
+  sentry sentry(&m_mutex);	// Acquire exlusive access
 
   DBUG_ENTER("table_mapping::get_table(ulong)");
   DBUG_PRINT("enter", ("table_id=%d", table_id));
   pos_type const pos = find_pos(table_id);
-  if (pos < m_count) {
+  if (pos < m_count) 
+  {
     DBUG_PRINT("info", ("tid %d -> table %p (%s)", 
 			table_id, m_array[pos].table,
 			m_array[pos].table->s->table_name));
@@ -83,7 +86,8 @@
   DBUG_PRINT("enter", ("table=%p (%s)", 
 		       table, table ? table->s->table_name : "<>"));
   pos_type const pos = find_pos(table);
-  if (pos < m_count) {
+  if (pos < m_count)
+  {
     DBUG_PRINT("info", ("table %p (%s) -> table id %d", 
 			m_array[pos].table,
 			m_array[pos].table->s->table_name,
@@ -106,26 +110,33 @@
 		       table, table ? table->s->table_name : "<>"));
   pos_type const pos = find_pos(table_id);
 
-  // See if we need to allocate a larger array
-  if (pos == m_count && m_reserve == m_count) {
+  if (pos == m_count && m_reserve == m_count) 
+  {
+    // We're adding a new table now, we need to extend the array to make space
+    // for the new table data.
     if (m_reserve > ULONG_MAX/2)
-      DBUG_RETURN(1);			// Table upper limit exceeded
+      DBUG_RETURN(ERR_LIMIT_EXCEEDED); // Table upper limit exceeded
 
     int const reserve = 2*m_reserve;
     entry* const 
       array = (entry*) my_realloc((char*) m_array, 
 				  reserve*sizeof(*m_array), 
 				  MYF(MY_WME));
-    if (array == NULL) {
-      DBUG_RETURN(1);			// Memory allocation failed
+    if (array == NULL)
+    {
+      DBUG_RETURN(ERR_MEMORY_ALLOCATION); // Memory allocation failed
     }
     m_reserve = reserve;
     m_array = array;
   }
 
+  DBUG_ASSERT(m_count <= m_reserve);
+  if (pos == m_count)
+    ++m_count;
+
+  DBUG_ASSERT(pos < m_count);
   m_array[pos].table_id = table_id;
   m_array[pos].table = table;
-  ++m_count;
   DBUG_PRINT("info", ("tid %d -> table %p (%s)", 
 		      table_id, m_array[pos].table,
 		      m_array[pos].table->s->table_name));
@@ -138,7 +149,8 @@
   sentry sentry(&m_mutex);
 
   ulong pos = find_pos(table_id);
-  if (pos < m_count) {
+  if (pos < m_count)
+  {
     while (++pos < m_count)
       m_array[pos-1] = m_array[pos];
     --m_count;

--- 1.3/sql/rpl_tblmap.h	2005-04-28 14:27:29 +02:00
+++ 1.4/sql/rpl_tblmap.h	2005-05-04 15:59:53 +02:00
@@ -28,6 +28,12 @@
     NO_TABLE = ULONG_MAX
   };
 
+  enum enum_error {
+      ERR_NO_ERROR = 0,
+      ERR_LIMIT_EXCEEDED,
+      ERR_MEMORY_ALLOCATION
+  };
+
   table_mapping();
   ~table_mapping();
 
Thread
bk commit into 5.1 tree (mats:1.1818)Mats Kindahl4 May