It made the code look a little messy, is easily resolved in the build
system and made the dependencies more complicated than needed. Each
GParted header was tracked via multiple different names (different
numbers of "../include/" prefixes). For example just looking at how
DialogFeatures.o depends on Utils.h:
$ cd src
$ make DialogFeatures.o
$ egrep ' [^ ]*Utils.h' .deps/DialogFeatures.Po
../include/DialogFeatures.h ../include/../include/Utils.h \
../include/../include/../include/../include/../include/../include/Utils.h \
../include/../include/../include/Utils.h \
After removing "../include/" from the GParted header #includes, just
need to add "-I../include" to the compile command via the AM_CPPFLAGS in
src/Makefile.am. Now the dependencies on GParted header files are
tracked under a single name (with a single "../include/" prefix). Now
DialogFeatures.o only depends on a single name to Utils.h:
$ make DialogFeatures.o
$ egrep ' [^ ]*Utils.h' .deps/DialogFeatures.Po
../include/DialogFeatures.h ../include/Utils.h ../include/i18n.h \
When a base class destructor is virtual, derived class destructors are
also virtual [1] even if they don't have the virtual qualifier.
As the Operation destructor is virtual, derived Operation* classes
destructors are virtual too. Add virtual qualifier just to reflect what
the C++ language mandates the compiler implement.
[1] Derived class with non-virtual destructor
http://stackoverflow.com/questions/7403883/derived-class-with-non-virtual-destructor
Operation classes now internally use pointers to Partition objects and
take on management of their lifetimes. As before, with the
PartitionVector class, when storing pointers in a class the Big 3 of
destructor, copy constructor and copy assignment operator also have to
be considered.
First, all the Partition objects are allocated in the derived Operation*
class parameterised constructors and freed in the associated
destructors. However the Operation classes are never copy constructed
or copy assigned; they are only ever created and destroyed. Only
pointers to the derived Operations are copied into the vector of pending
operations. Therefore the copy construtor and copy assignment operator
aren't needed. To enforce this provide inaccessible private
declarations without any implementation so that the compiler will
enforce this [1][2].
This example code fragment:
1 OperationCheck o1( device, partition );
2 OperationCheck o2 = o1;
3 o2 = o1;
Does these OperationCheck calls:
1 Implemented parameterised construtor,
2 Disallowed copy constructor,
3 Disallowed copy assignment
Trying to compile the above code would fail with errors like these:
../include/OperationCheck.h: In member function 'void GParted::Win_GParted::activate_check()':
../include/OperationCheck.h:36:2: error: 'GParted::OperationCheck::OperationCheck(const GParted::OperationCheck&)' is private
OperationCheck( const OperationCheck & src ); // Not implemented copy constructor
^
test.cc:2:21: error: within this context
OperationCheck o2 = o1;
^
../include/OperationCheck.h:37:19: error: 'GParted::OperationCheck& GParted::OperationCheck::operator=(const GParted::OperationCheck&)' is private
OperationCheck & operator=( const OperationCheck & rhs ); // Not implemented copy assignment operator
^
test.cc:3:4: error: within this context
o2 = o1;
^
[1] Disable copy constructor
http://stackoverflow.com/questions/6077143/disable-copy-constructor
[2] Disable compiler-generated copy-assignment operator [duplicate]
http://stackoverflow.com/questions/7823845/disable-compiler-generated-copy-assignment-operator
Bug 759726 - Implement Partition object polymorphism
The Operation classes contain partition objects which are copied by
value. Need to replace these with pointers to Partition objects instead
and manage their lifetimes so that they can be used polymorphically.
First step is to protect the partition members partition_new,
partition_original, and for OperationCopy class only, partition_copied
within the Operation classes and provide accessor methods.
get_partition_new() and get_partition_original() accessors are
implemented in the Operation base class so all derived classes get an
implementation. get_partition_new() is also virtual so that
OperationCheck and OperationDelete can override the implementation and
assert that they don't use partition_new. get_partition_copied() is
provided for the OperationCopy class only so can only be accessed via an
OperationCopy type variable.
Bug 759726 - Implement Partition object polymorphism
Win_GParted::Merge_Operations() method was modifying the internals of
Operation* objects; in particular the partition_new member variable.
This is breaking data hiding and encapsulation tenant of object oriented
programming.
Implement exactly the same operation merge semantics, but hide the
manipulation of the internals of the Operation* objects within the
Operation* classes themselves.
Bug 755214 - Refactor operation merging
This is part of parent bug:
Bug #721455 - Obsolete info in license text on multiple modules
and GNOME Goal:
https://wiki.gnome.org/Initiatives/GnomeGoals/Proposals
* verify all source files to make sure they have a license and a
copyright, and that both are up-to-date
Bug #721565 - License text contains obsolete FSF postal address
Include guards need to be unique within GParted code and all included
library header files.
http://en.wikipedia.org/wiki/Include_guard#Difficulties
Use this model for all include guards:
#ifndef GPARTED_FILE_NAME_H
#define GPARTED_FILE_NAME_H
...
#endif /* GPARTED_FILE_NAME_H */
Closes Bug #539297 - Make include guards unique
Mike Fleetwood