Skip to content

Conversation

@Johnson-zs
Copy link
Contributor

Changed DBus service names and paths to use new naming convention for Qt6 version while maintaining backward compatibility with Qt5. The old service names 'com.deepin.filemanager.daemon' are replaced with 'org.deepin.Filemanager.MountControl' for Qt6 builds. Also removed unused error message key constant and improved code formatting for consistency.

Log: Fixed network mount functionality for Qt6 compatibility

Influence:

  1. Test network device mounting and unmounting on both Qt5 and Qt6 environments
  2. Verify DBus communication with the mount control service
  3. Check error handling for authentication failures and authorization dismissals
  4. Test network operations with different file protocols (SMB, etc.)

fix: 更新 DBus 服务名称以支持 Qt6 兼容性

修改了 DBus 服务名称和路径,在 Qt6 版本中使用新的
命名约定,同时保持对 Qt5 的向后兼容性。Qt6 构建中将
旧的服务名称 'com.deepin.filemanager.daemon' 替换为
'org.deepin.Filemanager.MountControl'。同时移除了未使用的错误消息键常 量,并改进了代码格式以保持一致性。

Log: 修复网络挂载功能以支持 Qt6 兼容性

Influence:

  1. 在 Qt5 和 Qt6 环境中测试网络设备的挂载和卸载功能
  2. 验证与挂载控制服务的 DBus 通信
  3. 检查认证失败和授权取消的错误处理
  4. 测试不同文件协议(SMB 等)的网络操作

Changed DBus service names and paths to use new naming convention for
Qt6 version while maintaining backward compatibility with Qt5. The
old service names 'com.deepin.filemanager.daemon' are replaced with
'org.deepin.Filemanager.MountControl' for Qt6 builds. Also removed
unused error message key constant and improved code formatting for
consistency.

Log: Fixed network mount functionality for Qt6 compatibility

Influence:
1. Test network device mounting and unmounting on both Qt5 and Qt6
environments
2. Verify DBus communication with the mount control service
3. Check error handling for authentication failures and authorization
dismissals
4. Test network operations with different file protocols (SMB, etc.)

fix: 更新 DBus 服务名称以支持 Qt6 兼容性

修改了 DBus 服务名称和路径,在 Qt6 版本中使用新的
命名约定,同时保持对 Qt5 的向后兼容性。Qt6 构建中将
旧的服务名称 'com.deepin.filemanager.daemon' 替换为
'org.deepin.Filemanager.MountControl'。同时移除了未使用的错误消息键常
量,并改进了代码格式以保持一致性。

Log: 修复网络挂载功能以支持 Qt6 兼容性

Influence:
1. 在 Qt5 和 Qt6 环境中测试网络设备的挂载和卸载功能
2. 验证与挂载控制服务的 DBus 通信
3. 检查认证失败和授权取消的错误处理
4. 测试不同文件协议(SMB 等)的网络操作
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要进行了一些重构和兼容性调整,以下是对代码的详细审查和改进建议:

1. 语法逻辑审查

优点:

  • 正确使用了 #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) 预处理指令来实现 Qt5 和 Qt6 的兼容性
  • 代码结构清晰,常量定义合理
  • 错误处理逻辑基本完整

问题与建议:

  1. 条件编译范围
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
static constexpr char kDaemonService[] { "org.deepin.Filemanager.MountControl" };
// ...
#else
static constexpr char kDaemonService[] { "com.deepin.filemanager.daemon" };
// ...
#endif
  • 建议将所有与 DBus 服务路径相关的常量都放入条件编译块中,保持一致性
  • 考虑使用命名空间或类静态成员来组织这些常量,避免全局命名空间污染
  1. 错误处理逻辑
