From 7cfaba2d539e609544cb4d17034a25fcad790be0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 25 Feb 2019 16:39:19 -0500 Subject: [PATCH 1/5] [partition] In logging, name device nicely - Provide a convenience method that names a Partition* with the best human-readable name we can find (worst-case, spit out a pointer representation which will at least help figure out the identity of the Partition*). --- src/modules/partition/core/PartUtils.cpp | 39 ++++++++++++++++++------ 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/modules/partition/core/PartUtils.cpp b/src/modules/partition/core/PartUtils.cpp index d09bcd149..c9b46e82e 100644 --- a/src/modules/partition/core/PartUtils.cpp +++ b/src/modules/partition/core/PartUtils.cpp @@ -42,6 +42,25 @@ namespace PartUtils { +static QString +convenienceName( const Partition* const candidate ) +{ + if ( !candidate->mountPoint().isEmpty() ) + return candidate->mountPoint(); + if ( !candidate->partitionPath().isEmpty() ) + return candidate->partitionPath(); + if ( !candidate->devicePath().isEmpty() ) + return candidate->devicePath(); + if ( !candidate->deviceNode().isEmpty() ) + return candidate->devicePath(); + + QString p; + QTextStream s( &p ); + s << (void *)candidate; + + return p; +} + bool canBeReplaced( Partition* candidate ) { @@ -63,12 +82,12 @@ canBeReplaced( Partition* candidate ) << QString( "(%1GB)" ).arg( requiredStorageB / 1024 / 1024 / 1024 ); cDebug() << "Storage capacity B:" << availableStorageB << QString( "(%1GB)" ).arg( availableStorageB / 1024 / 1024 / 1024 ) - << "for" << candidate->partitionPath() << " length:" << candidate->length(); + << "for" << convenienceName( candidate ) << " length:" << candidate->length(); if ( ok && availableStorageB > requiredStorageB ) { - cDebug() << "Partition" << candidate->partitionPath() << "authorized for replace install."; + cDebug() << "Partition" << convenienceName( candidate ) << "authorized for replace install."; return true; } @@ -85,7 +104,7 @@ canBeResized( Partition* candidate ) return false; } - cDebug() << "Checking if" << candidate->partitionPath() << "can be resized."; + cDebug() << "Checking if" << convenienceName( candidate ) << "can be resized."; if ( !candidate->fileSystem().supportGrow() || !candidate->fileSystem().supportShrink() ) { @@ -117,7 +136,7 @@ canBeResized( Partition* candidate ) if ( table->numPrimaries() >= table->maxPrimaries() ) { - cDebug() << " .. partition table already has" + cDebug() << " .. partition table already has" << table->maxPrimaries() << "primary partitions."; return false; } @@ -139,13 +158,13 @@ canBeResized( Partition* candidate ) << QString( "(%1GB)" ).arg( advisedStorageGB ); cDebug() << "Available storage B:" << availableStorageB << QString( "(%1GB)" ).arg( availableStorageB / 1024 / 1024 / 1024 ) - << "for" << candidate->partitionPath() << " length:" << candidate->length() + << "for" << convenienceName( candidate ) << " length:" << candidate->length() << " sectorsUsed:" << candidate->sectorsUsed() << " fsType:" << candidate->fileSystem().name(); if ( ok && availableStorageB > advisedStorageB ) { - cDebug() << "Partition" << candidate->partitionPath() << "authorized for resize + autopartition install."; + cDebug() << "Partition" << convenienceName( candidate ) << "authorized for resize + autopartition install."; return true; } @@ -198,7 +217,7 @@ lookForFstabEntries( const QString& partitionPath ) mountOptions.append( "noload" ); } - cDebug() << "Checking device" << partitionPath + cDebug() << "Checking device" << partitionPath << "for fstab (fs=" << r.getOutput() << ')'; FstabEntryList fstabEntries; @@ -209,9 +228,9 @@ lookForFstabEntries( const QString& partitionPath ) if ( !exit ) // if all is well { QFile fstabFile( mountsDir.path() + "/etc/fstab" ); - + cDebug() << " .. reading" << fstabFile.fileName(); - + if ( fstabFile.open( QIODevice::ReadOnly | QIODevice::Text ) ) { const QStringList fstabLines = QString::fromLocal8Bit( fstabFile.readAll() ) @@ -381,7 +400,7 @@ isEfiSystem() bool isEfiBootable( const Partition* candidate ) { - cDebug() << "Check EFI bootable" << candidate->partitionPath() << candidate->devicePath(); + cDebug() << "Check EFI bootable" << convenienceName( candidate ) << candidate->devicePath(); cDebug() << " .. flags" << candidate->activeFlags(); auto flags = PartitionInfo::flags( candidate ); From 92d9c9491a4549eb0e7e60059a1db98bd0e17147 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Feb 2019 07:05:32 -0500 Subject: [PATCH 2/5] [partition] Reduce lambda-happiness - Make some methods that are called mostly as slots, actual slots, instead of going through extra lambdas. - Use QOverload<>::of for disambiguation instead of homebrew casts. --- src/modules/partition/gui/PartitionPage.cpp | 25 ++++++++------------- src/modules/partition/gui/PartitionPage.h | 10 +++++++-- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index a2f2eab60..a8d58f1cd 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -90,22 +90,9 @@ PartitionPage::PartitionPage( PartitionCoreModule* core, QWidget* parent ) updateFromCurrentDevice(); - connect( m_ui->deviceComboBox, &QComboBox::currentTextChanged, - [ this ]( const QString& /* text */ ) - { - updateFromCurrentDevice(); - } ); - connect( m_ui->bootLoaderComboBox, static_cast(&QComboBox::activated), - [ this ]( const QString& /* text */ ) - { - m_lastSelectedBootLoaderIndex = m_ui->bootLoaderComboBox->currentIndex(); - } ); - - connect( m_ui->bootLoaderComboBox, &QComboBox::currentTextChanged, - [ this ]( const QString& /* text */ ) - { - updateBootLoaderInstallPath(); - } ); + connect( m_ui->deviceComboBox, &QComboBox::currentTextChanged, this, &PartitionPage::updateFromCurrentDevice ); + connect( m_ui->bootLoaderComboBox, QOverload::of(&QComboBox::activated), this, &PartitionPage::updateSelectedBootLoaderIndex ); + connect( m_ui->bootLoaderComboBox, &QComboBox::currentTextChanged, this, &PartitionPage::updateBootLoaderInstallPath ); connect( m_core, &PartitionCoreModule::isDirtyChanged, m_ui->revertButton, &QWidget::setEnabled ); @@ -512,6 +499,12 @@ PartitionPage::updateBootLoaderInstallPath() m_core->setBootLoaderInstallPath( var.toString() ); } +void +PartitionPage::updateSelectedBootLoaderIndex() +{ + m_lastSelectedBootLoaderIndex = m_ui->bootLoaderComboBox->currentIndex(); +} + void PartitionPage::updateFromCurrentDevice() { diff --git a/src/modules/partition/gui/PartitionPage.h b/src/modules/partition/gui/PartitionPage.h index 8289f2cdd..75d39c9dc 100644 --- a/src/modules/partition/gui/PartitionPage.h +++ b/src/modules/partition/gui/PartitionPage.h @@ -50,6 +50,14 @@ public: int selectedDeviceIndex(); void selectDeviceByIndex( int index ); +private slots: + /// @brief Update everything when the base device changes + void updateFromCurrentDevice(); + /// @brief Update when the selected device for boot loader changes + void updateBootLoaderInstallPath(); + /// @brief Explicitly selected boot loader path + void updateSelectedBootLoaderIndex(); + private: QScopedPointer< Ui_PartitionPage > m_ui; PartitionCoreModule* m_core; @@ -67,8 +75,6 @@ private: void updatePartitionToCreate( Device*, Partition* ); void editExistingPartition( Device*, Partition* ); - void updateBootLoaderInstallPath(); - void updateFromCurrentDevice(); void updateBootLoaderIndex(); /** From 943f3fb1f912600a3ee0c9aa0116c8e6c8474932 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Feb 2019 07:10:56 -0500 Subject: [PATCH 3/5] [partition] Improve debug-logging - Use cDebug() instead of qDebug() - Be more chatty when selecting a bootloader installation path --- src/modules/partition/gui/PartitionPage.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index a8d58f1cd..4e1e093b1 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -56,7 +56,6 @@ #include // Qt -#include #include #include #include @@ -495,7 +494,7 @@ PartitionPage::updateBootLoaderInstallPath() QVariant var = m_ui->bootLoaderComboBox->currentData( BootLoaderModel::BootLoaderPathRole ); if ( !var.isValid() ) return; - qDebug() << "PartitionPage::updateBootLoaderInstallPath" << var.toString(); + cDebug() << "PartitionPage::updateBootLoaderInstallPath" << var.toString(); m_core->setBootLoaderInstallPath( var.toString() ); } @@ -503,6 +502,7 @@ void PartitionPage::updateSelectedBootLoaderIndex() { m_lastSelectedBootLoaderIndex = m_ui->bootLoaderComboBox->currentIndex(); + cDebug() << "Selected bootloader index" << m_lastSelectedBootLoaderIndex; } void From b4cefff975138e621d9a9437891c9d1b09616b74 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Feb 2019 07:21:36 -0500 Subject: [PATCH 4/5] [partition] Avoid heap-wrangling - The CreatePartitionDialog doesn't need to be on the heap, it's modal here. Avoid QPointer weirdness as well. --- src/modules/partition/gui/PartitionPage.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index 4e1e093b1..5b8f17fd0 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -362,18 +362,18 @@ PartitionPage::onCreateClicked() if ( !checkCanCreate( model->device() ) ) return; - QPointer< CreatePartitionDialog > dlg = new CreatePartitionDialog( model->device(), - partition->parent(), - nullptr, - getCurrentUsedMountpoints(), - this ); - dlg->initFromFreeSpace( partition ); - if ( dlg->exec() == QDialog::Accepted ) + CreatePartitionDialog dlg( + model->device(), + partition->parent(), + nullptr, + getCurrentUsedMountpoints(), + this ); + dlg.initFromFreeSpace( partition ); + if ( dlg.exec() == QDialog::Accepted ) { - Partition* newPart = dlg->createPartition(); - m_core->createPartition( model->device(), newPart, dlg->newFlags() ); + Partition* newPart = dlg.createPartition(); + m_core->createPartition( model->device(), newPart, dlg.newFlags() ); } - delete dlg; } void From 3e067e617e9fed5e86a39aa87f861114da61f124 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 28 Feb 2019 10:26:17 -0500 Subject: [PATCH 5/5] [partition] Add accessor and documentation to BootLoaderInstallPath --- src/modules/partition/core/PartitionCoreModule.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/modules/partition/core/PartitionCoreModule.h b/src/modules/partition/core/PartitionCoreModule.h index c48c1562c..743f9c178 100644 --- a/src/modules/partition/core/PartitionCoreModule.h +++ b/src/modules/partition/core/PartitionCoreModule.h @@ -154,6 +154,9 @@ public: void setPartitionFlags( Device* device, Partition* partition, PartitionTable::Flags flags ); + /// @brief Retrieve the path where the bootloader will be installed + QString bootLoaderInstallPath() const { return m_bootLoaderInstallPath; } + /// @brief Set the path where the bootloader will be installed void setBootLoaderInstallPath( const QString& path ); void initLayout();