Skip to content

pr1#482

Merged
deepin-bot[bot] merged 2 commits intolinuxdeepin:masterfrom
itsXuSt:pr1
Dec 18, 2025
Merged

pr1#482
deepin-bot[bot] merged 2 commits intolinuxdeepin:masterfrom
itsXuSt:pr1

Conversation

@itsXuSt
Copy link
Contributor

@itsXuSt itsXuSt commented Dec 18, 2025

  • fix: 通过dconfig屏蔽进程暂停与恢复进程功能
  • chore: redirect crash log to cache path.

myk1343 and others added 2 commits December 18, 2025 09:55
通过dconfig屏蔽进程暂停与恢复进程功能

Bug: https://pms.uniontech.com/bug-view-272331.html
Log: 通过dconfig屏蔽进程暂停与恢复进程功能
(cherry picked from commit 2d70563)
as above.

Log: as above.
(cherry picked from commit 006c772)
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个 diff 进行详细审查:

  1. 配置文件修改 (org.deepin.system-monitor.json):
"displayMenuPauseAndRecovery": {
    "value": 0,
    "serial": 0,
    "flags": ["global"],
    "name": "Display pause recovery process",
    "name[zh_CN]": "显示暂停恢复进程",
    "description": "显示暂停恢复进程",
    "permissions": "readwrite",
    "visibility": "public"
}

优点:

  • 添加了完整的配置项定义
  • 包含了中英文支持
  • 权限设置合理

建议改进:

  • description[zh_CN] 字段缺失,应该添加中文描述
  • 可以考虑添加默认值说明
  1. 进程表视图修改 (process_table_view.cpp):
DConfig *config = DConfig::create("org.deepin.system-monitor", "org.deepin.system-monitor");
if(config) {
    QVariant varMenu = config->value("displayMenuPauseAndRecovery");
    if(varMenu.isValid() && varMenu.toInt() == 0) {
        pauseProcAction->setVisible(false);
        resumeProcAction->setVisible(false);
    }
}

优点:

  • 实现了配置驱动的菜单项显示控制
  • 添加了必要的空指针检查

建议改进:

  • DConfig 对象没有释放,存在内存泄漏风险
  • 配置值的类型检查不够严格,建议使用更安全的类型转换
  • 可以将配置读取逻辑封装成单独的函数
  • 建议添加配置变更的监听机制
  1. 堆栈跟踪修改 (stack_trace.h):
auto cachePath = QStandardPaths::writableLocation(QStandardPaths::CacheLocation);
if (!cachePath.endsWith("/")) cachePath.append("/");
std::string logN {cachePath.toStdString()};

优点:

  • 使用了标准的缓存目录,而不是硬编码的 /tmp
  • 添加了路径分隔符检查

建议改进:

  • 需要检查 QStandardPaths::writableLocation() 的返回值是否为空
  • toStdString() 可能导致编码问题,建议使用 toUtf8()
  • 可以考虑使用 QDir::toNativeSeparators() 确保路径分隔符正确

安全性建议:

  1. 配置文件:
  • 添加配置值的范围限制
  • 考虑添加配置值变更的审计日志
  1. 进程表视图:
  • 添加配置值的合法性检查
  • 实现配置变更的实时响应
  • 考虑添加配置加载失败的错误处理
  1. 堆栈跟踪:
  • 添加日志文件的权限检查
  • 考虑添加日志文件的大小限制
  • 实现日志轮转机制

代码质量建议:

  1. 添加更多的错误处理
  2. 增加代码注释
  3. 考虑单元测试覆盖
  4. 遵循统一的代码风格

性能建议:

  1. 配置读取可以考虑缓存机制
  2. 日志写入可以考虑异步处理

这些修改整体上是合理的,但在错误处理、资源管理和安全性方面还有改进空间。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: itsXuSt, lzwind

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@itsXuSt
Copy link
Contributor Author

itsXuSt commented Dec 18, 2025

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Dec 18, 2025

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 6ca2b8a into linuxdeepin:master Dec 18, 2025
7 checks passed
@itsXuSt itsXuSt deleted the pr1 branch December 18, 2025 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants