After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 757671 - Rework Dialog_Partition_New::Get_New_Partition() a bit
Rework Dialog_Partition_New::Get_New_Partition() a bit
Status: RESOLVED FIXED
Product: gparted
Classification: Other
Component: application
GIT HEAD
Other Linux
: Normal normal
: ---
Assigned To: Mike Fleetwood
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2015-11-06 08:05 UTC by Mike Fleetwood
Modified: 2016-01-18 17:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Rework Get_New_Partition a bit (v1) (19.76 KB, patch)
2015-11-08 09:27 UTC, Mike Fleetwood
none Details | Review

Description Mike Fleetwood 2015-11-06 08:05:19 UTC
Primarily to:
1) Remove use of temporary partition object in
   Dialog_Partition_New::Get_New_Partition(), using new_partition member
   instead; and
2) Return new_partition by reference instead of by value from
   *::Get_New_Partition().

Part of the ongoing tiding in preparation LUKS support using polymorphic
Partition classes.

References:
Bug 750168 - Reduce the amount of copying of partition objects
Bug 627701 - option to encrypt new partitions (using LUKS)
Comment 1 Mike Fleetwood 2015-11-08 09:27:51 UTC
Created attachment 315074 [details] [review]
Rework Get_New_Partition a bit (v1)

Hi Curtis,

Here's patchset v1 for this.

Note that patch "Create unallocated  space within new extended partition
after aligning boundaries (#757671)" finds another case of where having
snap_to_alignment() changing the partition boundaries after returning
from the dialog that created it is broken.  Left a FIXME in the code for
it as it's out of scope for this bug.
(As it's a very unusual case I would prefer to leave the code broken
rather than add a sticking plaster).

Thanks,
Mike
Comment 2 Curtis Gedak 2015-11-09 16:14:57 UTC
Hi Mike,

Using patch set v1 from comment #1 I tested creating, resizing, copying and pasting primary and logical partitions.  I also tested creating and resizing an extended partition.  All of these tests completed successfully.

Would you like me to wait a while before committing this patch set to allow you more time in case you need other changes?

If not, then I will commit patch set v1.

Curtis
Comment 3 Mike Fleetwood 2015-11-11 08:44:17 UTC
Hi Curtis,

Sorry for waiting for a few days before replying.  I was just waiting to
see if there was anything else I wanted to add.  There isn't.  This is
ready for pushing upstream.

I only planned to straighten out use of partition objects primarily in
Dialog_Partition_New class with this patchset.  This has been achieved.

I have stated looking at what is next, which is to take the parsed LUKS
information and create a derived PartitionLUKS class and see where that
leads.  Even though I have reduced the amount of partition object
copying, I know there is still quite a lot going on.  Some of it will be
needed, and some probably won't.  May need to spin off some other bugs
to capture further cleanups of partition object manipulation, but not
reached any yet.  Think that it will be likely to need polymorphic
copying of Partition objects but won't use unless necessary.

(C++ doesn't directly support polymorphic copy constructors but there
are techniques to implement them).
    More C++ Idioms/Virtual Constructor
    https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Virtual_Constructor

Thanks
Mike
Comment 4 Curtis Gedak 2015-11-11 17:21:02 UTC
Hi Mike,

No worries on waiting before committing the patch set.  I don't think we can predict exactly what needs changing so I am okay with refactoring code as needed.  :-)

Patch set v1 from comment #1 has been committed to the git repository.

The relevant git commits can be viewed at the following links:

Rename parameter to selected_partition in Dialog_Partition_New methods (#757671)
https://git.gnome.org/browse/gparted/commit/?id=451c2eac43e741a4d7ff6ce79638a41336c2c611

Create unallocated space within new extended partition after aligning boundaries (#757671)
https://git.gnome.org/browse/gparted/commit/?id=c86b25847511ff1f85c8d8c5f228881e5bf7437f

Return class member from Dialog_Partition_New::Get_New_Partition() (#757671)
https://git.gnome.org/browse/gparted/commit/?id=762cd1094aece03986226a39d1f6b4fecfd73ee9

Return reference from Get_New_Partition() (#757671)
https://git.gnome.org/browse/gparted/commit/?id=2c4df87a2c4e6521773323a052e6f7eab8278d66

Curtis
Comment 5 Curtis Gedak 2016-01-18 17:42:00 UTC
This enhancement was included in the GParted 0.25.0 release on January 18, 2016.