if (e.type() == QDBusError::NoReply || name == QStringLiteral("org.freedesktop.DBus.Error.NoReply")) {
  • 这里的条件判断有些冗余,QDBusError::NoReply 对应的错误名称就是 org.freedesktop.DBus.Error.NoReply
  • 建议简化为:if (e.type() == QDBusError::NoReply)

2. 代码质量审查

优点:

  • 使用了 static constexpr 来定义常量,这是良好的 C++ 实践
  • 错误信息字符串使用 QStringLiteral 包装,避免了不必要的内存分配

改进建议:

  1. 常量组织
    • 建议将所有 DBus 相关常量组织到一个命名空间或类中:
namespace DBusConstants {
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
    static constexpr char kDaemonService[] { "org.deepin.Filemanager.MountControl" };
#else
    static constexpr char kDaemonService[] { "com.deepin.filemanager.daemon" };
#endif
    // 其他常量...
}
  1. 字符串比较
if (name == QStringLiteral("org.freedesktop.DBus.Error.NoReply")) {
  • 考虑使用 QLatin1String 进行比较,效率更高:
if (name == QLatin1String("org.freedesktop.DBus.Error.NoReply")) {
  1. 代码格式
    • 建议统一使用空格而不是制表符进行缩进
    • 确保整个项目中使用一致的代码风格

3. 代码性能审查

  1. 字符串处理

    • 在 Qt6 中,QStringLiteral 的实现有所优化,但在 Qt5 中仍然有一定开销
    • 对于频繁比较的字符串,考虑使用 QLatin1String 或预定义的 QString 静态变量
  2. 错误信息生成

    • Utils::genOperateErrorInfo 可能会在每次调用时生成新的字符串
    • 考虑缓存常用的错误信息

4. 代码安全审查

  1. 输入验证
    • unmountNetworkDevAsyncunmountNetworkDevAsyncDetailed 方法接收 mpt 参数
    • 建议添加对 mpt 参数的验证,防止路径遍历攻击:
bool DNetworkMounter::unmountNetworkDevAsyncDetailed(const QString &mpt, int *errCode, QString *errMsg) {
    if (mpt.isEmpty() || mpt.contains("..")) {
        if (errCode) *errCode = static_cast<int>(DeviceError::kUserError);
        if (errMsg) *errMsg = QStringLiteral("Invalid mount point");
        return false;
    }
    // ...
}
  1. DBus 通信安全

    • 确保 DBus 通信使用适当的权限和认证机制
    • 考虑添加对 DBus 调用来源的验证
  2. 错误信息泄露

    • 确保错误信息不包含敏感信息
    • 考虑对错误信息进行适当的过滤或脱敏

5. 其他建议

  1. 日志记录

    • 建议在关键操作处添加日志记录,便于调试和问题追踪
    • 例如:记录挂载/卸载操作的开始和结束状态
  2. 单元测试

    • 建议为这些方法添加单元测试,特别是错误处理逻辑
    • 测试用例应包括各种错误场景
  3. 文档注释

    • 建议为公共方法添加详细的文档注释,说明参数、返回值和可能的异常

修改后的代码示例:

namespace DBusConstants {
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
    static constexpr char kDaemonService[] { "org.deepin.Filemanager.MountControl" };
    static constexpr char kDaemonPath[] { "/org/deepin/Filemanager" };
    static constexpr char kMountControlPath[] { "/org/deepin/Filemanager/MountControl" };
    static constexpr char kMountControlIFace[] { "org.deepin.Filemanager.MountControl" };
#else
    static constexpr char kDaemonService[] { "com.deepin.filemanager.daemon" };
    static constexpr char kDaemonPath[] { "/com/deepin/filemanager/daemon" };
    static constexpr char kMountControlPath[] { "/com/deepin/filemanager/daemon/MountControl" };
    static constexpr char kMountControlIFace[] { "com.deepin.filemanager.daemon.MountControl" };
#endif

    static constexpr char kDaemonIntro[] { "org.freedesktop.DBus.Introspectable" };
    static constexpr char kDaemonIntroMethod[] { "Introspect" };
    static constexpr char kMountControlMount[] { "Mount" };
    static constexpr char kMountControlUnmount[] { "Unmount" };
}

bool DNetworkMounter::unmountNetworkDevAsyncDetailed(const QString &mpt, int *errCode, QString *errMsg) {
    // 输入验证
    if (mpt.isEmpty() || mpt.contains("..")) {
        if (errCode) *errCode = static_cast<int>(DeviceError::kUserError);
        if (errMsg) *errMsg = QStringLiteral("Invalid mount point");
        return false;
    }

    // ... 其他代码 ...

    if (!ret.isValid()) {
        const auto &e = ret.error();
        if (e.type() == QDBusError::NoReply) {
            if (errCode) *errCode = static_cast<int>(DFMMOUNT::DeviceError::kGDBusErrorNoReply);
            if (errMsg) *errMsg = QStringLiteral("Authorization pending (NoReply)");
            return false;
        }
        // ... 其他错误处理 ...
    }

    // ... 其他代码 ...
}

这些改进将使代码更加健壮、安全和易于维护。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: itsXuSt, Johnson-zs

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

@Johnson-zs
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Jan 14, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit ae95e36 into linuxdeepin:master Jan 14, 2026
20 checks passed
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