[gparted] Implement and use virtual Partition copy constructor clone() (#759726)
- From: Curtis Gedak <gedakc src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gparted] Implement and use virtual Partition copy constructor clone() (#759726)
- Date: Sat, 30 Jan 2016 17:40:39 +0000 (UTC)
commit 320e166c039918ad93b9dcaa811ee0181e2e318d
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date: Sat Dec 5 16:26:30 2015 +0000
Implement and use virtual Partition copy constructor clone() (#759726)
Final step for full polymorphic handling of Partition objects is to
implement a virtual copy constructor. C++ doesn't directly support
virtual copy constructors, so instead use the virtual copy constructor
idiom [1]. (Just a virtual method called clone() which is implemented
in every polymorphic class and creates a clone of the current object and
returns a pointer to it).
Then replace all calls to the (monomorphic) Partition object copy
constructor throughout the code, except in the clone() implementation
itself, with calls to the new virtual clone() method "virtual copy
constructor".
Also have to make the Partition destructor virtual too [2][3] so that
the derived class destructor is called when deleting using a base class
pointer. C++ supports this directly.
[1] Wikibooks: More C++ Idioms / Virtual Constructor
https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Virtual_Constructor
[2] When to use virtual destructors?
http://stackoverflow.com/questions/461203/when-to-use-virtual-destructors
[3] Virtuality
Guideline #4: A base class destructor should be either public and
virtual, or protected and nonvirtual
http://www.gotw.ca/publications/mill18.htm
Bug 759726 - Implement Partition object polymorphism
SQUASH: When first using pointers to Partition and calling delete
include/Partition.h | 3 +-
src/Dialog_Partition_Copy.cc | 2 +-
src/Dialog_Partition_New.cc | 2 +-
src/Dialog_Partition_Resize_Move.cc | 2 +-
src/GParted_Core.cc | 37 ++++++++++++++++++++--------------
src/OperationChangeUUID.cc | 4 +-
src/OperationCheck.cc | 2 +-
src/OperationCopy.cc | 6 ++--
src/OperationCreate.cc | 8 +++---
src/OperationDelete.cc | 2 +-
src/OperationFormat.cc | 6 ++--
src/OperationLabelFileSystem.cc | 4 +-
src/OperationNamePartition.cc | 4 +-
src/OperationResizeMove.cc | 6 ++--
src/Partition.cc | 9 ++++++++
src/PartitionVector.cc | 4 +-
src/Win_GParted.cc | 18 ++++++++--------
17 files changed, 68 insertions(+), 51 deletions(-)
---
diff --git a/include/Partition.h b/include/Partition.h
index 0f3c4e1..1d45d4a 100644
--- a/include/Partition.h
+++ b/include/Partition.h
@@ -64,7 +64,8 @@ class Partition
public:
Partition() ;
Partition( const Glib::ustring & path ) ;
- ~Partition() ;
+ virtual ~Partition();
+ virtual Partition * clone() const;
void Reset() ;
diff --git a/src/Dialog_Partition_Copy.cc b/src/Dialog_Partition_Copy.cc
index 2d7b064..3b4d46c 100644
--- a/src/Dialog_Partition_Copy.cc
+++ b/src/Dialog_Partition_Copy.cc
@@ -108,7 +108,7 @@ void Dialog_Partition_Copy::set_data( const Partition & selected_partition, cons
) ;
// Set member variable used in Dialog_Base_Partition::prepare_new_partition()
- new_partition = new Partition( copied_partition );
+ new_partition = copied_partition.clone();
new_partition->device_path = selected_partition.device_path;
new_partition->inside_extended = selected_partition.inside_extended;
new_partition->type = selected_partition.inside_extended ? TYPE_LOGICAL : TYPE_PRIMARY;
diff --git a/src/Dialog_Partition_New.cc b/src/Dialog_Partition_New.cc
index f162ddd..5dc7651 100644
--- a/src/Dialog_Partition_New.cc
+++ b/src/Dialog_Partition_New.cc
@@ -52,7 +52,7 @@ void Dialog_Partition_New::set_data( const Device & device,
const std::vector<FS> & FILESYSTEMS )
{
this ->new_count = new_count;
- new_partition = new Partition( selected_partition );
+ new_partition = selected_partition.clone();
// Copy only supported file systems from GParted_Core FILESYSTEMS vector. Add
// FS_CLEARED, FS_UNFORMATTED and FS_EXTENDED at the end. This decides the order
diff --git a/src/Dialog_Partition_Resize_Move.cc b/src/Dialog_Partition_Resize_Move.cc
index 1ab81f2..b16b00d 100644
--- a/src/Dialog_Partition_Resize_Move.cc
+++ b/src/Dialog_Partition_Resize_Move.cc
@@ -41,7 +41,7 @@ void Dialog_Partition_Resize_Move::set_data( const Partition & selected_partitio
{
GRIP = true ; //prevents on spinbutton_changed from getting activated prematurely
- new_partition = new Partition( selected_partition );
+ new_partition = selected_partition.clone();
if ( selected_partition .type == GParted::TYPE_EXTENDED )
{
diff --git a/src/GParted_Core.cc b/src/GParted_Core.cc
index e76951b..82eee10 100644
--- a/src/GParted_Core.cc
+++ b/src/GParted_Core.cc
@@ -2406,13 +2406,13 @@ bool GParted_Core::resize_move( const Partition & partition_old,
if ( partition_new .get_sector_length() > partition_old .get_sector_length() )
{
//first move, then grow. Since old.length < new.length and new.start is valid, temp
is valid.
- temp = new Partition( partition_new );
+ temp = partition_new.clone();
temp->sector_end = temp->sector_start + partition_old.get_sector_length() - 1;
}
else // ( partition_new.get_sector_length() < partition_old.get_sector_length() )
{
//first shrink, then move. Since new.length < old.length and old.start is valid, temp
is valid.
- temp = new Partition( partition_old );
+ temp = partition_old.clone();
temp->sector_end = partition_old.sector_start + partition_new.get_sector_length() - 1;
}
@@ -2459,16 +2459,16 @@ bool GParted_Core::move( const Partition & partition_old,
// this partition to the left. We also prevent overwriting the meta
// data of a following partition when we move this partition to the
// right.
- Partition partition_all_space = partition_old ;
- partition_all_space .alignment = ALIGN_STRICT ;
- if ( partition_new .sector_start < partition_all_space. sector_start )
- partition_all_space .sector_start = partition_new. sector_start ;
- if ( partition_new .sector_end > partition_all_space.sector_end )
- partition_all_space .sector_end = partition_new. sector_end ;
+ Partition * partition_all_space = partition_old.clone();
+ partition_all_space->alignment = ALIGN_STRICT;
+ if ( partition_new.sector_start < partition_all_space->sector_start )
+ partition_all_space->sector_start = partition_new.sector_start;
+ if ( partition_new.sector_end > partition_all_space->sector_end )
+ partition_all_space->sector_end = partition_new.sector_end;
//Make old partition all encompassing and if move file system fails
// then return partition table to original state
- if ( resize_move_partition( partition_old, partition_all_space, operationdetail ) )
+ if ( resize_move_partition( partition_old, *partition_all_space, operationdetail ) )
{
//Note move of file system is from old values to new values, not from
// the all encompassing values.
@@ -2476,23 +2476,30 @@ bool GParted_Core::move( const Partition & partition_old,
{
operationdetail .add_child( OperationDetail( _("rollback last change to the
partition table") ) ) ;
- Partition partition_restore = partition_old ;
- partition_restore .alignment = ALIGN_STRICT ; //Ensure that old partition
boundaries are not modified
+ Partition * partition_restore = partition_old.clone();
+ partition_restore->alignment = ALIGN_STRICT; //Ensure that old partition
boundaries are not modified
if ( resize_move_partition(
- partition_all_space, partition_restore,
operationdetail.get_last_child() ) ) {
+ *partition_all_space, *partition_restore,
operationdetail.get_last_child() ) )
+ {
operationdetail.get_last_child().set_status( STATUS_SUCCES );
check_repair_filesystem( partition_old, operationdetail );
}
else
operationdetail .get_last_child() .set_status( STATUS_ERROR ) ;
+
+ delete partition_restore;
+ partition_restore = NULL;
}
else
succes = true ;
}
//Make new partition from all encompassing partition
- succes = succes && resize_move_partition( partition_all_space, partition_new,
operationdetail ) ;
+ succes = succes && resize_move_partition( *partition_all_space, partition_new,
operationdetail );
+
+ delete partition_all_space;
+ partition_all_space = NULL;
succes = ( succes
&& update_bootsector( partition_new, operationdetail )
@@ -3196,8 +3203,8 @@ void GParted_Core::rollback_transaction( const Partition & partition_src,
if ( total_done > 0 )
{
//find out exactly which part of the file system was copied (and to where it was copied)..
- Partition * temp_src = new Partition( partition_src );
- Partition * temp_dst = new Partition( partition_dst );
+ Partition * temp_src = partition_src.clone();
+ Partition * temp_dst = partition_dst.clone();
bool rollback_needed = true;
if ( partition_dst .sector_start > partition_src .sector_start )
diff --git a/src/OperationChangeUUID.cc b/src/OperationChangeUUID.cc
index f21275c..8293049 100644
--- a/src/OperationChangeUUID.cc
+++ b/src/OperationChangeUUID.cc
@@ -29,8 +29,8 @@ OperationChangeUUID::OperationChangeUUID( const Device & device
type = OPERATION_CHANGE_UUID ;
this->device = device.get_copy_without_partitions();
- this->partition_original = new Partition( partition_orig );
- this->partition_new = new Partition( partition_new );
+ this->partition_original = partition_orig.clone();
+ this->partition_new = partition_new.clone();
}
OperationChangeUUID::~OperationChangeUUID()
diff --git a/src/OperationCheck.cc b/src/OperationCheck.cc
index 4a27c08..666d646 100644
--- a/src/OperationCheck.cc
+++ b/src/OperationCheck.cc
@@ -26,7 +26,7 @@ OperationCheck::OperationCheck( const Device & device, const Partition & partiti
type = OPERATION_CHECK ;
this->device = device.get_copy_without_partitions();
- this->partition_original = new Partition( partition );
+ this->partition_original = partition.clone();
}
OperationCheck::~OperationCheck()
diff --git a/src/OperationCopy.cc b/src/OperationCopy.cc
index b0680da..1b7d540 100644
--- a/src/OperationCopy.cc
+++ b/src/OperationCopy.cc
@@ -30,9 +30,9 @@ OperationCopy::OperationCopy( const Device & device,
type = OPERATION_COPY ;
this->device = device.get_copy_without_partitions();
- this->partition_original = new Partition( partition_orig );
- this->partition_new = new Partition( partition_new );
- this->partition_copied = new Partition( partition_copied );
+ this->partition_original = partition_orig.clone();
+ this->partition_new = partition_new.clone();
+ this->partition_copied = partition_copied.clone();
this->partition_new->add_path(
String::ucompose( _("copy of %1"), this->partition_copied->get_path() ), true );
diff --git a/src/OperationCreate.cc b/src/OperationCreate.cc
index 0e8f4ba..317cf46 100644
--- a/src/OperationCreate.cc
+++ b/src/OperationCreate.cc
@@ -31,8 +31,8 @@ OperationCreate::OperationCreate( const Device & device,
type = OPERATION_CREATE ;
this->device = device.get_copy_without_partitions();
- this->partition_original = new Partition( partition_orig );
- this->partition_new = new Partition( partition_new );
+ this->partition_original = partition_orig.clone();
+ this->partition_new = partition_new.clone();
}
OperationCreate::~OperationCreate()
@@ -88,7 +88,7 @@ bool OperationCreate::merge_operations( const Operation & candidate )
// Merge a format operation on a not yet created partition with the
// earlier operation which will create it.
delete partition_new;
- partition_new = new Partition( candidate.get_partition_new() );
+ partition_new = candidate.get_partition_new().clone();
create_description();
return true;
}
@@ -99,7 +99,7 @@ bool OperationCreate::merge_operations( const Operation & candidate )
// Merge a resize/move operation on a not yet created partition with the
// earlier operation which will create it.
delete partition_new;
- partition_new = new Partition( candidate.get_partition_new() );
+ partition_new = candidate.get_partition_new().clone();
create_description();
return true;
}
diff --git a/src/OperationDelete.cc b/src/OperationDelete.cc
index d615b19..45ee289 100644
--- a/src/OperationDelete.cc
+++ b/src/OperationDelete.cc
@@ -27,7 +27,7 @@ OperationDelete::OperationDelete( const Device & device, const Partition & parti
type = OPERATION_DELETE ;
this->device = device.get_copy_without_partitions();
- this->partition_original = new Partition( partition_orig );
+ this->partition_original = partition_orig.clone();
}
OperationDelete::~OperationDelete()
diff --git a/src/OperationFormat.cc b/src/OperationFormat.cc
index b5469b9..b02eafc 100644
--- a/src/OperationFormat.cc
+++ b/src/OperationFormat.cc
@@ -28,8 +28,8 @@ OperationFormat::OperationFormat( const Device & device,
type = OPERATION_FORMAT ;
this->device = device.get_copy_without_partitions();
- this->partition_original = new Partition( partition_orig );
- this->partition_new = new Partition( partition_new );
+ this->partition_original = partition_orig.clone();
+ this->partition_new = partition_new.clone();
}
OperationFormat::~OperationFormat()
@@ -85,7 +85,7 @@ bool OperationFormat::merge_operations( const Operation & candidate )
*partition_new == candidate.get_partition_original() )
{
delete partition_new;
- partition_new = new Partition( candidate.get_partition_new() );
+ partition_new = candidate.get_partition_new().clone();
create_description();
return true;
}
diff --git a/src/OperationLabelFileSystem.cc b/src/OperationLabelFileSystem.cc
index 18f1c5d..f45b1ea 100644
--- a/src/OperationLabelFileSystem.cc
+++ b/src/OperationLabelFileSystem.cc
@@ -28,8 +28,8 @@ OperationLabelFileSystem::OperationLabelFileSystem( const Device & device,
type = OPERATION_LABEL_FILESYSTEM;
this->device = device.get_copy_without_partitions();
- this->partition_original = new Partition( partition_orig );
- this->partition_new = new Partition( partition_new );
+ this->partition_original = partition_orig.clone();
+ this->partition_new = partition_new.clone();
}
OperationLabelFileSystem::~OperationLabelFileSystem()
diff --git a/src/OperationNamePartition.cc b/src/OperationNamePartition.cc
index 0561918..89ef42b 100644
--- a/src/OperationNamePartition.cc
+++ b/src/OperationNamePartition.cc
@@ -28,8 +28,8 @@ OperationNamePartition::OperationNamePartition( const Device & device,
type = OPERATION_NAME_PARTITION;
this->device = device.get_copy_without_partitions();
- this->partition_original = new Partition( partition_orig );
- this->partition_new = new Partition( partition_new );
+ this->partition_original = partition_orig.clone();
+ this->partition_new = partition_new.clone();
}
OperationNamePartition::~OperationNamePartition()
diff --git a/src/OperationResizeMove.cc b/src/OperationResizeMove.cc
index aa21023..6c930b7 100644
--- a/src/OperationResizeMove.cc
+++ b/src/OperationResizeMove.cc
@@ -29,8 +29,8 @@ OperationResizeMove::OperationResizeMove( const Device & device,
type = OPERATION_RESIZE_MOVE ;
this->device = device.get_copy_without_partitions();
- this->partition_original = new Partition( partition_orig );
- this->partition_new = new Partition( partition_new );
+ this->partition_original = partition_orig.clone();
+ this->partition_new = partition_new.clone();
}
OperationResizeMove::~OperationResizeMove()
@@ -248,7 +248,7 @@ bool OperationResizeMove::merge_operations( const Operation & candidate )
*partition_new == candidate.get_partition_original() )
{
delete partition_new;
- partition_new = new Partition( candidate.get_partition_new() );
+ partition_new = candidate.get_partition_new().clone();
create_description();
return true;
}
diff --git a/src/Partition.cc b/src/Partition.cc
index 403f9ec..ed3455e 100644
--- a/src/Partition.cc
+++ b/src/Partition.cc
@@ -32,6 +32,15 @@ Partition::Partition( const Glib::ustring & path )
paths .push_back( path ) ;
}
+Partition * Partition::clone() const
+{
+ // Virtual copy constructor method
+ // Reference:
+ // Wikibooks: More C++ Idioms / Virtual Constructor
+ // https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Virtual_Constructor
+ return new Partition( *this );
+}
+
void Partition::Reset()
{
paths .clear() ;
diff --git a/src/PartitionVector.cc b/src/PartitionVector.cc
index 630289d..19a8cb6 100644
--- a/src/PartitionVector.cc
+++ b/src/PartitionVector.cc
@@ -26,7 +26,7 @@ PartitionVector::PartitionVector( const PartitionVector & src )
{
v.resize( src.size() );
for ( unsigned int i = 0 ; i < src.size() ; i ++ )
- v[i] = new Partition( src[i] );
+ v[i] = src[i].clone();
}
PartitionVector::~PartitionVector()
@@ -85,7 +85,7 @@ void PartitionVector::insert_adopt( iterator position, Partition * partition )
void PartitionVector::replace_at( size_type n, const Partition * partition )
{
- Partition *p = new Partition( *partition );
+ Partition *p = partition->clone();
delete v[n];
v[n] = p;
}
diff --git a/src/Win_GParted.cc b/src/Win_GParted.cc
index 0870558..b9ea0f2 100644
--- a/src/Win_GParted.cc
+++ b/src/Win_GParted.cc
@@ -935,7 +935,7 @@ void Win_GParted::Refresh_Visual()
if ( copied_partition != NULL && display_partitions[t].get_path() ==
copied_partition->get_path() )
{
delete copied_partition;
- copied_partition = new Partition( display_partitions[t] );
+ copied_partition = display_partitions[t].clone();
}
switch ( display_partitions[t].type )
@@ -947,7 +947,7 @@ void Win_GParted::Refresh_Visual()
display_partitions[t].logicals[u].get_path() ==
copied_partition->get_path() )
{
delete copied_partition;
- copied_partition = new Partition(
display_partitions[t].logicals[u] );
+ copied_partition = display_partitions[t].logicals[u].clone();
}
switch ( display_partitions[t].logicals[u].type )
@@ -1741,7 +1741,7 @@ void Win_GParted::activate_resize()
{
dialog .hide() ;
- Partition * part_temp = new Partition( dialog.Get_New_Partition(
devices[current_device].sector_size ) );
+ Partition * part_temp = dialog.Get_New_Partition( devices[current_device].sector_size
).clone();
// When resizing/moving a partition which already exists on the disk all
// possible operations could be pending so only try merging with the
@@ -1810,7 +1810,7 @@ void Win_GParted::activate_copy()
g_assert( valid_display_partition_ptr( selected_partition_ptr ) ); // Bug: Not pointing at a valid
display partition object
delete copied_partition;
- copied_partition = new Partition( *selected_partition_ptr );
+ copied_partition = selected_partition_ptr->clone();
}
void Win_GParted::activate_paste()
@@ -1832,7 +1832,7 @@ void Win_GParted::activate_paste()
{
// We don't want the messages, mount points or name of the source
// partition for the new partition being created.
- Partition * part_temp = new Partition( *copied_partition );
+ Partition * part_temp = copied_partition->clone();
part_temp->messages.clear();
part_temp->clear_mountpoints();
part_temp->name.clear();
@@ -1870,7 +1870,7 @@ void Win_GParted::activate_paste()
shown_dialog = true ;
}
- Partition * partition_new = new Partition( *selected_partition_ptr );
+ Partition * partition_new = selected_partition_ptr->clone();
partition_new->alignment = ALIGN_STRICT;
partition_new->filesystem = copied_partition->filesystem;
partition_new->set_filesystem_label( copied_partition->get_filesystem_label() );
@@ -2655,7 +2655,7 @@ void Win_GParted::activate_label_filesystem()
{
dialog .hide() ;
// Make a duplicate of the selected partition (used in UNDO)
- Partition * part_temp = new Partition( *selected_partition_ptr );
+ Partition * part_temp = selected_partition_ptr->clone();
part_temp->set_filesystem_label( dialog.get_new_label() );
@@ -2689,7 +2689,7 @@ void Win_GParted::activate_name_partition()
{
dialog.hide();
// Make a duplicate of the selected partition (used in UNDO)
- Partition * part_temp = new Partition( *selected_partition_ptr );
+ Partition * part_temp = selected_partition_ptr->clone();
part_temp->name = dialog.get_new_name();
@@ -2736,7 +2736,7 @@ void Win_GParted::activate_change_uuid()
}
// Make a duplicate of the selected partition (used in UNDO)
- Partition * part_temp = new Partition( *selected_partition_ptr );
+ Partition * part_temp = selected_partition_ptr->clone();
if ( part_temp->filesystem == FS_NTFS )
//Explicitly ask for half, so that the user will be aware of it
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
