From 6d76a24b6f0257d8544cd17986dc291e7b05a3e0 Mon Sep 17 00:00:00 2001 From: Prachi194agrawal Date: Tue, 16 Dec 2025 09:54:39 +0530 Subject: [PATCH 1/8] Fix: Prevent UI reset when new device inserted during write (#825) When a new USB drive was inserted while another drive was being written to, the drive selection would automatically change to the newly inserted drive, causing the progress bar to disappear. The write process continued in the background, but users had no visual feedback. This fix checks if the currently selected drive has an active write operation (progress > 0 and < drive size) before changing the selection. If a write is in progress, the selection remains unchanged when new drives are connected, preserving the progress display. Fixes #825 Signed-off-by: Prachi194agrawal --- src/app/drivemanager.cpp | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/app/drivemanager.cpp b/src/app/drivemanager.cpp index 0ad1cb34..96dc792a 100644 --- a/src/app/drivemanager.cpp +++ b/src/app/drivemanager.cpp @@ -157,12 +157,26 @@ void DriveManager::onDriveConnected(Drive *d) endInsertRows(); emit drivesChanged(); - if (m_provider->initialized()) { - m_selectedIndex = position; - emit selectedChanged(); - } else { - m_selectedIndex = 0; - emit selectedChanged(); + // Fix for issue #825: Only change selection if the currently selected drive + // is not being written to or restored + bool currentDriveActive = false; + Drive *currentDrive = selected(); + if (currentDrive && currentDrive->progress()) { + qreal progress = currentDrive->progress()->value(); + qreal driveSize = currentDrive->size(); + // Check if there's an active write or restore operation + currentDriveActive = (progress > 0 && progress < driveSize); + } + + // Only auto-select the new drive if no operation is in progress + if (!currentDriveActive) { + if (m_provider->initialized()) { + m_selectedIndex = position; + emit selectedChanged(); + } else { + m_selectedIndex = 0; + emit selectedChanged(); + } } if (d->restoreStatus() == Drive::CONTAINS_LIVE) { From f8cfe68c795f93d0c86089fe06d3d2e68a1ef54f Mon Sep 17 00:00:00 2001 From: Prachi194agrawal Date: Tue, 13 Jan 2026 23:50:44 +0530 Subject: [PATCH 2/8] Refactor: Use state-based isBusy() instead of progress checking (#825) Replace progress-based logic with a cleaner state-based approach by adding an isBusy() method to the Drive class. This encapsulates the busy state within the Drive object itself, making the code more maintainable. Changes: - Add isBusy() getter method and m_isBusy member variable to Drive class - Set m_isBusy = true when write() or restore() operations start - Reset m_isBusy = false when operations complete in onFinished() callbacks - Update DriveManager::onDriveConnected() to check isBusy() instead of progress - Apply changes across all platform implementations (Linux, Mac, Windows) This prevents the UI from resetting when a new USB device is inserted during an active write or restore operation. Fixes #825 Signed-off-by: Prachi194agrawal --- src/app/drivemanager.cpp | 34 ++++++++++++++++------------------ src/app/drivemanager.h | 2 ++ src/app/linuxdrivemanager.cpp | 7 +++++++ src/app/macdrivemanager.cpp | 7 +++++++ src/app/windrivemanager.cpp | 7 +++++++ 5 files changed, 39 insertions(+), 18 deletions(-) diff --git a/src/app/drivemanager.cpp b/src/app/drivemanager.cpp index 96dc792a..6b4a6bc7 100644 --- a/src/app/drivemanager.cpp +++ b/src/app/drivemanager.cpp @@ -157,26 +157,18 @@ void DriveManager::onDriveConnected(Drive *d) endInsertRows(); emit drivesChanged(); - // Fix for issue #825: Only change selection if the currently selected drive - // is not being written to or restored - bool currentDriveActive = false; - Drive *currentDrive = selected(); - if (currentDrive && currentDrive->progress()) { - qreal progress = currentDrive->progress()->value(); - qreal driveSize = currentDrive->size(); - // Check if there's an active write or restore operation - currentDriveActive = (progress > 0 && progress < driveSize); + // Fix for issue #825: Only change selection if the currently selected drive is not busy + if (selected() && selected()->isBusy()) { + return; } - // Only auto-select the new drive if no operation is in progress - if (!currentDriveActive) { - if (m_provider->initialized()) { - m_selectedIndex = position; - emit selectedChanged(); - } else { - m_selectedIndex = 0; - emit selectedChanged(); - } + // Standard logic: Select the new drive if not busy + if (m_provider->initialized()) { + m_selectedIndex = position; + emit selectedChanged(); + } else { + m_selectedIndex = 0; + emit selectedChanged(); } if (d->restoreStatus() == Drive::CONTAINS_LIVE) { @@ -303,6 +295,11 @@ bool Drive::delayedWrite() const return m_delayedWrite; } +bool Drive::isBusy() const +{ + return m_isBusy; +} + void Drive::setDelayedWrite(const bool &o) { if (m_delayedWrite != o) { @@ -343,6 +340,7 @@ void Drive::cancel() m_error = QString(); m_restoreStatus = CLEAN; emit restoreStatusChanged(); + m_isBusy = false; } bool Drive::operator==(const Drive &o) const diff --git a/src/app/drivemanager.h b/src/app/drivemanager.h index 4e1d396c..8eba0522 100644 --- a/src/app/drivemanager.h +++ b/src/app/drivemanager.h @@ -169,6 +169,7 @@ class Drive : public QObject virtual qreal size() const; virtual RestoreStatus restoreStatus(); virtual bool delayedWrite() const; + virtual bool isBusy() const; virtual void setDelayedWrite(const bool &o); @@ -194,6 +195,7 @@ public slots: RestoreStatus m_restoreStatus{CLEAN}; QString m_error{}; bool m_delayedWrite{false}; + bool m_isBusy{false}; }; #endif // DRIVEMANAGER_H diff --git a/src/app/linuxdrivemanager.cpp b/src/app/linuxdrivemanager.cpp index b67c1ecd..12513593 100644 --- a/src/app/linuxdrivemanager.cpp +++ b/src/app/linuxdrivemanager.cpp @@ -210,6 +210,8 @@ bool LinuxDrive::write(ReleaseVariant *data) if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED) m_image->setStatus(ReleaseVariant::WRITING); + m_isBusy = true; // Set busy state + if (!m_process) m_process = new QProcess(this); @@ -271,6 +273,8 @@ void LinuxDrive::restore() { mDebug() << this->metaObject()->className() << "Will now restore" << this->m_device; + m_isBusy = true; // Set busy state + if (!m_process) m_process = new QProcess(this); @@ -339,6 +343,7 @@ void LinuxDrive::onFinished(int exitCode, QProcess::ExitStatus status) mDebug() << this->metaObject()->className() << "Helper process finished with status" << status; setDelayedWrite(false); + m_isBusy = false; // Operation complete if (!m_process) return; @@ -364,6 +369,8 @@ void LinuxDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus status) { mDebug() << this->metaObject()->className() << "Helper process finished with status" << status; + m_isBusy = false; // Operation complete + if (exitCode != 0) { if (m_process) mWarning() << "Drive restoration failed:" << m_process->readAllStandardError(); diff --git a/src/app/macdrivemanager.cpp b/src/app/macdrivemanager.cpp index 79cf8b65..8fc8a67f 100644 --- a/src/app/macdrivemanager.cpp +++ b/src/app/macdrivemanager.cpp @@ -87,6 +87,8 @@ bool MacDrive::write(ReleaseVariant *data) if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED) m_image->setStatus(ReleaseVariant::WRITING); + m_isBusy = true; // Set busy state + if (m_child) { // TODO some handling of an already present process m_child->deleteLater(); @@ -138,6 +140,8 @@ void MacDrive::restore() if (m_child) m_child->deleteLater(); + m_isBusy = true; // Set busy state + m_child = new QProcess(this); m_restoreStatus = RESTORING; @@ -172,6 +176,7 @@ void MacDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus) Q_UNUSED(exitStatus) setDelayedWrite(false); + m_isBusy = false; // Operation complete if (!m_child) return; @@ -197,6 +202,8 @@ void MacDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) if (!m_child) return; + m_isBusy = false; // Operation complete + mCritical() << "Process finished" << exitCode << exitStatus; mCritical() << m_child->readAllStandardError(); diff --git a/src/app/windrivemanager.cpp b/src/app/windrivemanager.cpp index 8bda71c4..01deea0f 100644 --- a/src/app/windrivemanager.cpp +++ b/src/app/windrivemanager.cpp @@ -129,6 +129,8 @@ bool WinDrive::write(ReleaseVariant *data) if (!Drive::write(data)) return false; + m_isBusy = true; // Set busy state + if (m_child) { // TODO some handling of an already present process m_child->deleteLater(); @@ -181,6 +183,8 @@ void WinDrive::restore() if (m_child) m_child->deleteLater(); + m_isBusy = true; // Set busy state + m_child = new QProcess(this); m_restoreStatus = RESTORING; @@ -227,6 +231,7 @@ bool WinDrive::operator==(const WinDrive &o) const void WinDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus) { setDelayedWrite(false); + m_isBusy = false; // Operation complete if (!m_child) return; @@ -255,6 +260,8 @@ void WinDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) return; } + m_isBusy = false; // Operation complete + mCritical() << "Process finished" << exitCode << exitStatus; mCritical() << m_child->readAllStandardError(); From 6d63fbcfdaf94d859464fe054883aa2396262d71 Mon Sep 17 00:00:00 2001 From: Prachi194agrawal Date: Thu, 15 Jan 2026 00:10:27 +0530 Subject: [PATCH 3/8] Refactor: Move QProcess to base Drive class and remove m_isBusy flag - Moved QProcess *m_process to Drive base class (shared by all platforms) - Removed manual m_isBusy boolean flag entirely - Implemented isBusy() to check actual QProcess::state() - Updated all platform implementations (Linux/Mac/Windows) to use inherited m_process - Renamed m_child to m_process in Mac and Windows implementations - Removed all manual busy flag assignments This provides a cleaner, state-based architecture where busy status is determined by the actual process state rather than manual tracking. --- src/app/drivemanager.cpp | 3 +- src/app/drivemanager.h | 3 +- src/app/linuxdrivemanager.cpp | 7 --- src/app/linuxdrivemanager.h | 2 - src/app/macdrivemanager.cpp | 77 ++++++++++++++----------------- src/app/macdrivemanager.h | 1 - src/app/windrivemanager.cpp | 87 ++++++++++++++++------------------- src/app/windrivemanager.h | 1 - 8 files changed, 78 insertions(+), 103 deletions(-) diff --git a/src/app/drivemanager.cpp b/src/app/drivemanager.cpp index 6b4a6bc7..7958e258 100644 --- a/src/app/drivemanager.cpp +++ b/src/app/drivemanager.cpp @@ -297,7 +297,7 @@ bool Drive::delayedWrite() const bool Drive::isBusy() const { - return m_isBusy; + return m_process && m_process->state() != QProcess::NotRunning; } void Drive::setDelayedWrite(const bool &o) @@ -340,7 +340,6 @@ void Drive::cancel() m_error = QString(); m_restoreStatus = CLEAN; emit restoreStatusChanged(); - m_isBusy = false; } bool Drive::operator==(const Drive &o) const diff --git a/src/app/drivemanager.h b/src/app/drivemanager.h index 8eba0522..9de46daa 100644 --- a/src/app/drivemanager.h +++ b/src/app/drivemanager.h @@ -22,6 +22,7 @@ #include #include +#include #include "releasemanager.h" @@ -195,7 +196,7 @@ public slots: RestoreStatus m_restoreStatus{CLEAN}; QString m_error{}; bool m_delayedWrite{false}; - bool m_isBusy{false}; + QProcess *m_process{nullptr}; }; #endif // DRIVEMANAGER_H diff --git a/src/app/linuxdrivemanager.cpp b/src/app/linuxdrivemanager.cpp index 12513593..b67c1ecd 100644 --- a/src/app/linuxdrivemanager.cpp +++ b/src/app/linuxdrivemanager.cpp @@ -210,8 +210,6 @@ bool LinuxDrive::write(ReleaseVariant *data) if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED) m_image->setStatus(ReleaseVariant::WRITING); - m_isBusy = true; // Set busy state - if (!m_process) m_process = new QProcess(this); @@ -273,8 +271,6 @@ void LinuxDrive::restore() { mDebug() << this->metaObject()->className() << "Will now restore" << this->m_device; - m_isBusy = true; // Set busy state - if (!m_process) m_process = new QProcess(this); @@ -343,7 +339,6 @@ void LinuxDrive::onFinished(int exitCode, QProcess::ExitStatus status) mDebug() << this->metaObject()->className() << "Helper process finished with status" << status; setDelayedWrite(false); - m_isBusy = false; // Operation complete if (!m_process) return; @@ -369,8 +364,6 @@ void LinuxDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus status) { mDebug() << this->metaObject()->className() << "Helper process finished with status" << status; - m_isBusy = false; // Operation complete - if (exitCode != 0) { if (m_process) mWarning() << "Drive restoration failed:" << m_process->readAllStandardError(); diff --git a/src/app/linuxdrivemanager.h b/src/app/linuxdrivemanager.h index 91b51311..2304a322 100644 --- a/src/app/linuxdrivemanager.h +++ b/src/app/linuxdrivemanager.h @@ -76,8 +76,6 @@ private slots: private: QString m_device; - - QProcess *m_process{nullptr}; }; #endif // LINUXDRIVEMANAGER_H diff --git a/src/app/macdrivemanager.cpp b/src/app/macdrivemanager.cpp index 8fc8a67f..93f7a53e 100644 --- a/src/app/macdrivemanager.cpp +++ b/src/app/macdrivemanager.cpp @@ -87,21 +87,19 @@ bool MacDrive::write(ReleaseVariant *data) if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED) m_image->setStatus(ReleaseVariant::WRITING); - m_isBusy = true; // Set busy state - - if (m_child) { + if (m_process) { // TODO some handling of an already present process - m_child->deleteLater(); + m_process->deleteLater(); } - m_child = new QProcess(this); - connect(m_child, static_cast(&QProcess::finished), this, &MacDrive::onFinished); - connect(m_child, &QProcess::readyRead, this, &MacDrive::onReadyRead); - connect(qApp, &QCoreApplication::aboutToQuit, m_child, &QProcess::terminate); + m_process = new QProcess(this); + connect(m_process, static_cast(&QProcess::finished), this, &MacDrive::onFinished); + connect(m_process, &QProcess::readyRead, this, &MacDrive::onReadyRead); + connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate); if (QFile::exists(qApp->applicationDirPath() + "/../../../../helper/mac/helper.app/Contents/MacOS/helper")) { - m_child->setProgram(QString("%1/../../../../helper/mac/helper.app/Contents/MacOS/helper").arg(qApp->applicationDirPath())); + m_process->setProgram(QString("%1/../../../../helper/mac/helper.app/Contents/MacOS/helper").arg(qApp->applicationDirPath())); } else if (QFile::exists(qApp->applicationDirPath() + "/helper")) { - m_child->setProgram(QString("%1/helper").arg(qApp->applicationDirPath())); + m_process->setProgram(QString("%1/helper").arg(qApp->applicationDirPath())); } else { data->setErrorString(tr("Could not find the helper binary. Check your installation.")); setDelayedWrite(false); @@ -116,10 +114,10 @@ bool MacDrive::write(ReleaseVariant *data) } args.append(m_bsdDevice); - mCritical() << "The command is" << m_child->program() << args; - m_child->setArguments(args); + mCritical() << "The command is" << m_process->program() << args; + m_process->setArguments(args); - m_child->start(); + m_process->start(); return true; } @@ -127,30 +125,28 @@ bool MacDrive::write(ReleaseVariant *data) void MacDrive::cancel() { Drive::cancel(); - if (m_child) { - m_child->kill(); - m_child->deleteLater(); - m_child = nullptr; + if (m_process) { + m_process->kill(); + m_process->deleteLater(); + m_process = nullptr; } } void MacDrive::restore() { mCritical() << "starting to restore"; - if (m_child) - m_child->deleteLater(); - - m_isBusy = true; // Set busy state + if (m_process) + m_process->deleteLater(); - m_child = new QProcess(this); + m_process = new QProcess(this); m_restoreStatus = RESTORING; emit restoreStatusChanged(); if (QFile::exists(qApp->applicationDirPath() + "/../../../../helper/mac/helper.app/Contents/MacOS/helper")) { - m_child->setProgram(QString("%1/../../../../helper/mac/helper.app/Contents/MacOS/helper").arg(qApp->applicationDirPath())); + m_process->setProgram(QString("%1/../../../../helper/mac/helper.app/Contents/MacOS/helper").arg(qApp->applicationDirPath())); } else if (QFile::exists(qApp->applicationDirPath() + "/helper")) { - m_child->setProgram(QString("%1/helper").arg(qApp->applicationDirPath())); + m_process->setProgram(QString("%1/helper").arg(qApp->applicationDirPath())); } else { m_restoreStatus = RESTORE_ERROR; return; @@ -159,16 +155,16 @@ void MacDrive::restore() QStringList args; args.append("restore"); args.append(m_bsdDevice); - m_child->setArguments(args); + m_process->setArguments(args); - m_child->setProcessChannelMode(QProcess::ForwardedChannels); + m_process->setProcessChannelMode(QProcess::ForwardedChannels); - connect(m_child, static_cast(&QProcess::finished), this, &MacDrive::onRestoreFinished); - connect(qApp, &QCoreApplication::aboutToQuit, m_child, &QProcess::terminate); + connect(m_process, static_cast(&QProcess::finished), this, &MacDrive::onRestoreFinished); + connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate); - mCritical() << "The command is" << m_child->program() << args; + mCritical() << "The command is" << m_process->program() << args; - m_child->start(); + m_process->start(); } void MacDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus) @@ -176,13 +172,12 @@ void MacDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus) Q_UNUSED(exitStatus) setDelayedWrite(false); - m_isBusy = false; // Operation complete - if (!m_child) + if (!m_process) return; if (exitCode != 0) { - QString output = m_child->readAllStandardError(); + QString output = m_process->readAllStandardError(); QRegularExpression re("^.+:.+: "); QStringList lines = output.split('\n'); if (lines.length() > 0) { @@ -199,13 +194,11 @@ void MacDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus) void MacDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) { - if (!m_child) + if (!m_process) return; - m_isBusy = false; // Operation complete - mCritical() << "Process finished" << exitCode << exitStatus; - mCritical() << m_child->readAllStandardError(); + mCritical() << m_process->readAllStandardError(); if (exitCode == 0) m_restoreStatus = RESTORED; @@ -213,13 +206,13 @@ void MacDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) m_restoreStatus = RESTORE_ERROR; emit restoreStatusChanged(); - m_child->deleteLater(); - m_child = nullptr; + m_process->deleteLater(); + m_process = nullptr; } void MacDrive::onReadyRead() { - if (!m_child) + if (!m_process) return; if (m_image->status() == ReleaseVariant::WRITING) { @@ -230,9 +223,9 @@ void MacDrive::onReadyRead() if (m_image->status() != ReleaseVariant::WRITE_VERIFYING && m_image->status() != ReleaseVariant::WRITING) m_image->setStatus(ReleaseVariant::WRITING); - while (m_child->bytesAvailable() > 0) { + while (m_process->bytesAvailable() > 0) { bool ok; - int64_t bytes = m_child->readLine().trimmed().toULongLong(&ok); + int64_t bytes = m_process->readLine().trimmed().toULongLong(&ok); if (ok) m_progress->setValue(bytes); } diff --git a/src/app/macdrivemanager.h b/src/app/macdrivemanager.h index 98fc9661..3c9d5960 100644 --- a/src/app/macdrivemanager.h +++ b/src/app/macdrivemanager.h @@ -60,7 +60,6 @@ private slots: private: QString m_bsdDevice; - QProcess *m_child{nullptr}; }; #endif // MACDRIVEMANAGER_H diff --git a/src/app/windrivemanager.cpp b/src/app/windrivemanager.cpp index 01deea0f..6b3fa86c 100644 --- a/src/app/windrivemanager.cpp +++ b/src/app/windrivemanager.cpp @@ -119,8 +119,8 @@ WinDrive::WinDrive(WinDriveProvider *parent, const QString &name, uint64_t size, WinDrive::~WinDrive() { - if (m_child) - m_child->terminate(); + if (m_process) + m_process->terminate(); } bool WinDrive::write(ReleaseVariant *data) @@ -129,24 +129,22 @@ bool WinDrive::write(ReleaseVariant *data) if (!Drive::write(data)) return false; - m_isBusy = true; // Set busy state - - if (m_child) { + if (m_process) { // TODO some handling of an already present process - m_child->deleteLater(); + m_process->deleteLater(); } - m_child = new QProcess(this); - connect(m_child, static_cast(&QProcess::finished), this, &WinDrive::onFinished); - connect(m_child, &QProcess::readyRead, this, &WinDrive::onReadyRead); - connect(qApp, &QCoreApplication::aboutToQuit, m_child, &QProcess::terminate); + m_process = new QProcess(this); + connect(m_process, static_cast(&QProcess::finished), this, &WinDrive::onFinished); + connect(m_process, &QProcess::readyRead, this, &WinDrive::onReadyRead); + connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate); if (data->status() != ReleaseVariant::DOWNLOADING) m_image->setStatus(ReleaseVariant::WRITING); if (QFile::exists(qApp->applicationDirPath() + "/helper.exe")) { - m_child->setProgram(qApp->applicationDirPath() + "/helper.exe"); + m_process->setProgram(qApp->applicationDirPath() + "/helper.exe"); } else if (QFile::exists(qApp->applicationDirPath() + "/../helper.exe")) { - m_child->setProgram(qApp->applicationDirPath() + "/../helper.exe"); + m_process->setProgram(qApp->applicationDirPath() + "/../helper.exe"); } else { data->setErrorString(tr("Could not find the helper binary. Check your installation.")); setDelayedWrite(false); @@ -160,40 +158,38 @@ bool WinDrive::write(ReleaseVariant *data) else args << data->temporaryPath(); args << QString("%1").arg(m_device); - m_child->setArguments(args); + m_process->setArguments(args); - mDebug() << this->metaObject()->className() << "Starting" << m_child->program() << args; - m_child->start(); + mDebug() << this->metaObject()->className() << "Starting" << m_process->program() << args; + m_process->start(); return true; } void WinDrive::cancel() { Drive::cancel(); - if (m_child) { - m_child->terminate(); - m_child->deleteLater(); - m_child = nullptr; + if (m_process) { + m_process->terminate(); + m_process->deleteLater(); + m_process = nullptr; } } void WinDrive::restore() { mDebug() << this->metaObject()->className() << "Preparing to restore disk" << m_device; - if (m_child) - m_child->deleteLater(); - - m_isBusy = true; // Set busy state + if (m_process) + m_process->deleteLater(); - m_child = new QProcess(this); + m_process = new QProcess(this); m_restoreStatus = RESTORING; emit restoreStatusChanged(); if (QFile::exists(qApp->applicationDirPath() + "/helper.exe")) { - m_child->setProgram(qApp->applicationDirPath() + "/helper.exe"); + m_process->setProgram(qApp->applicationDirPath() + "/helper.exe"); } else if (QFile::exists(qApp->applicationDirPath() + "/../helper.exe")) { - m_child->setProgram(qApp->applicationDirPath() + "/../helper.exe"); + m_process->setProgram(qApp->applicationDirPath() + "/../helper.exe"); } else { m_restoreStatus = RESTORE_ERROR; return; @@ -202,20 +198,20 @@ void WinDrive::restore() QStringList args; args << "restore"; args << QString("%1").arg(m_device); - m_child->setArguments(args); + m_process->setArguments(args); // connect(m_process, &QProcess::readyRead, this, &LinuxDrive::onReadyRead); - connect(m_child, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onRestoreFinished(int, QProcess::ExitStatus))); - connect(qApp, &QCoreApplication::aboutToQuit, m_child, &QProcess::terminate); + connect(m_process, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onRestoreFinished(int, QProcess::ExitStatus))); + connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate); - mDebug() << this->metaObject()->className() << "Starting" << m_child->program() << args; + mDebug() << this->metaObject()->className() << "Starting" << m_process->program() << args; - m_child->start(QIODevice::ReadOnly); + m_process->start(QIODevice::ReadOnly); } bool WinDrive::busy() const { - return (m_child && m_child->state() == QProcess::Running); + return (m_process && m_process->state() == QProcess::Running); } QString WinDrive::serialNumber() const @@ -231,39 +227,36 @@ bool WinDrive::operator==(const WinDrive &o) const void WinDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus) { setDelayedWrite(false); - m_isBusy = false; // Operation complete - if (!m_child) + if (!m_process) return; mDebug() << "Child finished" << exitCode << exitStatus; - mDebug() << m_child->errorString(); + mDebug() << m_process->errorString(); if (exitCode == 0) { m_image->setStatus(ReleaseVariant::FINISHED); Notifications::notify(tr("Finished!"), tr("Writing %1 was successful").arg(m_image->fullName())); } else if (exitCode == 1) { - m_image->setErrorString(m_child->readAllStandardError().trimmed()); + m_image->setErrorString(m_process->readAllStandardError().trimmed()); m_image->setStatus(ReleaseVariant::FAILED); } else if (exitCode == 2) { m_image->setErrorString(tr("Writing has been cancelled")); m_image->setStatus(ReleaseVariant::FAILED); } - m_child->deleteLater(); - m_child = nullptr; + m_process->deleteLater(); + m_process = nullptr; } void WinDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) { - if (!m_child) { + if (!m_process) { return; } - m_isBusy = false; // Operation complete - mCritical() << "Process finished" << exitCode << exitStatus; - mCritical() << m_child->readAllStandardError(); + mCritical() << m_process->readAllStandardError(); if (exitCode == 0) { m_restoreStatus = RESTORED; @@ -272,13 +265,13 @@ void WinDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) } emit restoreStatusChanged(); - m_child->deleteLater(); - m_child = nullptr; + m_process->deleteLater(); + m_process = nullptr; } void WinDrive::onReadyRead() { - if (!m_child) { + if (!m_process) { return; } @@ -289,8 +282,8 @@ void WinDrive::onReadyRead() m_image->setStatus(ReleaseVariant::WRITING); } - while (m_child->bytesAvailable() > 0) { - QString line = m_child->readLine().trimmed(); + while (m_process->bytesAvailable() > 0) { + QString line = m_process->readLine().trimmed(); if (line == "CHECK") { mDebug() << this->metaObject()->className() << "Written media check starting"; m_progress->setValue(0); diff --git a/src/app/windrivemanager.h b/src/app/windrivemanager.h index 54e9d094..993c35f4 100644 --- a/src/app/windrivemanager.h +++ b/src/app/windrivemanager.h @@ -66,7 +66,6 @@ private slots: private: int m_device; QString m_serialNo; - QProcess *m_child{nullptr}; }; #endif // WINDRIVEMANAGER_H From 9f5aa752665430049d394bd6c3ec540bbd69f794 Mon Sep 17 00:00:00 2001 From: Prachi194agrawal Date: Thu, 15 Jan 2026 20:52:52 +0530 Subject: [PATCH 4/8] Refactor: Use std::unique_ptr for QProcess management Implement mentor's suggestion to use RAII (Resource Acquisition Is Initialization) with std::unique_ptr and a custom deleter for process management. This modernizes the codebase and eliminates manual memory management issues. Changes: - Added DriveOperationDeleter struct with platform-specific cleanup logic - Changed m_process from raw QProcess* to std::unique_ptr with custom deleter - Removed all manual deleteLater() and nullptr assignments - Simplified cancel() methods across all Drive implementations - Reduced boilerplate code by ~30 lines across platforms Benefits: - Automatic cleanup prevents memory leaks even with exceptions - Centralized cleanup logic in one place (DriveOperationDeleter) - Safer and more maintainable code following modern C++ practices - Consistent behavior across Windows, Linux, and macOS platforms The DriveOperationDeleter handles: - Windows: kill() then waitForFinished() - Unix/Mac: terminate() with 3-second grace period, then kill() if needed This addresses the mentor's feedback on PR #914. Signed-off-by: Prachi194agrawal --- src/app/drivemanager.h | 31 +++++++++++++++++++++++- src/app/linuxdrivemanager.cpp | 44 ++++++++++++++--------------------- src/app/macdrivemanager.cpp | 32 ++++++++++--------------- src/app/windrivemanager.cpp | 42 +++++++++++++-------------------- 4 files changed, 76 insertions(+), 73 deletions(-) diff --git a/src/app/drivemanager.h b/src/app/drivemanager.h index 9de46daa..4e09caf7 100644 --- a/src/app/drivemanager.h +++ b/src/app/drivemanager.h @@ -23,6 +23,7 @@ #include #include #include +#include #include "releasemanager.h" @@ -31,6 +32,34 @@ class DriveProvider; class Drive; class UdisksDrive; +/** + * @brief Custom deleter for QProcess to ensure proper cleanup + * + * Handles platform-specific process termination before deletion + */ +struct DriveOperationDeleter { + void operator()(QProcess *process) { + if (!process) { + return; + } + + if (process->state() != QProcess::NotRunning) { +#ifdef Q_OS_WIN + process->kill(); + process->waitForFinished(); +#else + process->terminate(); + if (!process->waitForFinished(3000)) { + process->kill(); + process->waitForFinished(); + } +#endif + } + + delete process; + } +}; + /** * @brief The DriveManager class * @@ -196,7 +225,7 @@ public slots: RestoreStatus m_restoreStatus{CLEAN}; QString m_error{}; bool m_delayedWrite{false}; - QProcess *m_process{nullptr}; + std::unique_ptr m_process; }; #endif // DRIVEMANAGER_H diff --git a/src/app/linuxdrivemanager.cpp b/src/app/linuxdrivemanager.cpp index b67c1ecd..63be4daf 100644 --- a/src/app/linuxdrivemanager.cpp +++ b/src/app/linuxdrivemanager.cpp @@ -210,8 +210,8 @@ bool LinuxDrive::write(ReleaseVariant *data) if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED) m_image->setStatus(ReleaseVariant::WRITING); - if (!m_process) - m_process = new QProcess(this); + // Create new process using unique_ptr + m_process.reset(new QProcess(this)); QStringList args; if (QFile::exists(qApp->applicationDirPath() + "/../helper/linux/helper")) { @@ -235,11 +235,11 @@ bool LinuxDrive::write(ReleaseVariant *data) mDebug() << this->metaObject()->className() << "Helper command will be" << args; m_process->setArguments(args); - connect(m_process, &QProcess::readyRead, this, &LinuxDrive::onReadyRead); - connect(m_process, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onFinished(int, QProcess::ExitStatus))); + connect(m_process.get(), &QProcess::readyRead, this, &LinuxDrive::onReadyRead); + connect(m_process.get(), SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onFinished(int, QProcess::ExitStatus))); // TODO check if this is actually necessary - it should work just fine even without it - connect(m_process, &QProcess::errorOccurred, this, &LinuxDrive::onErrorOccurred); - connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate); + connect(m_process.get(), &QProcess::errorOccurred, this, &LinuxDrive::onErrorOccurred); + connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); m_process->start(QIODevice::ReadOnly); @@ -250,7 +250,7 @@ void LinuxDrive::cancel() { Drive::cancel(); static bool beingCancelled = false; - if (m_process != nullptr && !beingCancelled) { + if (m_process && !beingCancelled) { beingCancelled = true; if (m_image) { if (m_image->status() == ReleaseVariant::WRITE_VERIFYING) { @@ -260,9 +260,8 @@ void LinuxDrive::cancel() m_image->setStatus(ReleaseVariant::FAILED); } } - m_process->kill(); - m_process->deleteLater(); - m_process = nullptr; + // Reset the unique_ptr, which automatically invokes DriveOperationDeleter + m_process.reset(); beingCancelled = false; } } @@ -271,8 +270,8 @@ void LinuxDrive::restore() { mDebug() << this->metaObject()->className() << "Will now restore" << this->m_device; - if (!m_process) - m_process = new QProcess(this); + // Create new process using unique_ptr + m_process.reset(new QProcess(this)); m_restoreStatus = RESTORING; emit restoreStatusChanged(); @@ -294,9 +293,9 @@ void LinuxDrive::restore() mDebug() << this->metaObject()->className() << "Helper command will be" << args; m_process->setArguments(args); - connect(m_process, &QProcess::readyRead, this, &LinuxDrive::onReadyRead); - connect(m_process, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onRestoreFinished(int, QProcess::ExitStatus))); - connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate); + connect(m_process.get(), &QProcess::readyRead, this, &LinuxDrive::onReadyRead); + connect(m_process.get(), SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onRestoreFinished(int, QProcess::ExitStatus))); + connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); m_process->start(QIODevice::ReadOnly); } @@ -353,11 +352,8 @@ void LinuxDrive::onFinished(int exitCode, QProcess::ExitStatus status) } } - if (m_process) { - m_process->deleteLater(); - m_process = nullptr; - m_image = nullptr; - } + // Process cleanup handled automatically by unique_ptr deleter + m_image = nullptr; } void LinuxDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus status) @@ -373,10 +369,7 @@ void LinuxDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus status) } else { m_restoreStatus = RESTORED; } - if (m_process) { - m_process->deleteLater(); - m_process = nullptr; - } + // Process cleanup handled automatically by unique_ptr deleter emit restoreStatusChanged(); } @@ -389,8 +382,7 @@ void LinuxDrive::onErrorOccurred(QProcess::ProcessError e) QString errorMessage = m_process->errorString(); mWarning() << "Restoring failed:" << errorMessage; m_image->setErrorString(errorMessage); - m_process->deleteLater(); - m_process = nullptr; + // Process cleanup handled automatically by unique_ptr deleter m_image->setStatus(ReleaseVariant::FAILED); m_image = nullptr; } diff --git a/src/app/macdrivemanager.cpp b/src/app/macdrivemanager.cpp index 93f7a53e..b33aec87 100644 --- a/src/app/macdrivemanager.cpp +++ b/src/app/macdrivemanager.cpp @@ -87,14 +87,11 @@ bool MacDrive::write(ReleaseVariant *data) if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED) m_image->setStatus(ReleaseVariant::WRITING); - if (m_process) { - // TODO some handling of an already present process - m_process->deleteLater(); - } - m_process = new QProcess(this); - connect(m_process, static_cast(&QProcess::finished), this, &MacDrive::onFinished); - connect(m_process, &QProcess::readyRead, this, &MacDrive::onReadyRead); - connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate); + // Create new process using unique_ptr + m_process.reset(new QProcess(this)); + connect(m_process.get(), static_cast(&QProcess::finished), this, &MacDrive::onFinished); + connect(m_process.get(), &QProcess::readyRead, this, &MacDrive::onReadyRead); + connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); if (QFile::exists(qApp->applicationDirPath() + "/../../../../helper/mac/helper.app/Contents/MacOS/helper")) { m_process->setProgram(QString("%1/../../../../helper/mac/helper.app/Contents/MacOS/helper").arg(qApp->applicationDirPath())); @@ -125,20 +122,16 @@ bool MacDrive::write(ReleaseVariant *data) void MacDrive::cancel() { Drive::cancel(); - if (m_process) { - m_process->kill(); - m_process->deleteLater(); - m_process = nullptr; - } + // Reset the unique_ptr, which automatically invokes DriveOperationDeleter + m_process.reset(); } void MacDrive::restore() { mCritical() << "starting to restore"; - if (m_process) - m_process->deleteLater(); - m_process = new QProcess(this); + // Create new process using unique_ptr + m_process.reset(new QProcess(this)); m_restoreStatus = RESTORING; emit restoreStatusChanged(); @@ -159,8 +152,8 @@ void MacDrive::restore() m_process->setProcessChannelMode(QProcess::ForwardedChannels); - connect(m_process, static_cast(&QProcess::finished), this, &MacDrive::onRestoreFinished); - connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate); + connect(m_process.get(), static_cast(&QProcess::finished), this, &MacDrive::onRestoreFinished); + connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); mCritical() << "The command is" << m_process->program() << args; @@ -206,8 +199,7 @@ void MacDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) m_restoreStatus = RESTORE_ERROR; emit restoreStatusChanged(); - m_process->deleteLater(); - m_process = nullptr; + // Process cleanup handled automatically by unique_ptr deleter } void MacDrive::onReadyRead() diff --git a/src/app/windrivemanager.cpp b/src/app/windrivemanager.cpp index 6b3fa86c..fd10536d 100644 --- a/src/app/windrivemanager.cpp +++ b/src/app/windrivemanager.cpp @@ -119,8 +119,7 @@ WinDrive::WinDrive(WinDriveProvider *parent, const QString &name, uint64_t size, WinDrive::~WinDrive() { - if (m_process) - m_process->terminate(); + // Process cleanup is handled automatically by std::unique_ptr with DriveOperationDeleter } bool WinDrive::write(ReleaseVariant *data) @@ -129,14 +128,11 @@ bool WinDrive::write(ReleaseVariant *data) if (!Drive::write(data)) return false; - if (m_process) { - // TODO some handling of an already present process - m_process->deleteLater(); - } - m_process = new QProcess(this); - connect(m_process, static_cast(&QProcess::finished), this, &WinDrive::onFinished); - connect(m_process, &QProcess::readyRead, this, &WinDrive::onReadyRead); - connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate); + // Create new process using unique_ptr + m_process.reset(new QProcess(this)); + connect(m_process.get(), static_cast(&QProcess::finished), this, &WinDrive::onFinished); + connect(m_process.get(), &QProcess::readyRead, this, &WinDrive::onReadyRead); + connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); if (data->status() != ReleaseVariant::DOWNLOADING) m_image->setStatus(ReleaseVariant::WRITING); @@ -168,20 +164,16 @@ bool WinDrive::write(ReleaseVariant *data) void WinDrive::cancel() { Drive::cancel(); - if (m_process) { - m_process->terminate(); - m_process->deleteLater(); - m_process = nullptr; - } + // Reset the unique_ptr, which automatically invokes DriveOperationDeleter + m_process.reset(); } void WinDrive::restore() { mDebug() << this->metaObject()->className() << "Preparing to restore disk" << m_device; - if (m_process) - m_process->deleteLater(); - - m_process = new QProcess(this); + + // Create new process using unique_ptr + m_process.reset(new QProcess(this)); m_restoreStatus = RESTORING; emit restoreStatusChanged(); @@ -200,9 +192,9 @@ void WinDrive::restore() args << QString("%1").arg(m_device); m_process->setArguments(args); - // connect(m_process, &QProcess::readyRead, this, &LinuxDrive::onReadyRead); - connect(m_process, SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onRestoreFinished(int, QProcess::ExitStatus))); - connect(qApp, &QCoreApplication::aboutToQuit, m_process, &QProcess::terminate); + // connect(m_process.get(), &QProcess::readyRead, this, &LinuxDrive::onReadyRead); + connect(m_process.get(), SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onRestoreFinished(int, QProcess::ExitStatus))); + connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); mDebug() << this->metaObject()->className() << "Starting" << m_process->program() << args; @@ -245,8 +237,7 @@ void WinDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus) m_image->setStatus(ReleaseVariant::FAILED); } - m_process->deleteLater(); - m_process = nullptr; + // Process cleanup handled automatically by unique_ptr deleter } void WinDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) @@ -265,8 +256,7 @@ void WinDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) } emit restoreStatusChanged(); - m_process->deleteLater(); - m_process = nullptr; + // Process cleanup handled automatically by unique_ptr deleter } void WinDrive::onReadyRead() From 3169c70e785a94034d5c4a767babb033de1dca37 Mon Sep 17 00:00:00 2001 From: Prachi194agrawal Date: Thu, 15 Jan 2026 22:39:00 +0530 Subject: [PATCH 5/8] Address code review feedback: clean up RAII implementation - Remove redundant comment in drivemanager.cpp - Move m_process.reset() to base Drive::cancel() method - Remove duplicate reset() calls from platform-specific cancel() methods - Add explicit reset() calls in LinuxDrive completion handlers - Simplify WinDrive destructor to use default - Remove all implementation comments --- src/app/drivemanager.cpp | 2 +- src/app/linuxdrivemanager.cpp | 9 +++------ src/app/macdrivemanager.cpp | 5 ----- src/app/windrivemanager.cpp | 12 ------------ src/app/windrivemanager.h | 2 +- 5 files changed, 5 insertions(+), 25 deletions(-) diff --git a/src/app/drivemanager.cpp b/src/app/drivemanager.cpp index 7958e258..0b7d57ff 100644 --- a/src/app/drivemanager.cpp +++ b/src/app/drivemanager.cpp @@ -162,7 +162,6 @@ void DriveManager::onDriveConnected(Drive *d) return; } - // Standard logic: Select the new drive if not busy if (m_provider->initialized()) { m_selectedIndex = position; emit selectedChanged(); @@ -340,6 +339,7 @@ void Drive::cancel() m_error = QString(); m_restoreStatus = CLEAN; emit restoreStatusChanged(); + m_process.reset(); } bool Drive::operator==(const Drive &o) const diff --git a/src/app/linuxdrivemanager.cpp b/src/app/linuxdrivemanager.cpp index 63be4daf..2cc37279 100644 --- a/src/app/linuxdrivemanager.cpp +++ b/src/app/linuxdrivemanager.cpp @@ -260,8 +260,6 @@ void LinuxDrive::cancel() m_image->setStatus(ReleaseVariant::FAILED); } } - // Reset the unique_ptr, which automatically invokes DriveOperationDeleter - m_process.reset(); beingCancelled = false; } } @@ -270,7 +268,6 @@ void LinuxDrive::restore() { mDebug() << this->metaObject()->className() << "Will now restore" << this->m_device; - // Create new process using unique_ptr m_process.reset(new QProcess(this)); m_restoreStatus = RESTORING; @@ -352,7 +349,7 @@ void LinuxDrive::onFinished(int exitCode, QProcess::ExitStatus status) } } - // Process cleanup handled automatically by unique_ptr deleter + m_process.reset(); m_image = nullptr; } @@ -369,7 +366,7 @@ void LinuxDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus status) } else { m_restoreStatus = RESTORED; } - // Process cleanup handled automatically by unique_ptr deleter + m_process.reset(); emit restoreStatusChanged(); } @@ -382,7 +379,7 @@ void LinuxDrive::onErrorOccurred(QProcess::ProcessError e) QString errorMessage = m_process->errorString(); mWarning() << "Restoring failed:" << errorMessage; m_image->setErrorString(errorMessage); - // Process cleanup handled automatically by unique_ptr deleter + m_process.reset(); m_image->setStatus(ReleaseVariant::FAILED); m_image = nullptr; } diff --git a/src/app/macdrivemanager.cpp b/src/app/macdrivemanager.cpp index b33aec87..ab2940c9 100644 --- a/src/app/macdrivemanager.cpp +++ b/src/app/macdrivemanager.cpp @@ -122,15 +122,12 @@ bool MacDrive::write(ReleaseVariant *data) void MacDrive::cancel() { Drive::cancel(); - // Reset the unique_ptr, which automatically invokes DriveOperationDeleter - m_process.reset(); } void MacDrive::restore() { mCritical() << "starting to restore"; - // Create new process using unique_ptr m_process.reset(new QProcess(this)); m_restoreStatus = RESTORING; @@ -198,8 +195,6 @@ void MacDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) else m_restoreStatus = RESTORE_ERROR; emit restoreStatusChanged(); - - // Process cleanup handled automatically by unique_ptr deleter } void MacDrive::onReadyRead() diff --git a/src/app/windrivemanager.cpp b/src/app/windrivemanager.cpp index fd10536d..2091d4d2 100644 --- a/src/app/windrivemanager.cpp +++ b/src/app/windrivemanager.cpp @@ -117,11 +117,6 @@ WinDrive::WinDrive(WinDriveProvider *parent, const QString &name, uint64_t size, { } -WinDrive::~WinDrive() -{ - // Process cleanup is handled automatically by std::unique_ptr with DriveOperationDeleter -} - bool WinDrive::write(ReleaseVariant *data) { mDebug() << this->metaObject()->className() << "Preparing to write" << data->fullName() << "to drive" << m_device; @@ -164,15 +159,12 @@ bool WinDrive::write(ReleaseVariant *data) void WinDrive::cancel() { Drive::cancel(); - // Reset the unique_ptr, which automatically invokes DriveOperationDeleter - m_process.reset(); } void WinDrive::restore() { mDebug() << this->metaObject()->className() << "Preparing to restore disk" << m_device; - // Create new process using unique_ptr m_process.reset(new QProcess(this)); m_restoreStatus = RESTORING; @@ -236,8 +228,6 @@ void WinDrive::onFinished(int exitCode, QProcess::ExitStatus exitStatus) m_image->setErrorString(tr("Writing has been cancelled")); m_image->setStatus(ReleaseVariant::FAILED); } - - // Process cleanup handled automatically by unique_ptr deleter } void WinDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) @@ -255,8 +245,6 @@ void WinDrive::onRestoreFinished(int exitCode, QProcess::ExitStatus exitStatus) m_restoreStatus = RESTORE_ERROR; } emit restoreStatusChanged(); - - // Process cleanup handled automatically by unique_ptr deleter } void WinDrive::onReadyRead() diff --git a/src/app/windrivemanager.h b/src/app/windrivemanager.h index 993c35f4..add8c846 100644 --- a/src/app/windrivemanager.h +++ b/src/app/windrivemanager.h @@ -47,7 +47,7 @@ class WinDrive : public Drive Q_OBJECT public: WinDrive(WinDriveProvider *parent, const QString &name, uint64_t size, bool containsLive, int device, const QString &serialNumber); - ~WinDrive(); + ~WinDrive() = default; Q_INVOKABLE virtual bool write(ReleaseVariant *data) override; Q_INVOKABLE virtual void cancel() override; From f1972ee2fb0b5630481a264dfced6d7b78b5e094 Mon Sep 17 00:00:00 2001 From: Prachi194agrawal Date: Fri, 16 Jan 2026 14:28:15 +0530 Subject: [PATCH 6/8] Refactor QProcess management to use std::make_unique and remove platform-specific cancel() overrides --- src/app/drivemanager.cpp | 1 - src/app/linuxdrivemanager.cpp | 32 ++------------------------------ src/app/linuxdrivemanager.h | 2 -- src/app/macdrivemanager.cpp | 10 ++-------- src/app/macdrivemanager.h | 1 - src/app/windrivemanager.cpp | 14 +++----------- src/app/windrivemanager.h | 2 -- 7 files changed, 7 insertions(+), 55 deletions(-) diff --git a/src/app/drivemanager.cpp b/src/app/drivemanager.cpp index 0b7d57ff..3c26545e 100644 --- a/src/app/drivemanager.cpp +++ b/src/app/drivemanager.cpp @@ -157,7 +157,6 @@ void DriveManager::onDriveConnected(Drive *d) endInsertRows(); emit drivesChanged(); - // Fix for issue #825: Only change selection if the currently selected drive is not busy if (selected() && selected()->isBusy()) { return; } diff --git a/src/app/linuxdrivemanager.cpp b/src/app/linuxdrivemanager.cpp index 2cc37279..ae2c0045 100644 --- a/src/app/linuxdrivemanager.cpp +++ b/src/app/linuxdrivemanager.cpp @@ -193,14 +193,6 @@ LinuxDrive::LinuxDrive(LinuxDriveProvider *parent, QString device, QString name, { } -LinuxDrive::~LinuxDrive() -{ - if (m_image && m_image->status() == ReleaseVariant::WRITING) { - m_image->setErrorString(tr("The drive was removed while it was written to.")); - m_image->setStatus(ReleaseVariant::FAILED); - } -} - bool LinuxDrive::write(ReleaseVariant *data) { mDebug() << this->metaObject()->className() << "Will now write" << data->iso() << "to" << this->m_device; @@ -210,8 +202,7 @@ bool LinuxDrive::write(ReleaseVariant *data) if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED) m_image->setStatus(ReleaseVariant::WRITING); - // Create new process using unique_ptr - m_process.reset(new QProcess(this)); + m_process = std::make_unique(this); QStringList args; if (QFile::exists(qApp->applicationDirPath() + "/../helper/linux/helper")) { @@ -237,7 +228,6 @@ bool LinuxDrive::write(ReleaseVariant *data) connect(m_process.get(), &QProcess::readyRead, this, &LinuxDrive::onReadyRead); connect(m_process.get(), SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onFinished(int, QProcess::ExitStatus))); - // TODO check if this is actually necessary - it should work just fine even without it connect(m_process.get(), &QProcess::errorOccurred, this, &LinuxDrive::onErrorOccurred); connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); @@ -246,29 +236,11 @@ bool LinuxDrive::write(ReleaseVariant *data) return true; } -void LinuxDrive::cancel() -{ - Drive::cancel(); - static bool beingCancelled = false; - if (m_process && !beingCancelled) { - beingCancelled = true; - if (m_image) { - if (m_image->status() == ReleaseVariant::WRITE_VERIFYING) { - m_image->setStatus(ReleaseVariant::FINISHED); - } else if (m_image->status() != ReleaseVariant::DOWNLOADING && m_image->status() != ReleaseVariant::DOWNLOAD_VERIFYING) { - m_image->setErrorString(tr("Stopped before writing has finished.")); - m_image->setStatus(ReleaseVariant::FAILED); - } - } - beingCancelled = false; - } -} - void LinuxDrive::restore() { mDebug() << this->metaObject()->className() << "Will now restore" << this->m_device; - m_process.reset(new QProcess(this)); + m_process = std::make_unique(this); m_restoreStatus = RESTORING; emit restoreStatusChanged(); diff --git a/src/app/linuxdrivemanager.h b/src/app/linuxdrivemanager.h index 2304a322..d09ad2ef 100644 --- a/src/app/linuxdrivemanager.h +++ b/src/app/linuxdrivemanager.h @@ -62,10 +62,8 @@ class LinuxDrive : public Drive Q_OBJECT public: LinuxDrive(LinuxDriveProvider *parent, QString device, QString name, uint64_t size, bool isoLayout); - ~LinuxDrive(); Q_INVOKABLE virtual bool write(ReleaseVariant *data) override; - Q_INVOKABLE virtual void cancel() override; Q_INVOKABLE virtual void restore() override; private slots: diff --git a/src/app/macdrivemanager.cpp b/src/app/macdrivemanager.cpp index ab2940c9..5f5959af 100644 --- a/src/app/macdrivemanager.cpp +++ b/src/app/macdrivemanager.cpp @@ -87,8 +87,7 @@ bool MacDrive::write(ReleaseVariant *data) if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED) m_image->setStatus(ReleaseVariant::WRITING); - // Create new process using unique_ptr - m_process.reset(new QProcess(this)); + m_process = std::make_unique(this); connect(m_process.get(), static_cast(&QProcess::finished), this, &MacDrive::onFinished); connect(m_process.get(), &QProcess::readyRead, this, &MacDrive::onReadyRead); connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); @@ -119,16 +118,11 @@ bool MacDrive::write(ReleaseVariant *data) return true; } -void MacDrive::cancel() -{ - Drive::cancel(); -} - void MacDrive::restore() { mCritical() << "starting to restore"; - m_process.reset(new QProcess(this)); + m_process = std::make_unique(this); m_restoreStatus = RESTORING; emit restoreStatusChanged(); diff --git a/src/app/macdrivemanager.h b/src/app/macdrivemanager.h index 3c9d5960..b263b493 100644 --- a/src/app/macdrivemanager.h +++ b/src/app/macdrivemanager.h @@ -51,7 +51,6 @@ class MacDrive : public Drive MacDrive(DriveProvider *parent, const QString &name, uint64_t size, bool containsLive, const QString &bsdDevice); Q_INVOKABLE virtual bool write(ReleaseVariant *data) override; - Q_INVOKABLE virtual void cancel() override; Q_INVOKABLE virtual void restore() override; private slots: void onFinished(int exitCode, QProcess::ExitStatus exitStatus); diff --git a/src/app/windrivemanager.cpp b/src/app/windrivemanager.cpp index 2091d4d2..52e78c32 100644 --- a/src/app/windrivemanager.cpp +++ b/src/app/windrivemanager.cpp @@ -99,7 +99,6 @@ void WinDriveProvider::checkDrives() driveIndexes.removeAll(it.key()); } - // Remove our previously stored drives that were not present in the last check for (int index : driveIndexes) { mDebug() << "Removing old drive with index" << index; emit driveRemoved(m_drives[index]); @@ -123,8 +122,7 @@ bool WinDrive::write(ReleaseVariant *data) if (!Drive::write(data)) return false; - // Create new process using unique_ptr - m_process.reset(new QProcess(this)); + m_process = std::make_unique(this); connect(m_process.get(), static_cast(&QProcess::finished), this, &WinDrive::onFinished); connect(m_process.get(), &QProcess::readyRead, this, &WinDrive::onReadyRead); connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); @@ -156,16 +154,11 @@ bool WinDrive::write(ReleaseVariant *data) return true; } -void WinDrive::cancel() -{ - Drive::cancel(); -} - void WinDrive::restore() { mDebug() << this->metaObject()->className() << "Preparing to restore disk" << m_device; - - m_process.reset(new QProcess(this)); + + m_process = std::make_unique(this); m_restoreStatus = RESTORING; emit restoreStatusChanged(); @@ -184,7 +177,6 @@ void WinDrive::restore() args << QString("%1").arg(m_device); m_process->setArguments(args); - // connect(m_process.get(), &QProcess::readyRead, this, &LinuxDrive::onReadyRead); connect(m_process.get(), SIGNAL(finished(int, QProcess::ExitStatus)), this, SLOT(onRestoreFinished(int, QProcess::ExitStatus))); connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); diff --git a/src/app/windrivemanager.h b/src/app/windrivemanager.h index add8c846..46c2aa30 100644 --- a/src/app/windrivemanager.h +++ b/src/app/windrivemanager.h @@ -47,10 +47,8 @@ class WinDrive : public Drive Q_OBJECT public: WinDrive(WinDriveProvider *parent, const QString &name, uint64_t size, bool containsLive, int device, const QString &serialNumber); - ~WinDrive() = default; Q_INVOKABLE virtual bool write(ReleaseVariant *data) override; - Q_INVOKABLE virtual void cancel() override; Q_INVOKABLE virtual void restore() override; bool busy() const; From 56c55a798904b4aed2458901f0e5adf1c8fdf33b Mon Sep 17 00:00:00 2001 From: Prachi194agrawal Date: Fri, 16 Jan 2026 17:28:30 +0530 Subject: [PATCH 7/8] Fix: Revert to .reset() because make_unique is incompatible with custom deleter --- src/app/linuxdrivemanager.cpp | 4 ++-- src/app/macdrivemanager.cpp | 4 ++-- src/app/windrivemanager.cpp | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/app/linuxdrivemanager.cpp b/src/app/linuxdrivemanager.cpp index ae2c0045..51738118 100644 --- a/src/app/linuxdrivemanager.cpp +++ b/src/app/linuxdrivemanager.cpp @@ -202,7 +202,7 @@ bool LinuxDrive::write(ReleaseVariant *data) if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED) m_image->setStatus(ReleaseVariant::WRITING); - m_process = std::make_unique(this); + m_process.reset(new QProcess(this)); QStringList args; if (QFile::exists(qApp->applicationDirPath() + "/../helper/linux/helper")) { @@ -240,7 +240,7 @@ void LinuxDrive::restore() { mDebug() << this->metaObject()->className() << "Will now restore" << this->m_device; - m_process = std::make_unique(this); + m_process.reset(new QProcess(this)); m_restoreStatus = RESTORING; emit restoreStatusChanged(); diff --git a/src/app/macdrivemanager.cpp b/src/app/macdrivemanager.cpp index 5f5959af..b5102e00 100644 --- a/src/app/macdrivemanager.cpp +++ b/src/app/macdrivemanager.cpp @@ -87,7 +87,7 @@ bool MacDrive::write(ReleaseVariant *data) if (m_image->status() == ReleaseVariant::READY || m_image->status() == ReleaseVariant::FAILED || m_image->status() == ReleaseVariant::FAILED_VERIFICATION || m_image->status() == ReleaseVariant::FINISHED) m_image->setStatus(ReleaseVariant::WRITING); - m_process = std::make_unique(this); + m_process.reset(new QProcess(this)); connect(m_process.get(), static_cast(&QProcess::finished), this, &MacDrive::onFinished); connect(m_process.get(), &QProcess::readyRead, this, &MacDrive::onReadyRead); connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); @@ -122,7 +122,7 @@ void MacDrive::restore() { mCritical() << "starting to restore"; - m_process = std::make_unique(this); + m_process.reset(new QProcess(this)); m_restoreStatus = RESTORING; emit restoreStatusChanged(); diff --git a/src/app/windrivemanager.cpp b/src/app/windrivemanager.cpp index 52e78c32..3b7d65fb 100644 --- a/src/app/windrivemanager.cpp +++ b/src/app/windrivemanager.cpp @@ -122,7 +122,7 @@ bool WinDrive::write(ReleaseVariant *data) if (!Drive::write(data)) return false; - m_process = std::make_unique(this); + m_process.reset(new QProcess(this)); connect(m_process.get(), static_cast(&QProcess::finished), this, &WinDrive::onFinished); connect(m_process.get(), &QProcess::readyRead, this, &WinDrive::onReadyRead); connect(qApp, &QCoreApplication::aboutToQuit, m_process.get(), &QProcess::terminate); @@ -158,7 +158,7 @@ void WinDrive::restore() { mDebug() << this->metaObject()->className() << "Preparing to restore disk" << m_device; - m_process = std::make_unique(this); + m_process.reset(new QProcess(this)); m_restoreStatus = RESTORING; emit restoreStatusChanged(); From de28ba074141f89ebe8716837c94927283b8f498 Mon Sep 17 00:00:00 2001 From: Prachi194agrawal Date: Sun, 18 Jan 2026 11:57:14 +0530 Subject: [PATCH 8/8] Fix: Apply clang-format to DriveOperationDeleter --- src/app/drivemanager.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/app/drivemanager.h b/src/app/drivemanager.h index 4e09caf7..959dfa17 100644 --- a/src/app/drivemanager.h +++ b/src/app/drivemanager.h @@ -34,11 +34,12 @@ class UdisksDrive; /** * @brief Custom deleter for QProcess to ensure proper cleanup - * + * * Handles platform-specific process termination before deletion */ struct DriveOperationDeleter { - void operator()(QProcess *process) { + void operator()(QProcess *process) + { if (!process) { return; }