Skip to content

fix: fix fd marshaling in QtDBus#1715

Merged
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_dbusfd
Jul 1, 2026
Merged

fix: fix fd marshaling in QtDBus#1715
dengbo11 merged 1 commit into
OpenAtom-Linyaps:masterfrom
reddevillg:fix_dbusfd

Conversation

@reddevillg

Copy link
Copy Markdown
Collaborator

Call Ping() to trigger QtDBus's file descriptor passing capability update.

Call Ping() to trigger QtDBus's file descriptor passing capability
update.

Signed-off-by: reddevillg <reddevillg@gmail.com>
@reddevillg reddevillg requested a review from dengbo11 July 1, 2026 08:22
@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

deepin pr auto review

★ 总体评分:100分

■ 【总体评价】

代码修复了peerMode下缺失存活检测与文件描述符传递能力校验的问题,逻辑重构合理
逻辑完全正确且消除了重复代码,无任何扣分项

■ 【详细分析】

  • 1.语法逻辑(完全正确)✓

修复了旧代码在peerMode分支直接实例化pkgMan而跳过Ping检测的逻辑缺陷,通过将Ping检测提取至公共路径,确保了两种模式下的连接有效性验证。同时统一了pkgMan对象的实例化代码,消除了重复逻辑。
潜在问题:无
建议:无

  • 2.代码质量(良好)✓

代码遵循DRY原则,将peerMode和非peerMode下的公共逻辑进行合并,显著提升了代码的可读性和可维护性。错误提示信息根据peerMode进行了差异化处理,更加友好。
潜在问题:无
建议:无

  • 3.代码性能(无性能问题)✓

重构过程未引入额外的系统调用或资源消耗,QDBusConnection的隐式共享机制保证了赋值操作的高效性。
建议:无

  • 4.代码安全(存在0个安全漏洞)✓

存在 0 个安全漏洞
漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码在installFromFile函数中增加了对UnixFileDescriptorPassing能力的显式校验,防止在不支持该特性的连接上传递文件描述符导致未定义行为或静默失败,增强了接口调用的安全性。

  • 建议:无

■ 【改进建议代码示例】

// 当前diff中的修复代码已经非常规范,无需额外改进,此为当前最终正确形态的展示
utils::error::Result<api::dbus::v1::PackageManager *> Cli::getPkgMan()
{
    if (this->pkgMan != nullptr) {
        return this->pkgMan.get();
    }

    QString service;
    QDBusConnection pkgManConn{ "ll-package-manager" };

    if (this->peerMode) {
        LogW("some subcommands will failed in --no-dbus mode.");
        const auto pkgManAddress = QString("unix:path=/tmp/linglong-package-manager.socket");
        pkgManConn = QDBusConnection::connectToPeer(pkgManAddress, "ll-package-manager");
        if (!pkgManConn.isConnected()) {
            return LINGLONG_ERR(fmt::format("Failed to connect to ll-package-manager: {}",
                                            pkgManConn.lastError().message().toStdString()));
        }
    } else {
        service = "org.deepin.linglong.PackageManager1";
        pkgManConn = QDBusConnection::systemBus();
    }

    auto peer = linglong::api::dbus::v1::DBusPeer(service,
                                                  "/org/deepin/linglong/PackageManager1",
                                                  pkgManConn);
    auto reply = peer.Ping();
    reply.waitForFinished();
    if (!reply.isValid()) {
        const auto message = this->peerMode
          ? "Failed to initialize peer connection: {}"
          : "Failed to activate org.deepin.linglong.PackageManager1: {}";
        return LINGLONG_ERR(fmt::format(message, reply.error().message().toStdString()));
    }

    this->pkgMan =
      std::make_unique<api::dbus::v1::PackageManager>(service,
                                                      "/org/deepin/linglong/PackageManager1",
                                                      pkgManConn,
                                                      QCoreApplication::instance());

    auto ret = this->initPkgManSignals();
    if (!ret) {
        this->pkgMan.reset();
        return LINGLONG_ERR(ret.error());
    }

    return this->pkgMan.get();
}

int Cli::installFromFile(const QFileInfo &fileInfo, common::CommonOptions &commonOptions) {
    // ... 前置获取pkgMan及文件校验逻辑 ...
    
    auto caps = (*pkgMan)->connection().connectionCapabilities();
    if (!(caps & QDBusConnection::UnixFileDescriptorPassing)) {
        this->printer.printErr(LINGLONG_ERRV("peer connection does not support Unix FD passing"));
        return -1;
    }

    auto pendingReply = (*pkgMan)->InstallFromFile(dbusFileDescriptor,
                                                   fileInfo.suffix(),
                                                   common::serialize::toQVariantMap(commonOptions));
    // ... 后续处理逻辑 ...
}

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
libs/linglong/src/linglong/cli/cli.cpp 0.00% 18 Missing ⚠️
Files with missing lines Coverage Δ
libs/linglong/src/linglong/cli/cli.cpp 1.76% <0.00%> (-0.01%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@deepin-ci-robot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengbo11, reddevillg

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

@dengbo11 dengbo11 merged commit 3389e09 into OpenAtom-Linyaps:master Jul 1, 2026
17 of 18 checks passed
@reddevillg reddevillg deleted the fix_dbusfd branch July 1, 2026 08:40
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