Skip to content

fix(wallpaper-cache): support ENABLE_DSS_SNIPE and close file handle#64

Merged
yixinshark merged 1 commit intolinuxdeepin:masterfrom
yixinshark:fix-useWallpaperCache
Apr 15, 2026
Merged

fix(wallpaper-cache): support ENABLE_DSS_SNIPE and close file handle#64
yixinshark merged 1 commit intolinuxdeepin:masterfrom
yixinshark:fix-useWallpaperCache

Conversation

@yixinshark
Copy link
Copy Markdown
Contributor

  • Use conditional compilation for wallpaper cache service verification.

  • Added DBus interface validity check when ENABLE_DSS_SNIPE is defined.

  • Close origin image file handle to prevent leaks.

  • 在验证壁纸缓存服务时增加条件编译支持。

  • 当定义了 ENABLE_DSS_SNIPE 时,增加 DBus 接口有效性检查。

  • 关闭原始图片文件句柄以防止泄漏。

Log: fix(wallpaper-cache): support ENABLE_DSS_SNIPE and close file handle
Change-Id: I5440d4c3ebed502629f415c7ddb1ff0358cf1f31

- Use conditional compilation for wallpaper cache service verification.
- Added DBus interface validity check when ENABLE_DSS_SNIPE is defined.
- Close origin image file handle to prevent leaks.

- 在验证壁纸缓存服务时增加条件编译支持。
- 当定义了 ENABLE_DSS_SNIPE 时,增加 DBus 接口有效性检查。
- 关闭原始图片文件句柄以防止泄漏。

Log: fix(wallpaper-cache): support ENABLE_DSS_SNIPE and close file handle
Change-Id: I5440d4c3ebed502629f415c7ddb1ff0358cf1f31
deepin-ci-robot added a commit to linuxdeepin/dde-session-shell-snipe that referenced this pull request Apr 15, 2026
Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#64
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码主要修改了版权年份,并增加了对 ENABLE_DSS_SNIPE 宏的条件编译支持,以适配不同的构建环境。以下是对这段 diff 的详细审查意见:

1. 语法逻辑

  • 版权年份更新:将版权年份从 2022 年更新至 2026 年,这是常规维护操作,无逻辑问题。
  • 条件编译逻辑
    • 使用 #ifndef ENABLE_DSS_SNIPE 包裹原有的文件存在性检查,这是合理的,因为不同构建环境可能需要不同的服务检测方式。
    • 新增的 #ifdef ENABLE_DSS_SNIPE 块中,增加了对 D-Bus 接口有效性的检查,这是必要的补充,因为仅检查文件存在性不足以确保服务可用。
  • 文件关闭操作:在调用 wallpaperCacheInterface.Get() 后显式调用 file.close() 是良好的实践,虽然 QFile 析构时会自动关闭文件,但显式关闭可以更早释放资源。

2. 代码质量

  • 错误处理
    • wallpaperCacheInterface.isValid() 的检查是必要的,但建议在检查失败时提供更详细的错误信息(例如通过 QDBusError 获取具体错误)。
    • pathList.value().at(0) 的访问方式存在潜在风险,如果 pathList 为空,会导致未定义行为。建议先检查 pathList 是否为空。
  • 代码可读性
    • 条件编译块较多,可能影响可读性。建议通过注释或重构(如将服务检查逻辑提取为单独函数)来改善。
    • 变量命名清晰,但 wallpaperCacheInterface 较长,可考虑简化(如 cacheInterface)。

3. 代码性能

  • 文件操作
    • QFile::open()file.close() 是必要的,但频繁的文件操作可能影响性能。如果此函数被频繁调用,建议考虑缓存文件句柄或优化文件访问策略。
  • D-Bus 调用
    • wallpaperCacheInterface.Get() 是同步调用,可能阻塞主线程。如果此函数在 UI 线程调用,建议改为异步调用(如使用 QDBusPendingCall)。

4. 代码安全

  • 路径验证
    • originPathscaledPath 未经过路径合法性验证,可能存在路径遍历风险。建议添加路径检查(如 QFileInfo::exists() 或正则验证)。
  • 资源释放
    • 虽然 file.close() 已被显式调用,但建议使用 RAII(如 QFile 的析构函数)或 QScopeGuard 确保资源在任何情况下(如异常或提前返回)都能被释放。
  • 条件编译一致性
    • 确保 ENABLE_DSS_SNIPE 在所有相关代码中一致使用,避免部分代码未正确处理宏定义导致逻辑漏洞。

改进建议

  1. 增强错误处理

    #ifdef ENABLE_DSS_SNIPE
    if (!wallpaperCacheInterface.isValid()) {
        qWarning() << "dde-wallpaper-cache service not existed:" << wallpaperCacheInterface.lastError().message();
        return false;
    }
    #endif
  2. 安全访问 pathList

    if (pathList.isEmpty() || pathList.value().isEmpty()) {
        qWarning() << "Empty path list returned from wallpaper cache service";
        return false;
    }
    QString path = pathList.value().at(0);
  3. 路径验证

    if (originPath.isEmpty() || !QFileInfo::exists(originPath)) {
        qWarning() << "Invalid origin path:" << originPath;
        return false;
    }
  4. 异步 D-Bus 调用(如果性能是关键):

    QDBusPendingCall asyncCall = wallpaperCacheInterface.asyncCall("Get", originPath, ...);
    QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(asyncCall, this);
    connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, watcher, &scaledPath]() {
        QDBusReply<QStringList> reply = *watcher;
        if (reply.isValid()) {
            // 处理结果
        } else {
            qWarning() << "D-Bus call failed:" << reply.error().message();
        }
        watcher->deleteLater();
    });
  5. 重构条件编译

    • 将服务检查逻辑提取为单独函数:
      bool isWallpaperServiceAvailable() {
      #ifdef ENABLE_DSS_SNIPE
          QDBusInterface iface("org.deepin.dde.WallpaperCache", "/org/deepin/dde/WallpaperCache",
                             "org.deepin.dde.WallpaperCache", QDBusConnection::systemBus());
          return iface.isValid();
      #else
          return QFile::exists("/lib/systemd/system/dde-wallpaper-cache.service");
      #endif
      }

总结

这段代码的主要改进方向是增强错误处理、安全性检查和性能优化。通过上述建议,可以提升代码的健壮性、安全性和可维护性。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: waterlovemelon, yixinshark

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

@yixinshark yixinshark merged commit deedff7 into linuxdeepin:master Apr 15, 2026
15 of 16 checks passed
yixinshark pushed a commit to linuxdeepin/dde-session-shell-snipe that referenced this pull request Apr 15, 2026
Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#64
yixinshark pushed a commit to linuxdeepin/dde-session-shell-snipe that referenced this pull request Apr 15, 2026
Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#64
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