fix: Avoid hardcode binary path#197
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Mar 17, 2026
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplaces hardcoded absolute paths to binaries with portable lookups based on program name and relaxes a front-end process check to no longer depend on a fixed installation path. Flow diagram for updated front-end quit check in checkFrontEndQuitflowchart TD
A[Start checkFrontEndQuit frontEndPid] --> B[Build frontEndExe as /proc/frontEndPid/exe]
B --> C[Create QFileInfo info using frontEndExe]
C --> D[Resolve targetPath = info.symLinkTarget]
D --> E[Create QFileInfo targetInfo using targetPath]
E --> F[Extract exeName = targetInfo.fileName]
F --> G{exeName == deepin-diskmanager?}
G -- Yes --> H[Do nothing, keep service running]
G -- No --> I[Log warning Front-end process has quit]
I --> J[Exit QCoreApplication with code 0]
H --> K[End]
J --> K[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Relying on bare executable names for
dbus-sendanddeepin-diskmanager-authenticateProxyshifts behavior to whatever is at the front of$PATH; if the intent is portability rather than configurability, consider resolving the full path once (e.g., via a config or helper) to avoid unexpected PATH hijacking or environment-dependent behavior in system contexts. - The
dbus-sendcall sequence is duplicated in multiple places; consider extracting a small helper to construct and invoke this quit call so that arguments and behavior stay consistent if they need adjustment later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Relying on bare executable names for `dbus-send` and `deepin-diskmanager-authenticateProxy` shifts behavior to whatever is at the front of `$PATH`; if the intent is portability rather than configurability, consider resolving the full path once (e.g., via a config or helper) to avoid unexpected PATH hijacking or environment-dependent behavior in system contexts.
- The `dbus-send` call sequence is duplicated in multiple places; consider extracting a small helper to construct and invoke this quit call so that arguments and behavior stay consistent if they need adjustment later.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Note
详情{
"application/main.cpp": [
{
"line": " const QString acknowledgementLink = \"https://www.deepin.org/acknowledgments/deepin_reader\";",
"line_number": 124,
"rule": "S35",
"reason": "Url link | c08ef69076"
}
]
} |
lzwind
previously approved these changes
Mar 17, 2026
Avoid hardcoding executable file paths to improve portability. Bug: linuxdeepin/developer-center#3374
deepin pr auto review代码审查报告我已对提供的 git diff 进行了仔细审查，以下是关于语法逻辑、代码质量、代码性能和代码安全的改进建议： 1. 版权年份更新-// SPDX-FileCopyrightText: 2022 UnionTech Software Technology Co., Ltd.
+// SPDX-FileCopyrightText: 2022 - 2026 UnionTech Software Technology Co., Ltd.评价：这是一个简单的版权年份更新，将版权范围从2022年延长到2026年。这种做法是合理的，可以保持版权声明的有效性。 2. 移除绝对路径的改进-proc.startDetached("/usr/bin/dbus-send", argList);
+proc.startDetached("dbus-send", argList);评价：这是一个很好的改进，使用相对路径而非绝对路径有以下几个优点：
3. 进程路径比较的改进-if (info.symLinkTarget() != "/usr/bin/deepin-diskmanager") {
+if (QFileInfo(info.symLinkTarget()).fileName() != "deepin-diskmanager") {评价：这是一个很好的改进，原因如下：
4. 潜在问题与改进建议4.1 安全性问题在 while (1) {
// 循环查询后台服务是否已经启动
}问题：
建议： int maxAttempts = 30; // 最多尝试30次
int attemptCount = 0;
const int sleepInterval = 200; // 每次间隔200ms
while (attemptCount < maxAttempts) {
// 检查服务是否已启动
if (isServiceStarted()) {
break;
}
QThread::msleep(sleepInterval);
attemptCount++;
}
if (attemptCount >= maxAttempts) {
qWarning() << "Service failed to start within expected time";
// 处理超时情况
}4.2 代码质量问题在多个地方重复了相同的代码： QString args = QString("--system --type=method_call --dest=com.deepin.diskmanager /com/deepin/diskmanager com.deepin.diskmanager.Quit");
QStringList argList = args.split(" ");
proc.startDetached("dbus-send", argList);建议：将这段代码提取为一个函数： void sendQuitSignal()
{
QProcess proc;
QStringList argList;
argList << "--system"
<< "--type=method_call"
<< "--dest=com.deepin.diskmanager"
<< "/com/deepin/diskmanager"
<< "com.deepin.diskmanager.Quit";
proc.startDetached("dbus-send", argList);
}4.3 性能问题在 if (QFileInfo(info.symLinkTarget()).fileName() != "deepin-diskmanager") {建议：如果这段代码在频繁调用的循环中，可以考虑缓存文件名或使用更高效的检查方法。 4.4 错误处理在多处使用 proc.startDetached("dbus-send", argList);建议：添加错误处理： if (!proc.startDetached("dbus-send", argList)) {
qWarning() << "Failed to start dbus-send";
}5. 总体评价这次更改主要是改进了代码的可移植性和灵活性，移除了硬编码的绝对路径，这是一个好的实践。但代码中仍存在一些可以改进的地方，特别是在错误处理、代码重复和潜在的无限循环问题上。 建议在后续版本中考虑上述改进建议，以提高代码的健壮性和可维护性。 |
|
Note
详情{
"application/main.cpp": [
{
"line": " const QString acknowledgementLink = \"https://www.deepin.org/acknowledgments/deepin_reader\";",
"line_number": 124,
"rule": "S35",
"reason": "Url link | c08ef69076"
}
]
} |
lzwind
approved these changes
Mar 17, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, wangrong1069 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Avoid hardcoding executable file paths to improve portability.
Bug: linuxdeepin/developer-center#3374
Summary by Sourcery
Make disk manager processes rely on executable names rather than hardcoded absolute paths to improve portability.
Bug Fixes: