From: Date: November 13 2008 2:02pm Subject: bzr commit into mysql-6.0-backup branch (oystein.grovlen:2732) Bug#40480 List-Archive: http://lists.mysql.com/commits/58632 X-Bug: 40480 Message-Id: <0KA900DSJW8S0A80@fe-emea-09.sun.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7BIT #At file:///home/oysteing/mysql/mysql-6.0-backup-valgrind/ 2732 oystein.grovlen@stripped 2008-11-13 Bug#40480 - valgrind errors which are not seen in PB Fixes all backup related issues reported by valgrind when running the backup test suite. Also fixes a by accident shallow copy of Logger to make sure Logger destructs Backup_log object only once. Added private copy constructors to prevent similar accidents in the future. modified: sql/backup/backup_info.h sql/backup/backup_kernel.h sql/backup/kernel.cc sql/backup/logger.h sql/backup/restore_info.h sql/backup/stream.h sql/backup/stream_v1.c per-file messages: sql/backup/backup_info.h Create private copy constructor and assignment operator to make sure Backup_info object is not copied by accident. sql/backup/backup_kernel.h Create private copy constructor and assignment operator to make sure Backup_restore_ctx object is not copied by accident. sql/backup/kernel.cc 1. Clean-up is now done in Logger constructor. No need for report_cleanup() anymore. 2. Free allocated tablespace object in case of errors. sql/backup/logger.h 1. Do clean-up in destructor instead of report_cleanup(). 2. Make sure to initialize error in Logger constructor. 3. Create private copy constructor and assignment operator to make sure Logger object is not copied by accident. sql/backup/restore_info.h Create private copy constructor and assignment operator to make sure Restore_info object is not copied by accident. sql/backup/stream.h Make m_log a reference to the existing Logger, not a copy. sql/backup/stream_v1.c Make sure iterator is freed on errors. === modified file 'sql/backup/backup_info.h' --- a/sql/backup/backup_info.h 2008-10-27 13:06:21 +0000 +++ b/sql/backup/backup_info.h 2008-11-13 13:02:36 +0000 @@ -48,6 +48,10 @@ class Backup_info: public backup::Image_ private: + // Prevent copying/assignments + Backup_info(const Backup_info&); + Backup_info& operator=(const Backup_info&); + class Global_iterator; ///< Iterates over global items (for which meta-data is stored). class Perdb_iterator; ///< Iterates over all per-database objects (except tables). === modified file 'sql/backup/backup_kernel.h' --- a/sql/backup/backup_kernel.h 2008-10-30 17:53:24 +0000 +++ b/sql/backup/backup_kernel.h 2008-11-13 13:02:36 +0000 @@ -84,6 +84,10 @@ class Backup_restore_ctx: public backup: private: + // Prevent copying/assignments + Backup_restore_ctx(const Backup_restore_ctx&); + Backup_restore_ctx& operator=(const Backup_restore_ctx&); + /** @c current_op points to the @c Backup_restore_ctx for the ongoing backup/restore operation. If pointer is null, no operation is currently running. */ === modified file 'sql/backup/kernel.cc' --- a/sql/backup/kernel.cc 2008-10-30 20:02:15 +0000 +++ b/sql/backup/kernel.cc 2008-11-13 13:02:36 +0000 @@ -331,7 +331,6 @@ int send_reply(Backup_restore_ctx &conte goto err; } my_eof(context.thd()); // Never errors - context.report_cleanup(); // Never errors DBUG_RETURN(0); err: @@ -399,7 +398,7 @@ Backup_restore_ctx::Backup_restore_ctx(T Backup_restore_ctx::~Backup_restore_ctx() { close(); - + delete mem_alloc; delete m_catalog; delete m_stream; @@ -1971,6 +1970,7 @@ int bcat_create_item(st_bstream_image_he { DBUG_PRINT("restore",(" tablespace has changed on the server - aborting")); info->m_ctx.fatal_error(ER_BACKUP_TS_CHANGE, desc); + delete ts; return BSTREAM_ERROR; } } === modified file 'sql/backup/logger.h' --- a/sql/backup/logger.h 2008-10-30 17:53:24 +0000 +++ b/sql/backup/logger.h 2008-11-13 13:02:36 +0000 @@ -67,7 +67,6 @@ class Logger DBUG_ASSERT(backup_log); return backup_log->get_backup_id(); } - void report_cleanup() { delete backup_log; } void save_errors(); void stop_save_errors(); @@ -85,6 +84,10 @@ class Logger int write_message(log_level::value level , int error_code, const char *msg); private: + // Prevent copying/assigments + Logger(const Logger&); + Logger& operator=(const Logger&); + util::SAVED_MYSQL_ERROR error; ///< Used to store saved errors. bool m_save_errors; ///< Flag telling if errors should be saved. bool m_push_errors; ///< Should errors be pushed on warning stack? @@ -96,12 +99,16 @@ inline Logger::Logger(THD *thd) :m_type(BACKUP), m_state(CREATED), m_thd(thd), m_save_errors(FALSE), m_push_errors(TRUE), backup_log(0) -{} +{ + clear_saved_errors(); +} + inline Logger::~Logger() { clear_saved_errors(); + delete backup_log; } inline === modified file 'sql/backup/restore_info.h' --- a/sql/backup/restore_info.h 2008-05-05 15:06:40 +0000 +++ b/sql/backup/restore_info.h 2008-11-13 13:02:36 +0000 @@ -47,6 +47,10 @@ class Restore_info: public backup::Image private: + // Prevent copying/assignments + Restore_info(const Restore_info&); + Restore_info& operator=(const Restore_info&); + friend int backup::restore_table_data(THD*, Restore_info&, backup::Input_stream&); friend int ::bcat_add_item(st_bstream_image_header*, === modified file 'sql/backup/stream.h' --- a/sql/backup/stream.h 2008-10-15 20:00:48 +0000 +++ b/sql/backup/stream.h 2008-11-13 13:02:36 +0000 @@ -98,7 +98,7 @@ class Stream: public fd_stream ::String *m_path; int m_flags; ///< flags used when opening the file size_t m_block_size; - Logger m_log; + Logger& m_log; friend int stream_write(void*, bstream_blob*, bstream_blob); friend int stream_read(void*, bstream_blob*, bstream_blob); === modified file 'sql/backup/stream_v1.c' --- a/sql/backup/stream_v1.c 2008-09-30 08:08:16 +0000 +++ b/sql/backup/stream_v1.c 2008-11-13 13:02:36 +0000 @@ -1277,10 +1277,9 @@ int bstream_wr_meta_data(backup_stream * CHECK_WR_RES(bstream_wr_item_def(s,cat,PER_TABLE_ITEM,item)); } +wr_error: bcat_iterator_free(cat,iter); - wr_error: - return ret; }