From a0d643dd89d7b4ed6227c1d148435727edace951 Mon Sep 17 00:00:00 2001 From: Nick Bolton Date: Mon, 8 Jun 2026 12:02:03 +0100 Subject: [PATCH] refactor: replace qFatal with qCritical --- src/lib/gui/ScreenSetupModel.cpp | 25 +++++++++------- src/lib/gui/ScreenSetupModel.h | 6 ++-- src/lib/gui/core/CoreProcess.cpp | 30 +++++++++++++------- src/lib/gui/dialogs/SettingsDialog.cpp | 3 +- src/lib/gui/validators/LineEditValidator.cpp | 7 +++-- 5 files changed, 44 insertions(+), 27 deletions(-) diff --git a/src/lib/gui/ScreenSetupModel.cpp b/src/lib/gui/ScreenSetupModel.cpp index cc1b9e7aa..31f8c3378 100644 --- a/src/lib/gui/ScreenSetupModel.cpp +++ b/src/lib/gui/ScreenSetupModel.cpp @@ -8,12 +8,13 @@ #include "ScreenSetupModel.h" +#include "common/Constants.h" +#include "gui/config/Screen.h" + #include #include #include -#include "gui/config/Screen.h" - const QString ScreenSetupModel::m_MimeType = "application/x-deskflow-screen"; ScreenSetupModel::ScreenSetupModel(ScreenList &screens, int numColumns, int numRows) @@ -22,17 +23,19 @@ ScreenSetupModel::ScreenSetupModel(ScreenList &screens, int numColumns, int numR m_NumColumns(numColumns), m_NumRows(numRows) { - - // bound rows and columns to prevent multiply overflow. - // this is unlikely to happen, as the grid size is only 3x9. - if (m_NumColumns > 100 || m_NumRows > 100) { - qFatal("grid size out of bounds: %d columns x %d rows", m_NumColumns, m_NumRows); - return; + // bound the grid so that multiplying columns by rows cannot overflow. + if (m_NumColumns < 1 || m_NumColumns > kMaxGridSize || m_NumRows < 1 || m_NumRows > kMaxGridSize) { + qCritical("grid size out of bounds: %d columns x %d rows", m_NumColumns, m_NumRows); + m_NumColumns = kServerGridWidth; + m_NumRows = kServerGridHeight; } - const long span = static_cast(m_NumColumns) * m_NumRows; - if (span > screens.size()) { - qFatal("scrren list (%lld) too small for %d columns x %d rows", screens.size(), m_NumColumns, m_NumRows); + const int span = m_NumColumns * m_NumRows; + if (span > m_Screens.size()) { + qCritical( + "screen list too small for grid, screens: %lld, cells: %d", static_cast(m_Screens.size()), span + ); + m_Screens.resize(span); } } diff --git a/src/lib/gui/ScreenSetupModel.h b/src/lib/gui/ScreenSetupModel.h index f4f8ae9b4..e9a5e2fe0 100644 --- a/src/lib/gui/ScreenSetupModel.h +++ b/src/lib/gui/ScreenSetupModel.h @@ -79,9 +79,11 @@ protected: void addScreen(const Screen &newScreen); private: + static constexpr int kMaxGridSize = 100; + ScreenList &m_Screens; - const int m_NumColumns; - const int m_NumRows; + int m_NumColumns; + int m_NumRows; static const QString m_MimeType; }; diff --git a/src/lib/gui/core/CoreProcess.cpp b/src/lib/gui/core/CoreProcess.cpp index 652b7e8f3..5c8985cfe 100644 --- a/src/lib/gui/core/CoreProcess.cpp +++ b/src/lib/gui/core/CoreProcess.cpp @@ -105,8 +105,7 @@ CoreProcess::CoreProcess(const ServerConfig &serverConfig) { m_appPath = QStringLiteral("%1/%2").arg(QCoreApplication::applicationDirPath(), kCoreBinName); if (!QFile::exists(m_appPath)) { - qFatal("core server binary does not exist"); - return; + qCritical("core server binary does not exist"); } connect(m_daemonIpcClient, &ipc::DaemonIpcClient::connected, this, &CoreProcess::daemonIpcClientConnected); @@ -216,7 +215,8 @@ void CoreProcess::startForegroundProcess(const QStringList &args) using enum ProcessState; if (m_processState != Starting) { - qFatal("core process must be in starting state"); + qCritical("not starting core desktop process, unexpected process state"); + return; } // only make quoted args for printing the command for convenience; so that the @@ -245,7 +245,8 @@ void CoreProcess::startForegroundProcess(const QStringList &args) void CoreProcess::startProcessFromDaemon() { if (m_processState != ProcessState::Starting) { - qFatal("core process must be in starting state"); + qCritical("not starting core process from daemon, unexpected process state"); + return; } const auto configFile = Settings::settingsFile(); @@ -271,11 +272,13 @@ void CoreProcess::startProcessFromDaemon() void CoreProcess::stopForegroundProcess() const { if (m_processState != ProcessState::Stopping) { - qFatal("core process must be in stopping state"); + qCritical("not stopping core desktop process, unexpected process state"); + return; } if (!m_process) { - qFatal("process not set, cannot stop"); + qCritical("not stopping core desktop process, no process to stop"); + return; } qInfo("stopping core desktop process"); @@ -291,7 +294,8 @@ void CoreProcess::stopForegroundProcess() const void CoreProcess::stopProcessFromDaemon() { if (m_processState != ProcessState::Stopping) { - qFatal("core process must be in stopping state"); + qCritical("not stopping core process from daemon, unexpected process state"); + return; } auto sendStop = [this] { @@ -348,12 +352,12 @@ void CoreProcess::handleLogLines(const QString &text) void CoreProcess::start(std::optional processModeOption) { if (m_processState == ProcessState::Started) { - qCritical("core process already started"); + qCritical("not starting core process, already started"); return; } if (m_mode == Settings::CoreMode::None) { - qFatal("set core mode before starting"); + qCritical("not starting core process, no core mode set"); return; } @@ -391,7 +395,10 @@ void CoreProcess::start(std::optional processModeOption) if (m_mode == Settings::CoreMode::Server) { const auto [hasNeededPermissions, configFilename] = persistServerConfig(); if (configFilename.isEmpty()) { - qFatal("config file name empty for server args"); + qCritical("not starting core process, no server config file"); + setProcessState(ProcessState::Stopped); + setConnectionState(ConnectionState::Disconnected); + Q_EMIT error(Error::StartFailed); return; } if (!hasNeededPermissions) { @@ -654,7 +661,8 @@ void CoreProcess::clearSettings() } if (processMode != ProcessMode::Service) { - qFatal("invalid process mode"); + qCritical("not clearing core settings, unexpected process mode"); + return; } qInfo("clearing core settings through daemon"); diff --git a/src/lib/gui/dialogs/SettingsDialog.cpp b/src/lib/gui/dialogs/SettingsDialog.cpp index 684fb8c6f..c98391a57 100644 --- a/src/lib/gui/dialogs/SettingsDialog.cpp +++ b/src/lib/gui/dialogs/SettingsDialog.cpp @@ -351,7 +351,8 @@ bool SettingsDialog::isClientMode() const void SettingsDialog::updateKeyLengthOnFile(const QString &path) { if (!QFile(path).exists()) { - qFatal("tls certificate file not found: %s", qUtf8Printable(path)); + qCritical("tls certificate file not found: %s", qUtf8Printable(path)); + return; } auto length = TlsUtility::getCertKeyLength(path); diff --git a/src/lib/gui/validators/LineEditValidator.cpp b/src/lib/gui/validators/LineEditValidator.cpp index fc74b8bd2..f405e7ab8 100644 --- a/src/lib/gui/validators/LineEditValidator.cpp +++ b/src/lib/gui/validators/LineEditValidator.cpp @@ -21,7 +21,7 @@ LineEditValidator::LineEditValidator(QLineEdit *lineEdit, ValidationError *error { if (!m_pLineEdit) { - qFatal("validator line edit not set"); + qCritical("validator line edit not set"); } } @@ -32,7 +32,10 @@ void LineEditValidator::addValidator(std::unique_ptr validator QValidator::State LineEditValidator::validate(QString &input, int &) const { - assert(m_pLineEdit); + if (!m_pLineEdit) { + qCritical("cannot validate input, no line edit set"); + return Intermediate; + } QString errorMessage; for (const auto &validator : m_Validators) {