Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/common/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
configure_file("Constants.h.in" "Constants.h" IMMEDIATE @ONLY)
set(PUBLIC_HEADERS
${CMAKE_SOURCE_DIR}/src/common/ConfigReader.h
${CMAKE_SOURCE_DIR}/src/common/Configuration.h
${CMAKE_SOURCE_DIR}/src/common/Config.h
${CMAKE_SOURCE_DIR}/src/common/MessageHandler.h
${CMAKE_SOURCE_DIR}/src/common/Messages.h
${CMAKE_SOURCE_DIR}/src/common/Session.h
Expand All @@ -13,8 +12,6 @@ set(PUBLIC_HEADERS
)

set(SRCS
${CMAKE_SOURCE_DIR}/src/common/ConfigReader.cpp
${CMAKE_SOURCE_DIR}/src/common/Configuration.cpp
${CMAKE_SOURCE_DIR}/src/common/Session.cpp
${CMAKE_SOURCE_DIR}/src/common/SignalHandler.cpp
${CMAKE_SOURCE_DIR}/src/common/SocketWriter.cpp
Expand Down
283 changes: 283 additions & 0 deletions src/common/Config.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,283 @@
// Copyright (C) 2026 UnionTech Software Technology Co., Ltd.
// SPDX-License-Identifier: GPL-2.0-or-later

#pragma once

#include <QFile>

Check warning on line 6 in src/common/Config.h

View workflow job for this annotation

GitHub Actions / cppcheck

Include file: <QFile> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QMap>

Check warning on line 7 in src/common/Config.h

View workflow job for this annotation

GitHub Actions / cppcheck

Include file: <QMap> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QObject>

Check warning on line 8 in src/common/Config.h

View workflow job for this annotation

GitHub Actions / cppcheck

Include file: <QObject> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QPair>

Check warning on line 9 in src/common/Config.h

View workflow job for this annotation

GitHub Actions / cppcheck

Include file: <QPair> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QVariant>

Check warning on line 10 in src/common/Config.h

View workflow job for this annotation

GitHub Actions / cppcheck

Include file: <QVariant> not found. Please note: Cppcheck does not need standard library headers to get proper results.

#include "Constants.h"

Check warning on line 12 in src/common/Config.h

View workflow job for this annotation

GitHub Actions / cppcheck

Include file: "Constants.h" not found.

namespace DDM {
class Config : public QObject {
Q_OBJECT
public:
/**
* Constructor.
*
* @param configPath Path to the configuration file.
* @param defaults Default configuration values and descriptions.
* @param parent Parent QObject.
*/
Config(const QString &configPath,

Check warning on line 25 in src/common/Config.h

View workflow job for this annotation

GitHub Actions / cppcheck

Class 'Config' has a constructor with 1 argument that is not explicit. Such, so called "Converting constructors", should in general be explicit for type safety reasons as that prevents unintended implicit conversions.
// section entry default description
QMap<QString, QMap<QString, QPair<QVariant, QString>>> defaults = {},
QObject *parent = nullptr)
: QObject(parent)
, m_configPath(configPath)
, m_defaults(defaults) { }

/**
* Get a configuration entry value.
* If the entry is not found in the loaded configuration, the default value is returned.
* If no default value is set, an exception is thrown.
*
* @param section The configuration section name.
* @param entry The configuration entry name.
* @return The value of the configuration entry.
*/
template<typename T>
inline T get(const QString &section, const QString &entry) const {
if (m_data.contains(section) && m_data[section].contains(entry))
return m_data[section][entry].value<T>();
else if (m_defaults.contains(section) && m_defaults[section].contains(entry))
return m_defaults[section][entry].first.value<T>();
else
throw std::runtime_error(QStringLiteral("Config entry '%1/%2' not found")
.arg(section, entry)
.toStdString());
}
Comment on lines +33 to +52
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The get() method throws std::runtime_error when a configuration entry is not found, but this exception type and the throwing behavior are not documented in the class documentation or the method's docstring. Callers may not expect exceptions and could fail to handle them properly, potentially causing crashes.

Add exception specifications to the documentation, or consider returning an optional/default value instead of throwing to make the API more robust and Qt-like. If exceptions are the intended design, document them clearly in both the class-level and method-level documentation.

get() 方法在找不到配置条目时抛出 std::runtime_error,但此异常类型和抛出行为未在类文档或方法的文档字符串中记录。调用者可能不期望异常,可能无法正确处理它们,可能导致崩溃。

建议在文档中添加异常规范,或考虑返回可选/默认值而不是抛出异常,以使 API 更健壮和更像 Qt。如果异常是预期的设计,请在类级别和方法级别的文档中明确记录它们。

Copilot uses AI. Check for mistakes.

/**
* Get a configuration entry value inside the default section.
* If the entry is not found in the loaded configuration, the default value is returned.
* If no default value is set, an exception is thrown.
*
* @param entry The configuration entry name.
* @return The value of the configuration entry.
*/
template<typename T>
inline T get(const QString &entry) const {
return get<T>({}, entry);
}
Comment on lines +62 to +65
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using an empty string {} to represent the default section is inconsistent with the save() and defaultConfig() methods which explicitly check for non-empty section names (lines 167, 192). This works but makes the code less clear. Consider using a named constant like DEFAULT_SECTION or "General" to improve code readability and maintainability.

使用空字符串 {} 来表示默认部分与 save() 和 defaultConfig() 方法不一致,这些方法明确检查非空部分名称(第167、192行)。这有效但使代码不太清晰。建议使用命名常量,如 DEFAULT_SECTION 或 "General",以提高代码可读性和可维护性。

Copilot uses AI. Check for mistakes.

/**
* Set a configuration entry value.
*
* @param section The configuration section name.
* @param entry The configuration entry name.
* @param value The value to set.
*/
template<typename T>
inline void set(const QString &section, const QString &entry, const T &value) {

Check warning on line 75 in src/common/Config.h

View workflow job for this annotation

GitHub Actions / cppcheck

The function 'set' is never used.
m_data[section][entry] = QVariant::fromValue(value);
}

/**
* Set a configuration entry value inside the default section.
*
* @param entry The configuration entry name.
* @param value The value to set.
*/
template<typename T>
inline void set(const QString &entry, const T &value) {
m_data[{}][entry] = QVariant::fromValue(value);
}

/**
* Reset a configuration entry to its default value.
*
* @param section The configuration section name.
* @param entry The configuration entry name.
* @return True if the entry was reset, false if it was not found or already default.
*/
inline bool setDefault(const QString &section, const QString &entry) {
if (m_data.contains(section) && m_data[section].contains(entry)) {
m_data[section].remove(entry);
return true;
}
return false;
}

/**
* Reset a configuration entry to its default value inside the default section.
*
* @param entry The configuration entry name.
* @return True if the entry was reset, false if it was not found or already default.
*/
inline bool setDefault(const QString &entry) {
return setDefault({}, entry);
}

/**
* Load the configuration from the file.
*
* @return True if the configuration was loaded successfully, false otherwise.
*/
bool load() {

Check warning on line 120 in src/common/Config.h

View workflow job for this annotation

GitHub Actions / cppcheck

The function 'load' is never used.
QFile file(m_configPath);
if (!file.open(QIODevice::ReadOnly)) {
return false;
}
QString currentSection{};
while (!file.atEnd()) {
QString line = QString::fromLocal8Bit(file.readLine()).trimmed();
if (line.isEmpty() || line.startsWith('#'))
continue;
if (line.startsWith('[') && line.endsWith(']')) {
currentSection = line.mid(1, line.length() - 2).trimmed();
} else {
int separatorPosition = line.indexOf('=');
if (separatorPosition >= 0) {
if (!m_data.contains(currentSection))
m_data[currentSection] = {};
QString name = line.left(separatorPosition).trimmed();
QString value = line.mid(separatorPosition + 1).trimmed();
if (value.isEmpty()) {
m_data[currentSection][name] = QVariant();
} else if (value.indexOf(',') >= 0) {
QStringList list = value.split(',', Qt::SkipEmptyParts);
for (QString &item : list)
item = item.trimmed();
m_data[currentSection][name] = QVariant(list);
} else
m_data[currentSection][name] = QVariant(value);
Comment on lines +141 to +147
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The load() method treats any value containing a comma as a QStringList (line 141-145), which may incorrectly parse string values that legitimately contain commas but are not intended to be lists. For example, a description like "Hello, world" would be parsed as a list with two elements.

Consider checking the default value's type (if available) to determine whether a value should be parsed as a list, rather than relying solely on the presence of a comma. This would make the parsing more robust and align with the actual data type expectations.

load() 方法将任何包含逗号的值视为 QStringList(第141-145行),这可能会错误解析合法包含逗号但不打算作为列表的字符串值。例如,像 "Hello, world" 这样的描述将被解析为包含两个元素的列表。

建议检查默认值的类型(如果可用)来确定值是否应解析为列表,而不是仅依赖逗号的存在。这将使解析更加健壮,并与实际的数据类型期望保持一致。

Suggested change
} else if (value.indexOf(',') >= 0) {
QStringList list = value.split(',', Qt::SkipEmptyParts);
for (QString &item : list)
item = item.trimmed();
m_data[currentSection][name] = QVariant(list);
} else
m_data[currentSection][name] = QVariant(value);
} else {
// Decide whether to treat the value as a list based on the default type.
bool treatAsList = false;
auto sectionDefaultsIt = m_defaults.constFind(currentSection);
if (sectionDefaultsIt != m_defaults.cend()) {
const auto &entryDefaults = sectionDefaultsIt.value();
auto entryIt = entryDefaults.constFind(name);
if (entryIt != entryDefaults.cend() &&
entryIt->first.userType() == QMetaType::QStringList) {
treatAsList = true;
}
}
if (treatAsList) {
QStringList list = value.split(',', Qt::SkipEmptyParts);
for (QString &item : list)
item = item.trimmed();
m_data[currentSection][name] = QVariant(list);
} else {
m_data[currentSection][name] = QVariant(value);
}
}

Copilot uses AI. Check for mistakes.
}
}
}
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old ConfigReader implementation had backward compatibility for renamed sections: "XDisplay" was automatically mapped to "X11", and "WaylandDisplay" to "Wayland" during loading (see deleted ConfigReader.cpp lines 210-213). The new Config class doesn't include this backward compatibility, which means existing configuration files using the old section names will not work correctly.

Consider adding backward compatibility mappings in the load() method to handle legacy section names, or document this as a breaking change requiring manual configuration migration.

旧的 ConfigReader 实现对重命名的部分具有向后兼容性:"XDisplay" 自动映射到 "X11","WaylandDisplay" 映射到 "Wayland"(参见已删除的 ConfigReader.cpp 第210-213行)。新的 Config 类不包含此向后兼容性,这意味着使用旧部分名称的现有配置文件将无法正常工作。

建议在 load() 方法中添加向后兼容性映射以处理旧的部分名称,或将此记录为需要手动配置迁移的破坏性更改。

Suggested change
}
}
// Backward compatibility for legacy section names.
// Map "XDisplay" -> "X11".
if (m_data.contains(QStringLiteral("XDisplay"))) {
auto legacy = m_data.take(QStringLiteral("XDisplay"));
auto &target = m_data[QStringLiteral("X11")];
for (auto it = legacy.cbegin(); it != legacy.cend(); ++it) {
if (!target.contains(it.key())) {
target.insert(it.key(), it.value());
}
}
}
// Map "WaylandDisplay" -> "Wayland".
if (m_data.contains(QStringLiteral("WaylandDisplay"))) {
auto legacy = m_data.take(QStringLiteral("WaylandDisplay"));
auto &target = m_data[QStringLiteral("Wayland")];
for (auto it = legacy.cbegin(); it != legacy.cend(); ++it) {
if (!target.contains(it.key())) {
target.insert(it.key(), it.value());
}
}
}

Copilot uses AI. Check for mistakes.
return true;
}
Comment on lines +120 to +152
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The load() method stores all configuration values as QString (line 147), including boolean values. However, when calling get(), the code attempts to convert QString to bool using QVariant::value(). QVariant's QString to bool conversion doesn't work as expected - it only returns true if the QString can be converted to a number and that number is non-zero, not based on "true"/"false" string values. This will cause boolean configuration values like "RememberLastUser" to always evaluate to false when loaded from a file.

