[gparted] Change OperationDetail to not store complex objects in STL containers (#729139)
- From: Curtis Gedak <gedakc src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gparted] Change OperationDetail to not store complex objects in STL containers (#729139)
- Date: Sun, 18 May 2014 16:12:47 +0000 (UTC)
commit 947cd028575ceb9eee0e7facc2c1749397e50aa9
Author: Phillip Susi <psusi ubuntu com>
Date: Tue Apr 1 22:04:01 2014 -0400
Change OperationDetail to not store complex objects in STL containers (#729139)
OperationDetail was storing its children in a std::vector. This means they
can be moved around in memory arbitrarily, going through indeterminate
lifetimes. This is generally a bad thing for any non trivial object and
in the case of OperationDetail, it created havoc with the way it maintains
pointers between parent/child objects for signal connections. It will now
keep only pointers to children in a std::vector instead, so their lifetime
can be controlled, fixing various crashes.
Bug 729139 - Refactor OperationDetail to address random behavior
include/Dialog_Progress.h | 2 +-
include/OperationDetail.h | 6 +++---
src/Dialog_Progress.cc | 6 +++---
src/FileSystem.cc | 6 +++---
src/OperationDetail.cc | 28 +++++++++++++++++-----------
5 files changed, 27 insertions(+), 21 deletions(-)
---
diff --git a/include/Dialog_Progress.h b/include/Dialog_Progress.h
index d680ced..a3c88e7 100644
--- a/include/Dialog_Progress.h
+++ b/include/Dialog_Progress.h
@@ -89,7 +89,7 @@ private:
treeview_operations_Columns treeview_operations_columns;
std::vector<Operation *> operations ;
- OperationDetail operationdetail ;
+ Glib::ustring progress_text;
bool succes, cancel;
double fraction ;
unsigned int t, warnings ;
diff --git a/include/OperationDetail.h b/include/OperationDetail.h
index 290f17d..e8fb802 100644
--- a/include/OperationDetail.h
+++ b/include/OperationDetail.h
@@ -60,8 +60,8 @@ public:
Glib::ustring get_elapsed_time() const ;
void add_child( const OperationDetail & operationdetail ) ;
- std::vector<OperationDetail> & get_childs() ;
- const std::vector<OperationDetail> & get_childs() const ;
+ std::vector<OperationDetail*> & get_childs() ;
+ const std::vector<OperationDetail*> & get_childs() const ;
OperationDetail & get_last_child() ;
double fraction ;
@@ -78,7 +78,7 @@ private:
Glib::ustring treepath ;
- std::vector<OperationDetail> sub_details ;
+ std::vector<OperationDetail*> sub_details;
std::time_t time_start, time_elapsed ;
sigc::connection cancelconnection;
};
diff --git a/src/Dialog_Progress.cc b/src/Dialog_Progress.cc
index f1ee542..9526e64 100644
--- a/src/Dialog_Progress.cc
+++ b/src/Dialog_Progress.cc
@@ -148,7 +148,7 @@ void Dialog_Progress::on_signal_update( const OperationDetail & operationdetail
}
//update the gui elements..
- this ->operationdetail = operationdetail ;
+ progress_text = operationdetail.progress_text;
if ( operationdetail .get_status() == STATUS_EXECUTE )
label_current_sub_text = operationdetail .get_description() ;
@@ -182,7 +182,7 @@ void Dialog_Progress::update_gui_elements()
label_current_sub .set_markup( "<i>" + label_current_sub_text + "</i>\n" ) ;
//To ensure progress bar height remains the same, add a space in case message is empty
- progressbar_current .set_text( operationdetail .progress_text + " " ) ;
+ progressbar_current.set_text( progress_text + " " );
}
bool Dialog_Progress::pulsebar_pulse()
@@ -458,7 +458,7 @@ void Dialog_Progress::echo_operation_details( const OperationDetail & operationd
<< "<td>" << std::endl ;
for ( unsigned int t = 0 ; t < operationdetail .get_childs() .size() ; t++ )
- echo_operation_details( operationdetail .get_childs()[ t ], out ) ;
+ echo_operation_details( *(operationdetail.get_childs()[ t ]), out );
out << "</td>" << std::endl << "</tr>" << std::endl ;
}
diff --git a/src/FileSystem.cc b/src/FileSystem.cc
index 0a092c1..71bb246 100644
--- a/src/FileSystem.cc
+++ b/src/FileSystem.cc
@@ -118,12 +118,12 @@ int FileSystem::execute_command( const Glib::ustring & command, OperationDetail
OperationDetail( output, STATUS_NONE, FONT_ITALIC ) );
operationdetail.get_last_child().add_child(
OperationDetail( error, STATUS_NONE, FONT_ITALIC ) );
- std::vector<OperationDetail> &children = operationdetail.get_last_child().get_childs();
+ std::vector<OperationDetail*> &children = operationdetail.get_last_child().get_childs();
outputcapture.signal_update.connect( sigc::bind( sigc::ptr_fun( update_command_output ),
- &(children[children.size() - 2]),
+ children[children.size() - 2],
&output ) );
errorcapture.signal_update.connect( sigc::bind( sigc::ptr_fun( update_command_output ),
- &(children[children.size() - 1]),
+ children[children.size() - 1],
&error ) );
outputcapture.connect_signal();
errorcapture.connect_signal();
diff --git a/src/OperationDetail.cc b/src/OperationDetail.cc
index d08cba2..34d5a1e 100644
--- a/src/OperationDetail.cc
+++ b/src/OperationDetail.cc
@@ -36,6 +36,11 @@ OperationDetail::OperationDetail( const Glib::ustring & description, OperationDe
OperationDetail::~OperationDetail()
{
cancelconnection.disconnect();
+ while (sub_details.size())
+ {
+ delete sub_details.back();
+ sub_details.pop_back();
+ }
}
void OperationDetail::set_description( const Glib::ustring & description, Font font )
@@ -121,23 +126,23 @@ Glib::ustring OperationDetail::get_elapsed_time() const
void OperationDetail::add_child( const OperationDetail & operationdetail )
{
- sub_details .push_back( operationdetail ) ;
+ sub_details .push_back( new OperationDetail(operationdetail) );
- sub_details .back() .set_treepath( treepath + ":" + Utils::num_to_str( sub_details .size() -1 ) ) ;
- sub_details .back() .signal_update .connect( sigc::mem_fun( this, &OperationDetail::on_update ) ) ;
- sub_details.back().cancelconnection = signal_cancel.connect(
- sigc::mem_fun( &sub_details.back(), &OperationDetail::cancel ) );
+ sub_details.back()->set_treepath( treepath + ":" + Utils::num_to_str( sub_details .size() -1 ) );
+ sub_details.back()->signal_update.connect( sigc::mem_fun( this, &OperationDetail::on_update ) );
+ sub_details.back()->cancelconnection = signal_cancel.connect(
+ sigc::mem_fun( sub_details.back(), &OperationDetail::cancel ) );
if ( cancelflag )
- sub_details.back().cancel( cancelflag == 2 );
- on_update( sub_details .back() ) ;
+ sub_details.back()->cancel( cancelflag == 2 );
+ on_update( *sub_details.back() );
}
-std::vector<OperationDetail> & OperationDetail::get_childs()
+std::vector<OperationDetail*> & OperationDetail::get_childs()
{
return sub_details ;
}
-const std::vector<OperationDetail> & OperationDetail::get_childs() const
+const std::vector<OperationDetail*> & OperationDetail::get_childs() const
{
return sub_details ;
}
@@ -148,7 +153,7 @@ OperationDetail & OperationDetail::get_last_child()
if ( sub_details .size() == 0 )
add_child( OperationDetail( "---", STATUS_ERROR ) ) ;
- return sub_details[sub_details.size() - 1];
+ return *sub_details[sub_details.size() - 1];
}
void OperationDetail::on_update( const OperationDetail & operationdetail )
@@ -161,7 +166,8 @@ void OperationDetail::cancel( bool force )
{
if ( force )
cancelflag = 2;
- else cancelflag = 1;
+ else
+ cancelflag = 1;
signal_cancel(force);
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]