From e2bf717ea0c9f820c7cb6d08ebc409bec70e9295 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 22 Jun 2021 00:06:17 +0200 Subject: [PATCH] [partition] Rewrite new-partition API The existing API required calling the one constructor with specific pointers (nullptr for a partition-from-free-space) followed by calling one of the initFrom*() functions. This is fragile design. Use tag-classes to distinguish create-from-free-space and edit-another-freshly-created-partition cases, refactor to merge the initFrom*() methods into the constructors and factor out the shared UI creation. Callers can now use the tag-class to distinguish. While here, adjust both callers to use QPointer, avoiding some very specific dialog-on-the-stack crash possibilities. --- .../partition/gui/CreatePartitionDialog.cpp | 76 +++++++++---------- .../partition/gui/CreatePartitionDialog.h | 45 ++++++----- src/modules/partition/gui/PartitionPage.cpp | 14 ++-- 3 files changed, 73 insertions(+), 62 deletions(-) diff --git a/src/modules/partition/gui/CreatePartitionDialog.cpp b/src/modules/partition/gui/CreatePartitionDialog.cpp index 37547ef8c..c09724463 100644 --- a/src/modules/partition/gui/CreatePartitionDialog.cpp +++ b/src/modules/partition/gui/CreatePartitionDialog.cpp @@ -52,7 +52,6 @@ static QSet< FileSystem::Type > s_unmountableFS( { FileSystem::Unformatted, CreatePartitionDialog::CreatePartitionDialog( Device* device, PartitionNode* parentPartition, - Partition* partition, const QStringList& usedMountPoints, QWidget* parentWidget ) : QDialog( parentWidget ) @@ -81,9 +80,6 @@ CreatePartitionDialog::CreatePartitionDialog( Device* device, m_ui->lvNameLineEdit->setValidator( validator ); } - standardMountPoints( *( m_ui->mountPointComboBox ), - partition ? PartitionInfo::mountPoint( partition ) : QString() ); - if ( device->partitionTable()->type() == PartitionTable::msdos || device->partitionTable()->type() == PartitionTable::msdos_sectorbased ) { @@ -132,13 +128,48 @@ CreatePartitionDialog::CreatePartitionDialog( Device* device, // Select a default m_ui->fsComboBox->setCurrentIndex( defaultFsIndex ); updateMountPointUi(); + checkMountPointSelection(); +} +CreatePartitionDialog::CreatePartitionDialog( Device* device, + const FreeSpace& freeSpacePartition, + const QStringList& usedMountPoints, + QWidget* parentWidget ) + : CreatePartitionDialog(device, freeSpacePartition.p->parent(), usedMountPoints, parentWidget ) +{ + standardMountPoints( *( m_ui->mountPointComboBox ), QString() ); setFlagList( *( m_ui->m_listFlags ), static_cast< PartitionTable::Flags >( ~PartitionTable::Flags::Int( 0 ) ), - partition ? PartitionInfo::flags( partition ) : PartitionTable::Flags() ); + PartitionTable::Flags() ); + initPartResizerWidget( freeSpacePartition.p ); +} - // Checks the initial selection. - checkMountPointSelection(); +CreatePartitionDialog::CreatePartitionDialog( Device* device, + const FreshPartition& existingNewPartition, + const QStringList& usedMountPoints, + QWidget* parentWidget ) + : CreatePartitionDialog(device, existingNewPartition.p->parent(), usedMountPoints, parentWidget ) +{ + standardMountPoints( *( m_ui->mountPointComboBox ), + PartitionInfo::mountPoint( existingNewPartition.p ) ); + setFlagList( *( m_ui->m_listFlags ), + static_cast< PartitionTable::Flags >( ~PartitionTable::Flags::Int( 0 ) ), + PartitionInfo::flags( existingNewPartition.p ) ); + + const bool isExtended = existingNewPartition.p->roles().has( PartitionRole::Extended ); + if ( isExtended ) + { + cDebug() << "Editing extended partitions is not supported."; + return; + } + + initPartResizerWidget( existingNewPartition.p ); + + FileSystem::Type fsType = existingNewPartition.p->fileSystem().type(); + m_ui->fsComboBox->setCurrentText( FileSystem::nameForType( fsType ) ); + + setSelectedMountPoint( m_ui->mountPointComboBox, PartitionInfo::mountPoint( existingNewPartition.p ) ); + updateMountPointUi(); } CreatePartitionDialog::~CreatePartitionDialog() {} @@ -288,34 +319,3 @@ CreatePartitionDialog::initPartResizerWidget( Partition* partition ) m_partitionSizeController->setPartResizerWidget( m_ui->partResizerWidget ); m_partitionSizeController->setSpinBox( m_ui->sizeSpinBox ); } - -void -CreatePartitionDialog::initFromFreeSpace( Partition* freeSpacePartition ) -{ - initPartResizerWidget( freeSpacePartition ); -} - -void -CreatePartitionDialog::initFromPartitionToCreate( Partition* partition ) -{ - Q_ASSERT( partition ); - - bool isExtended = partition->roles().has( PartitionRole::Extended ); - Q_ASSERT( !isExtended ); - if ( isExtended ) - { - cDebug() << "Editing extended partitions is not supported for now"; - return; - } - - initPartResizerWidget( partition ); - - // File System - FileSystem::Type fsType = partition->fileSystem().type(); - m_ui->fsComboBox->setCurrentText( FileSystem::nameForType( fsType ) ); - - // Mount point - setSelectedMountPoint( m_ui->mountPointComboBox, PartitionInfo::mountPoint( partition ) ); - - updateMountPointUi(); -} diff --git a/src/modules/partition/gui/CreatePartitionDialog.h b/src/modules/partition/gui/CreatePartitionDialog.h index 45e3f7cf4..e18840564 100644 --- a/src/modules/partition/gui/CreatePartitionDialog.h +++ b/src/modules/partition/gui/CreatePartitionDialog.h @@ -33,30 +33,41 @@ class Ui_CreatePartitionDialog; class CreatePartitionDialog : public QDialog { Q_OBJECT -public: - /** - * @brief Dialog for editing a new partition. + +private: + /** @brief Delegated constructor * - * For the (unlikely) case that a newly created partition is being re-edited, - * pass a pointer to that @p partition, otherwise pass nullptr. + * This does all the shared UI setup. */ CreatePartitionDialog( Device* device, - PartitionNode* parentPartition, - Partition* partition, + PartitionNode* parentPartition, + const QStringList& usedMountPoints, + QWidget* parentWidget ); + +public: + struct FreeSpace { Partition* p; }; + struct FreshPartition { Partition* p; }; + + /** @brief Dialog for editing a new partition based on free space. + * + * Creating from free space makes a wholly new partition with + * no flags set at all. + */ + CreatePartitionDialog( Device* device, + const FreeSpace& freeSpacePartition, + const QStringList& usedMountPoints, + QWidget* parentWidget = nullptr ); + /** @brief Dialog for editing a newly-created partition. + * + * A partition previously newly created (e.g. via this dialog + * and the constructor above) can be re-edited. + */ + CreatePartitionDialog( Device* device, + const FreshPartition& existingNewPartition, const QStringList& usedMountPoints, QWidget* parentWidget = nullptr ); ~CreatePartitionDialog() override; - /** - * Must be called when user wants to create a partition in - * freeSpacePartition. - */ - void initFromFreeSpace( Partition* freeSpacePartition ); - - /** - * Must be called when user wants to edit a to-be-created partition. - */ - void initFromPartitionToCreate( Partition* partition ); Partition* getNewlyCreatedPartition(); PartitionTable::Flags newFlags() const; diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index d9ff65f88..06aa7b1a8 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -391,13 +391,14 @@ PartitionPage::onCreateClicked() return; } - CreatePartitionDialog dlg( model->device(), partition->parent(), nullptr, getCurrentUsedMountpoints(), this ); - dlg.initFromFreeSpace( partition ); - if ( dlg.exec() == QDialog::Accepted ) + QPointer< CreatePartitionDialog > dlg = new + CreatePartitionDialog ( model->device(), CreatePartitionDialog::FreeSpace{ partition }, getCurrentUsedMountpoints(), this ); + if ( dlg->exec() == QDialog::Accepted ) { - Partition* newPart = dlg.getNewlyCreatedPartition(); - m_core->createPartition( model->device(), newPart, dlg.newFlags() ); + Partition* newPart = dlg->getNewlyCreatedPartition(); + m_core->createPartition( model->device(), newPart, dlg->newFlags() ); } + delete dlg; } void @@ -492,8 +493,7 @@ PartitionPage::updatePartitionToCreate( Device* device, Partition* partition ) mountPoints.removeOne( PartitionInfo::mountPoint( partition ) ); QPointer< CreatePartitionDialog > dlg - = new CreatePartitionDialog( device, partition->parent(), partition, mountPoints, this ); - dlg->initFromPartitionToCreate( partition ); + = new CreatePartitionDialog( device, CreatePartitionDialog::FreshPartition{ partition }, mountPoints, this ); if ( dlg->exec() == QDialog::Accepted ) { Partition* newPartition = dlg->getNewlyCreatedPartition();