bzr commit into mysql-5.4 branch (tor.didriksen:2857) Bug#45523

[Tor Didriksen]

#At file:///home/td136807/mysqldev/azalea-sql-list/ based on revid:alik@stripped

 2857 Tor Didriksen	2009-08-17
      Bug#45523 "Objects of class base_ilist should not be copyable".
      
      Suppress the compiler-generated public copy constructor
      and assignment operator of class base_ilist; instead, implement
      move_elements_to() function which transfers ownership of elements
      from one list to another.
      
      Tested with:
      BUILD/compile-pentium-debug-max
      mysql-test/mtr --mem --force
     @ sql/sp_head.cc
        Use base_ilist::move_elements_to() rather than assignment operator
        to move elements from thd->change_list to a new owner
     @ sql/sql_cursor.cc
        Use base_ilist::move_elements_to() rather than assignment operator
        to move elements from thd->change_list to a new owner
     @ sql/sql_list.h
        make member variables first and last private
        fix typo in 'friend class ...iterator'
        disable the compiler generated copy CTOR and assignment operator
        implement new function base_ilist::move_elements_to() instead
     @ sql/sql_prepare.cc
        Use base_ilist::move_elements_to() rather than assignment operator
        to move elements from thd->change_list to a new owner

    modified:
      sql/sp_head.cc
      sql/sql_cursor.cc
      sql/sql_list.h
      sql/sql_prepare.cc
=== modified file 'sql/sp_head.cc'
--- a/sql/sp_head.cc	2009-07-30 16:22:41 +0000
+++ b/sql/sp_head.cc	2009-08-17 12:44:58 +0000
@@ -1173,8 +1173,7 @@ sp_head::execute(THD *thd)
     We should also save Item tree change list to avoid rollback something
     too early in the calling query.
   */
-  old_change_list= thd->change_list;
-  thd->change_list.empty();
+  thd->change_list.move_elements_to(&old_change_list);
   /*
     Cursors will use thd->packet, so they may corrupt data which was prepared
     for sending by upper level. OTOH cursors in the same routine can share this
@@ -1320,9 +1319,7 @@ sp_head::execute(THD *thd)
   /* Restore all saved */
   old_packet.swap(thd->packet);
   DBUG_ASSERT(thd->change_list.is_empty());
-  thd->change_list= old_change_list;
-  /* To avoid wiping out thd->change_list on old_change_list destruction */
-  old_change_list.empty();
+  old_change_list.move_elements_to(&thd->change_list);
   thd->lex= old_lex;
   thd->query_id= old_query_id;
   DBUG_ASSERT(!thd->derived_tables);

=== modified file 'sql/sql_cursor.cc'
--- a/sql/sql_cursor.cc	2009-01-26 16:03:39 +0000
+++ b/sql/sql_cursor.cc	2009-08-17 12:44:58 +0000
@@ -320,7 +320,7 @@ Sensitive_cursor::post_open(THD *thd)
   lock=           thd->lock;
   query_id=       thd->query_id;
   free_list=      thd->free_list;
-  change_list=    thd->change_list;
+  thd->change_list.move_elements_to(&change_list);
   reset_thd(thd);
   /* Now we have an active cursor and can cause a deadlock */
   thd->lock_info.n_cursors++;
@@ -436,7 +436,7 @@ Sensitive_cursor::fetch(ulong num_rows)
   thd->open_tables= open_tables;
   thd->lock= lock;
   thd->query_id= query_id;
-  thd->change_list= change_list;
+  change_list.move_elements_to(&thd->change_list);
   /* save references to memory allocated during fetch */
   thd->set_n_backup_active_arena(this, &backup_arena);
 
@@ -458,7 +458,7 @@ Sensitive_cursor::fetch(ulong num_rows)
   /* Grab free_list here to correctly free it in close */
   thd->restore_active_arena(this, &backup_arena);
 
-  change_list= thd->change_list;
+  thd->change_list.move_elements_to(&change_list);
   reset_thd(thd);
 
   for (info= ht_info; info->read_view; info++)
