From 8fedcb6ba9a8b63d8d749f8de46524151078bffa Mon Sep 17 00:00:00 2001 From: hjk Date: Tue, 3 Dec 2024 10:10:43 +0100 Subject: [PATCH] ProjectExplorer: Create a thin DeviceRef wrapper class ... around a weak pointer to IDevice to pass around device identity. The idea is to ramp down the direct use, especially storage, of IDevice::{Const,}Ptr outside the DeviceManager and use DeviceRef instead, in order to avoid the problem of device shared pointers not dying when copies are held in dormant dialogs or similar. The separate class effectively acts as a weak pointer with somewhat nicer syntax. Change-Id: If735ed8af9589137640b73b023d04c063ea876be Reviewed-by: Christian Kandeler --- .../projectexplorer/devicesupport/idevice.cpp | 53 +++++++++++++++++++ .../projectexplorer/devicesupport/idevice.h | 27 ++++++++++ .../devicesupport/idevicefwd.h | 3 ++ .../remotelinux/publickeydeploymentdialog.cpp | 14 ++--- .../remotelinux/publickeydeploymentdialog.h | 9 ++-- src/plugins/remotelinux/sshdevicewizard.cpp | 28 +++++----- src/plugins/remotelinux/sshdevicewizard.h | 5 +- 7 files changed, 113 insertions(+), 26 deletions(-) diff --git a/src/plugins/projectexplorer/devicesupport/idevice.cpp b/src/plugins/projectexplorer/devicesupport/idevice.cpp index 96b9c2e0ca6..bb5479539f6 100644 --- a/src/plugins/projectexplorer/devicesupport/idevice.cpp +++ b/src/plugins/projectexplorer/devicesupport/idevice.cpp @@ -794,4 +794,57 @@ void DeviceProcessKillerTaskAdapter::start() task()->start(); } +// DeviceConstRef + +DeviceConstRef::DeviceConstRef(const IDevice::ConstPtr &device) + : m_constDevice(device) +{} + +DeviceConstRef::DeviceConstRef(const IDevice::Ptr &device) + : m_constDevice(device) +{} + +DeviceConstRef::~DeviceConstRef() = default; + +Id DeviceConstRef::id() const +{ + const IDevice::ConstPtr device = m_constDevice.lock(); + QTC_ASSERT(device, return {}); + return device->id(); +} + +QString DeviceConstRef::displayName() const +{ + const IDevice::ConstPtr device = m_constDevice.lock(); + QTC_ASSERT(device, return {}); + return device->displayName(); +} + +SshParameters DeviceConstRef::sshParameters() const +{ + const IDevice::ConstPtr device = m_constDevice.lock(); + QTC_ASSERT(device, return {}); + return device->sshParameters(); +} + +// DeviceRef, mutable + +DeviceRef::DeviceRef(const IDevice::Ptr &device) + : DeviceConstRef(device), m_mutableDevice(device) +{} + +void DeviceRef::setDisplayName(const QString &displayName) +{ + const IDevice::Ptr device = m_mutableDevice.lock(); + QTC_ASSERT(device, return); + device->setDisplayName(displayName); +} + +void DeviceRef::setSshParameters(const SshParameters ¶ms) +{ + const IDevice::Ptr device = m_mutableDevice.lock(); + QTC_ASSERT(device, return); + device->setSshParameters(params); +} + } // namespace ProjectExplorer diff --git a/src/plugins/projectexplorer/devicesupport/idevice.h b/src/plugins/projectexplorer/devicesupport/idevice.h index ec6220d30b1..58737ec6c7a 100644 --- a/src/plugins/projectexplorer/devicesupport/idevice.h +++ b/src/plugins/projectexplorer/devicesupport/idevice.h @@ -245,6 +245,33 @@ private: friend class DeviceManager; }; +class PROJECTEXPLORER_EXPORT DeviceConstRef +{ +public: + DeviceConstRef(const IDevice::ConstPtr &device); + DeviceConstRef(const IDevice::Ptr &device); + virtual ~DeviceConstRef(); + + Utils::Id id() const; + QString displayName() const; + SshParameters sshParameters() const; + +private: + std::weak_ptr m_constDevice; +}; + +class PROJECTEXPLORER_EXPORT DeviceRef : public DeviceConstRef +{ +public: + DeviceRef(const IDevice::Ptr &device); + + void setDisplayName(const QString &displayName); + void setSshParameters(const SshParameters ¶ms); + +private: + std::weak_ptr m_mutableDevice; +}; + class PROJECTEXPLORER_EXPORT DeviceTester : public QObject { Q_OBJECT diff --git a/src/plugins/projectexplorer/devicesupport/idevicefwd.h b/src/plugins/projectexplorer/devicesupport/idevicefwd.h index 533e88beb1c..236c10f2d31 100644 --- a/src/plugins/projectexplorer/devicesupport/idevicefwd.h +++ b/src/plugins/projectexplorer/devicesupport/idevicefwd.h @@ -7,6 +7,9 @@ namespace ProjectExplorer { +class DeviceRef; +class DeviceConstRef; + class IDevice; using IDevicePtr = std::shared_ptr; diff --git a/src/plugins/remotelinux/publickeydeploymentdialog.cpp b/src/plugins/remotelinux/publickeydeploymentdialog.cpp index b0da7f2b1d0..c7fdaaf50f9 100644 --- a/src/plugins/remotelinux/publickeydeploymentdialog.cpp +++ b/src/plugins/remotelinux/publickeydeploymentdialog.cpp @@ -28,19 +28,21 @@ public: }; PublicKeyDeploymentDialog *PublicKeyDeploymentDialog::createDialog( - const IDevice::ConstPtr &deviceConfig, QWidget *parent) + const DeviceConstRef &device, QWidget *parent) { - const FilePath dir = deviceConfig->sshParameters().privateKeyFile.parentDir(); + const FilePath dir = device.sshParameters().privateKeyFile.parentDir(); const FilePath publicKeyFileName = FileUtils::getOpenFilePath(nullptr, Tr::tr("Choose Public Key File"), dir, Tr::tr("Public Key Files (*.pub);;All Files (*)")); if (publicKeyFileName.isEmpty()) return nullptr; - return new PublicKeyDeploymentDialog(deviceConfig, publicKeyFileName, parent); + return new PublicKeyDeploymentDialog(device, publicKeyFileName, parent); } -PublicKeyDeploymentDialog::PublicKeyDeploymentDialog(const IDevice::ConstPtr &deviceConfig, - const FilePath &publicKeyFileName, QWidget *parent) +PublicKeyDeploymentDialog::PublicKeyDeploymentDialog( + const DeviceConstRef &device, + const FilePath &publicKeyFileName, + QWidget *parent) : QProgressDialog(parent), d(new PublicKeyDeploymentDialogPrivate) { setAutoReset(false); @@ -76,7 +78,7 @@ PublicKeyDeploymentDialog::PublicKeyDeploymentDialog(const IDevice::ConstPtr &de + QString::fromLocal8Bit(reader.data()) + "' >> .ssh/authorized_keys && chmod 0600 .ssh/authorized_keys"; - const SshParameters params = deviceConfig->sshParameters(); + const SshParameters params = device.sshParameters(); const QString hostKeyCheckingString = params.hostKeyCheckingMode == SshHostKeyCheckingStrict ? QLatin1String("yes") : QLatin1String("no"); const bool isWindows = HostOsInfo::isWindowsHost() diff --git a/src/plugins/remotelinux/publickeydeploymentdialog.h b/src/plugins/remotelinux/publickeydeploymentdialog.h index 8177e58935e..68b023e8133 100644 --- a/src/plugins/remotelinux/publickeydeploymentdialog.h +++ b/src/plugins/remotelinux/publickeydeploymentdialog.h @@ -18,11 +18,12 @@ class PublicKeyDeploymentDialog : public QProgressDialog Q_OBJECT public: // Asks for public key and returns null if the file dialog is canceled. - static PublicKeyDeploymentDialog *createDialog( - const ProjectExplorer::IDeviceConstPtr &deviceConfig, QWidget *parent = nullptr); + static PublicKeyDeploymentDialog *createDialog(const ProjectExplorer::DeviceConstRef &device, + QWidget *parent = nullptr); - PublicKeyDeploymentDialog(const ProjectExplorer::IDeviceConstPtr &deviceConfig, - const Utils::FilePath &publicKeyFileName, QWidget *parent = nullptr); + PublicKeyDeploymentDialog(const ProjectExplorer::DeviceConstRef &device, + const Utils::FilePath &publicKeyFileName, + QWidget *parent = nullptr); ~PublicKeyDeploymentDialog() override; diff --git a/src/plugins/remotelinux/sshdevicewizard.cpp b/src/plugins/remotelinux/sshdevicewizard.cpp index 1bea0cfa950..b1f41777fca 100644 --- a/src/plugins/remotelinux/sshdevicewizard.cpp +++ b/src/plugins/remotelinux/sshdevicewizard.cpp @@ -31,7 +31,7 @@ namespace RemoteLinux { class SetupPage : public QWizardPage { public: - explicit SetupPage(const ProjectExplorer::IDevicePtr &device) + explicit SetupPage(const DeviceRef &device) : m_device(device) { setTitle(Tr::tr("Connection")); @@ -65,11 +65,11 @@ public: private: void initializePage() final { - m_nameLineEdit->setText(m_device->displayName()); - m_hostNameLineEdit->setText(m_device->sshParameters().host()); + m_nameLineEdit->setText(m_device.displayName()); + m_hostNameLineEdit->setText(m_device.sshParameters().host()); m_sshPortSpinBox->setValue(22); m_sshPortSpinBox->setRange(1, 65535); - m_userNameLineEdit->setText(m_device->sshParameters().userName()); + m_userNameLineEdit->setText(m_device.sshParameters().userName()); } bool isComplete() const final { return !m_nameLineEdit->text().trimmed().isEmpty() @@ -77,12 +77,12 @@ private: && !m_userNameLineEdit->text().trimmed().isEmpty(); } bool validatePage() final { - m_device->setDisplayName(m_nameLineEdit->text().trimmed()); - SshParameters sshParams = m_device->sshParameters(); + m_device.setDisplayName(m_nameLineEdit->text().trimmed()); + SshParameters sshParams = m_device.sshParameters(); sshParams.setHost(m_hostNameLineEdit->text().trimmed()); sshParams.setUserName(m_userNameLineEdit->text().trimmed()); sshParams.setPort(m_sshPortSpinBox->value()); - m_device->setSshParameters(sshParams); + m_device.setSshParameters(sshParams); return true; } @@ -90,13 +90,13 @@ private: FancyLineEdit *m_hostNameLineEdit; QSpinBox *m_sshPortSpinBox; FancyLineEdit *m_userNameLineEdit; - IDevicePtr m_device; + DeviceRef m_device; }; class KeyDeploymentPage : public QWizardPage { public: - explicit KeyDeploymentPage(const ProjectExplorer::IDevicePtr &device) + explicit KeyDeploymentPage(const DeviceRef &device) : m_device(device) { setTitle(Tr::tr("Key Deployment")); @@ -150,7 +150,7 @@ public: private: void initializePage() final { - if (!m_device->sshParameters().privateKeyFile.isEmpty()) + if (!m_device.sshParameters().privateKeyFile.isEmpty()) m_keyFileChooser.setFilePath(m_keyFileChooser.filePath()); m_iconLabel.clear(); } @@ -160,10 +160,10 @@ private: } bool validatePage() final { if (!defaultKeys().contains(m_keyFileChooser.filePath())) { - SshParameters sshParams = m_device->sshParameters(); + SshParameters sshParams = m_device.sshParameters(); sshParams.authenticationType = SshParameters::AuthenticationTypeSpecificKey; sshParams.privateKeyFile = m_keyFileChooser.filePath(); - m_device->setSshParameters(sshParams); + m_device.setSshParameters(sshParams); } return true; } @@ -174,7 +174,7 @@ private: PathChooser m_keyFileChooser; QLabel m_iconLabel; - IDevicePtr m_device; + DeviceRef m_device; }; class FinalPage final : public QWizardPage @@ -193,7 +193,7 @@ public: } }; -SshDeviceWizard::SshDeviceWizard(const QString &title, const ProjectExplorer::IDevicePtr &device) +SshDeviceWizard::SshDeviceWizard(const QString &title, const DeviceRef &device) : Wizard(Core::ICore::dialogParent()) { setWindowTitle(title); diff --git a/src/plugins/remotelinux/sshdevicewizard.h b/src/plugins/remotelinux/sshdevicewizard.h index 3bd2af1ab22..319904b1234 100644 --- a/src/plugins/remotelinux/sshdevicewizard.h +++ b/src/plugins/remotelinux/sshdevicewizard.h @@ -5,15 +5,16 @@ #include "remotelinux_export.h" -#include #include +namespace ProjectExplorer { class DeviceRef; } + namespace RemoteLinux { class REMOTELINUX_EXPORT SshDeviceWizard : public Utils::Wizard { public: - SshDeviceWizard(const QString &title, const ProjectExplorer::IDevicePtr &device); + SshDeviceWizard(const QString &title, const ProjectExplorer::DeviceRef &device); }; } // namespace RemoteLinux