Consider implementing proper type detection and conversion in the load() method. For example, check if a default value exists for the entry and use its type to determine how to parse the string value. Alternatively, implement special parsing for known boolean string values like "true", "false", "yes", "no", "1", "0" (case-insensitive).

boolean 配置值加载存在bug。load() 方法将所有配置值存储为 QString(第147行),包括布尔值。但调用 get() 时,代码尝试使用 QVariant::value() 将 QString 转换为 bool。QVariant 的 QString 到 bool 的转换不是基于 "true"/"false" 字符串,而是检查字符串是否可以转换为非零数字。这将导致从文件加载的布尔配置值(如 "RememberLastUser")总是评估为 false。

建议在 load() 方法中实现正确的类型检测和转换。例如,检查该条目是否存在默认值,并使用其类型确定如何解析字符串值。或者,为已知的布尔字符串值(如 "true"、"false"、"yes"、"no"、"1"、"0",不区分大小写)实现特殊解析。

Copilot uses AI. Check for mistakes.
Comment on lines +120 to +152
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new Config class only loads from a single configuration file (m_configPath), while the old ConfigReader implementation supported loading from multiple configuration directories in order of priority: system config directory (/usr/lib/ddm/ddm.conf.d/), user config directory (/etc/ddm.conf.d/), and the main config file (/etc/ddm.conf). This is a breaking change that removes the ability to use drop-in configuration files, which is a common pattern in system configuration.