@@ -505,7 +505,7 @@ Sensitive_cursor::close()
     info->ht= 0;
   }
 
-  thd->change_list= change_list;
+  change_list.move_elements_to(&thd->change_list);
   {
     /*
       XXX: Another hack: we need to set THD state as if in a fetch to be
@@ -531,7 +531,6 @@ Sensitive_cursor::close()
   join= 0;
   stmt_arena= 0;
   free_items();
-  change_list.empty();
   DBUG_VOID_RETURN;
 }
 

=== modified file 'sql/sql_list.h'
--- a/sql/sql_list.h	2009-03-28 00:27:25 +0000
+++ b/sql/sql_list.h	2009-08-17 12:44:58 +0000
@@ -504,15 +504,12 @@ public:
 
 template <class T> class I_List_iterator;
 
-/*
-  WARNING: copy constructor of this class does not create a usable
-  copy, as its members may point at each other.
-*/
 
 class base_ilist
 {
+  struct ilink *first;
+  struct ilink last;
 public:
-  struct ilink *first,last;
   inline void empty() { first= &last; last.prev= &first; }
   base_ilist() { empty(); }
   inline bool is_empty() {  return first == &last; }
@@ -540,7 +537,31 @@ public:
   {
     return (first != &last) ? first : 0;
   }
-  friend class base_list_iterator;
+
+  /**
+    Moves list elements to new owner, and empties current owner (i.e. this).
+
+    @param[in,out]  new_owner  The new owner of the list elements.
+                               Should be empty in input.
+  */
+
+  void move_elements_to(base_ilist *new_owner)
+  {
+    DBUG_ASSERT(new_owner->is_empty());
+    new_owner->first= first;
+    new_owner->last= last;
+    empty();
+  }
+
+  friend class base_ilist_iterator;
+ private:
+  /*
+    We don't want to allow copying of this class, as that would give us
+    two list heads containing the same elements.
+    So we declare, but don't define copy CTOR and assignment operator.
+  */
+  base_ilist(const base_ilist&);
+  void operator=(const base_ilist&);
 };
 
 
@@ -573,6 +594,9 @@ public:
   inline void push_back(T* a)	{ base_ilist::push_back(a); }
   inline T* get()		{ return (T*) base_ilist::get(); }
   inline T* head()		{ return (T*) base_ilist::head(); }
+  inline void move_elements_to(I_List<T>* new_owner) {
+    base_ilist::move_elements_to(new_owner);
+  }
 #ifndef _lint
   friend class I_List_iterator<T>;
 #endif

=== modified file 'sql/sql_prepare.cc'
--- a/sql/sql_prepare.cc	2009-08-05 09:49:45 +0000
+++ b/sql/sql_prepare.cc	2009-08-17 12:44:58 +0000
@@ -3428,7 +3428,8 @@ Prepared_statement::execute_server_runna
   Statement stmt_backup;
   bool error;
   Query_arena *save_stmt_arena= thd->stmt_arena;
-  Item_change_list save_change_list= thd->change_list;
+  Item_change_list save_change_list;
+  thd->change_list.move_elements_to(&save_change_list);
 
   state= CONVENTIONAL_EXECUTION;
 
@@ -3438,7 +3439,6 @@ Prepared_statement::execute_server_runna
   thd->set_n_backup_statement(this, &stmt_backup);
   thd->set_n_backup_active_arena(this, &stmt_backup);
   thd->stmt_arena= this;
-  thd->change_list.empty();
 
   error= server_runnable->execute_server_code(thd);
 
@@ -3453,9 +3453,7 @@ Prepared_statement::execute_server_runna
   thd->restore_backup_statement(this, &stmt_backup);
   thd->stmt_arena= save_stmt_arena;
 
-  DBUG_ASSERT(thd->change_list.is_empty());
-  thd->change_list= save_change_list;
-  save_change_list.empty();
+  save_change_list.move_elements_to(&thd->change_list);
 
   /* Items and memory will freed in destructor */
 


Attachment: [text/bzr-bundle] bzr/[e-mail]