From 6766c0d676160deca76c6de69fa6d74cc19ebcdf Mon Sep 17 00:00:00 2001 From: Tor Didriksen Date: Tue, 12 Jan 2010 12:32:55 +0100 Subject: [PATCH] Backport of 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. --- sql/sp_head.cc | 7 ++----- sql/sql_cursor.cc | 9 ++++----- sql/sql_list.h | 36 ++++++++++++++++++++++++++++++------ sql/sql_prepare.cc | 4 ++-- 4 files changed, 38 insertions(+), 18 deletions(-) diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 4f5ca1fff04..cd7486dec72 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -1171,8 +1171,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 @@ -1318,9 +1317,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); diff --git a/sql/sql_cursor.cc b/sql/sql_cursor.cc index 098f049704c..31aa3e7ea52 100644 --- a/sql/sql_cursor.cc +++ b/sql/sql_cursor.cc @@ -321,7 +321,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++; @@ -437,7 +437,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); @@ -459,7 +459,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++) @@ -506,7 +506,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 @@ -532,7 +532,6 @@ Sensitive_cursor::close() join= 0; stmt_arena= 0; free_items(); - change_list.empty(); DBUG_VOID_RETURN; } diff --git a/sql/sql_list.h b/sql/sql_list.h index e1bf05fff23..fdc80b116a7 100644 --- a/sql/sql_list.h +++ b/sql/sql_list.h @@ -504,15 +504,12 @@ public: template 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* new_owner) { + base_ilist::move_elements_to(new_owner); + } #ifndef _lint friend class I_List_iterator; #endif diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 838f25320ec..70f69c75de3 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -3407,7 +3407,7 @@ Prepared_statement::execute_server_runnable(Server_runnable *server_runnable) bool error; Query_arena *save_stmt_arena= thd->stmt_arena; Item_change_list save_change_list; - thd->change_list= save_change_list; + thd->change_list.move_elements_to(&save_change_list); state= CONVENTIONAL_EXECUTION; @@ -3431,7 +3431,7 @@ Prepared_statement::execute_server_runnable(Server_runnable *server_runnable) thd->restore_backup_statement(this, &stmt_backup); thd->stmt_arena= save_stmt_arena; - save_change_list= thd->change_list; + save_change_list.move_elements_to(&thd->change_list); /* Items and memory will freed in destructor */