GNOME Bugzilla – Bug 757671
Rework Dialog_Partition_New::Get_New_Partition() a bit
Last modified: 2016-01-18 17:42:00 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)
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
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
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
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
This enhancement was included in the GParted 0.25.0 release on January 18, 2016.