Skip to content

fix: cannot build on Qt 6.9.1#279

Merged
BLumia merged 1 commit intolinuxdeepin:masterfrom
justforlxz:master
Aug 27, 2025
Merged

fix: cannot build on Qt 6.9.1#279
BLumia merged 1 commit intolinuxdeepin:masterfrom
justforlxz:master

Conversation

@justforlxz
Copy link
Member

missing some headers

Log:

missing some headers

Log:
@justforlxz justforlxz requested a review from BLumia August 27, 2025 03:16
deepin-ci-robot added a commit to linuxdeepin/qt6integration that referenced this pull request Aug 27, 2025
Synchronize source files from linuxdeepin/qt5integration.

Source-pull-request: linuxdeepin/qt5integration#279
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

我来审查这段代码变更。

代码审查意见

1. 新增头文件分析

qdeepinfiledialoghelper.cpp 中新增了 unistd.h

#include <unistd.h>

优点:

  • 这是一个标准的 POSIX 头文件,提供了 Unix 系统调用的接口
  • 在需要访问 POSIX 函数时是必要的

潜在问题:

  • 如果代码中实际并未使用 unistd.h 中的任何功能,那么这个包含是不必要的
  • 如果代码确实使用了 unistd.h 中的功能,应该添加注释说明具体用途

改进建议:

  • 检查代码中是否实际使用了 unistd.h 中的功能
  • 如果使用了,添加注释说明具体用途,例如:
// 用于使用 access() 函数检查文件可访问性
#include <unistd.h>

qdeepintheme.cpp 中新增了 qfactoryloader_p.h

#include <private/qfactoryloader_p.h>

优点:

  • 这是 Qt 内部 API,用于工厂加载器功能
  • 适用于需要动态加载插件或组件的场景

潜在问题:

  • 使用了 private 命名空间中的类,这意味着这些 API 可能会在 Qt 版本变更时不稳定
  • 如果可能,应优先使用公共 API

改进建议:

  • 确认是否有使用公共 API 替代方案
  • 如果必须使用私有 API,添加注释说明原因:
// 使用私有 API QFactoryLoader 动态加载平台主题插件
#include <private/qfactoryloader_p.h>

2. 整体代码质量

从提供的代码片段来看:

  • 头文件包含顺序符合常规(先系统头文件,后 Qt 头文件)
  • 使用了 DGUI_USE_NAMESPACE 宏,表明这是与 DDE (Deepin Desktop Environment) 集成的代码

改进建议:

  • 考虑将系统头文件和 Qt 头文件分组,并用空行分隔,提高可读性:
// System headers
#include <X11/Xlib.h>
#include <unistd.h>

// Qt headers
#include <private/qguiapplication_p.h>
#include <private/qfactoryloader_p.h>
#include <qpa/qwindowsysteminterface_p.h>

3. 安全性考虑

  • 如果 unistd.h 用于文件操作,应确保所有文件路径都经过适当验证,防止路径遍历攻击
  • 使用私有 API 时要注意 Qt 版本兼容性,可能需要在运行时检查 Qt 版本

4. 性能考虑

  • 头文件包含应该尽可能具体,避免包含整个头文件如果只需要少量功能
  • 如果 qfactoryloader_p.h 只用于特定功能,考虑是否可以延迟加载或按需加载

总结

这些变更看起来是为了支持 Deepin 桌面环境的特定功能。主要关注点是:

  1. 确认新增的头文件是否都被实际使用
  2. 为使用私有 API 添加适当的注释说明
  3. 保持代码组织清晰,便于维护
  4. 注意使用私有 API 可能带来的兼容性风险

如果这些头文件的添加是为了支持特定的平台功能,建议在代码中添加更详细的注释说明这些功能的具体用途和实现方式。

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, justforlxz

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

@BLumia BLumia merged commit c7eeeaf into linuxdeepin:master Aug 27, 2025
21 of 22 checks passed
BLumia pushed a commit to linuxdeepin/qt6integration that referenced this pull request Aug 27, 2025
Synchronize source files from linuxdeepin/qt5integration.

Source-pull-request: linuxdeepin/qt5integration#279
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.

3 participants