Consider restoring support for configuration directories to maintain backward compatibility and allow modular configuration. The old implementation loaded files from configDir and sysConfigDir in alphabetical order before loading the main config file.

新的 Config 类仅从单个配置文件(m_configPath)加载,而旧的 ConfigReader 实现支持按优先级顺序从多个配置目录加载:系统配置目录(/usr/lib/ddm/ddm.conf.d/)、用户配置目录(/etc/ddm.conf.d/)和主配置文件(/etc/ddm.conf)。这是一个破坏性更改,删除了使用插入式配置文件的能力,这是系统配置中的常见模式。

建议恢复对配置目录的支持以保持向后兼容性并允许模块化配置。旧实现在加载主配置文件之前按字母顺序从 configDir 和 sysConfigDir 加载文件。

Copilot uses AI. Check for mistakes.

/**
* Save the configuration to the file.
*
* @return True if the configuration was saved successfully, false otherwise.
*/
bool save() const {

Check warning on line 159 in src/common/Config.h

View workflow job for this annotation

GitHub Actions / cppcheck

The function 'save' is never used.
QFile file(m_configPath);
if (!file.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
return false;
}
QTextStream out(&file);
for (auto section = m_data.cbegin(); section != m_data.cend(); ++section) {
const QString &sectionName = section.key();
if (!sectionName.isEmpty())
out << "[" << sectionName << "]\n";
const auto &entries = section.value();
for (auto entry = entries.constBegin(); entry != entries.constEnd(); ++entry) {
if (entry.value().userType() == QMetaType::QStringList)
out << entry.key() << "=" << entry.value().toStringList().join(", ") << "\n";
else
out << entry.key() << "=" << entry.value().toString() << "\n";
}
out << "\n";
}
return true;
}

/**
* Serialize the default configuration to the format of the
* configuration file, as a string.
*
* @return The default configuration as a string.
*/
QString defaultConfig() const {
QString str;
QTextStream out(&str);
for (auto section = m_defaults.cbegin(); section != m_defaults.cend(); ++section) {
const QString &sectionName = section.key();
if (!sectionName.isEmpty())
out << "[" << sectionName << "]\n\n";
const auto &entries = section.value();
for (auto entry = entries.constBegin(); entry != entries.constEnd(); ++entry) {
const QString &description = entry.value().second;
if (!description.isEmpty())
for (const QString &line : description.split('\n'))
out << "# " << line << "\n";
if (entry.value().first.userType() == QMetaType::QStringList)
out << entry.key() << "=" << entry.value().first.toStringList().join(", ") << "\n\n";
else
out << entry.key() << "=" << entry.value().first.toString() << "\n\n";
}
}
return str;
}

/**
* Wipe all loaded configuration data, reset all entries to default.
*/
inline void wipe() {
m_data.clear();
}

private:
/** Path to the configuration file. */
QString m_configPath{};
// section entry value
QMap<QString, QMap<QString, QVariant>> m_data;
// section entry default description
QMap<QString, QMap<QString, QPair<QVariant, QString>>> m_defaults;
};

/** Main configuration instance. */
inline Config mainConfig(
CONFIG_FILE,
{ { {},
{ { "HaltCommand", { HALT_COMMAND, "Halt command" } },
{ "RebootCommand", { REBOOT_COMMAND, "Reboot command" } },
{ "Namespaces",
{ QStringList(),
"List of Linux namespaces for user session to enter" } } } },
{ "Theme",
{ { "CursorTheme", { QString(), "Cursor theme used in the greeter" } },
{ "CursorSize", { QString(), "Cursor size used in the greeter" } } } },
{ "X11",
{ { "ServerPath", { "/usr/bin/X", "Path to X server binary" } },
{ "ServerArguments",
{ "-nolisten tcp", "Arguments passed to the X server invocation" } },
{ "SessionDir",
{ QStringList{ "/usr/local/share/xsessions", "/usr/share/xsessions" },
"Comma-separated list of directories containing available X sessions" } },
{ "SessionCommand",
{ SESSION_COMMAND,
"Path to a script to execute when starting the desktop session" } },
{ "SessionLogFile",
{ ".local/share/ddm/xorg-session.log", "Path to the user session log file" } },
{ "DisplayCommand",
{ QStringLiteral(DATA_INSTALL_DIR "/scripts/Xsetup"),
"Path to a script to execute when starting the display server" } },
{ "DisplayStopCommand",
{ QStringLiteral(DATA_INSTALL_DIR "/scripts/Xstop"),
"Path to a script to execute when stopping the display server" } } } },
{ "Wayland",
{ { "SessionDir",
{ QStringList{ "/usr/local/share/wayland-sessions", "/usr/share/wayland-sessions" },
"Comma-separated list of directories containing available Wayland sessions" } },
{ "SessionCommand",
{ WAYLAND_SESSION_COMMAND,
"Path to a script to execute when starting the desktop session" } },
{ "SessionLogFile",
{ ".local/share/ddm/wayland-session.log",
"Path to the user session log file" } } } },
{ "Single",
{ { "SessionCommand",
{ WAYLAND_SESSION_COMMAND,
"Path to a script to execute when starting the desktop session" } } } },
{ "Users",
{ { "DefaultPath",
{ "/usr/local/bin:/usr/bin:/bin", "Default $PATH for logged in users" } },
{ "RememberLastUser", { true, "Remember the last successfully logged in user" } },
{ "RememberLastSession",
{ true, "Remember the session of the last successfully logged in user" } } } } });

/** State configuration instance. */
inline Config stateConfig(
QStringLiteral(STATE_DIR "/state.conf"),
{ { "Last",
{ { "User", { QString(), "The last successfully logged in user" } },
{ "Session",
{ QString(), "The session of the last successfully logged in user" } } } } });
} // namespace DDM
Comment on lines +276 to +283
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline stateConfig instance is never loaded from the configuration file. While stateConfig.save() is called when user state changes (lines 268, 392 in Display.cpp), there's no corresponding stateConfig.load() call anywhere in the codebase. This means the last user and session state are never restored across daemon restarts, breaking the "remember last user/session" functionality.

Add a stateConfig.load() call during application initialization, likely in DaemonApp constructor or before the first use in Display.cpp.

stateConfig 实例从未从配置文件加载。虽然在用户状态更改时调用了 stateConfig.save()(Display.cpp 的第268、392行),但代码库中没有相应的 stateConfig.load() 调用。这意味着最后一个用户和会话状态在守护进程重启后不会恢复,破坏了"记住上次用户/会话"功能。

建议在应用程序初始化期间添加 stateConfig.load() 调用,可能在 DaemonApp 构造函数中或在 Display.cpp 中首次使用之前。

Copilot uses AI. Check for mistakes.
Comment on lines +277 to +283
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stateConfig path has been changed from a dynamic computation to a static constant. The old implementation (in deleted Configuration.h) attempted to use the home directory of the "ddm" user if it exists, falling back to STATE_DIR otherwise. The new implementation uses STATE_DIR directly, which may break existing deployments where the state is stored in the ddm user's home directory.

Consider restoring the dynamic path computation for backward compatibility, or document this as a breaking change and provide a migration path for existing installations.

stateConfig 路径已从动态计算更改为静态常量。旧实现(在已删除的 Configuration.h 中)尝试使用 "ddm" 用户的主目录(如果存在),否则回退到 STATE_DIR。新实现直接使用 STATE_DIR,这可能会破坏将状态存储在 ddm 用户主目录中的现有部署。

建议恢复动态路径计算以保持向后兼容性,或将此记录为破坏性更改并为现有安装提供迁移路径。

Copilot uses AI. Check for mistakes.
Comment on lines +225 to +283
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline global Config instances (mainConfig and stateConfig) are not thread-safe. These instances may be accessed from multiple threads (e.g., different Display instances, signal handlers, etc.), but the Config class has no synchronization mechanisms. Operations like load(), save(), get(), and set() can race with each other, potentially causing data corruption or crashes.

Since Config inherits from QObject, consider using QMutex to protect all member data access, or clearly document that these instances should only be accessed from the main thread.

内联全局 Config 实例(mainConfig 和 stateConfig)不是线程安全的。这些实例可能从多个线程访问(例如,不同的 Display 实例、信号处理程序等),但 Config 类没有同步机制。load()、save()、get() 和 set() 等操作可能相互竞争,可能导致数据损坏或崩溃。

由于 Config 继承自 QObject,建议使用 QMutex 保护所有成员数据访问,或明确记录这些实例只应从主线程访问。

Copilot uses AI. Check for mistakes.
Loading