Skip to content

fix: update QProcess command syntax for package management#604

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:develop/eaglefrom
dengzhongyuan365-dev:develop/eagle
Feb 4, 2026
Merged

fix: update QProcess command syntax for package management#604
deepin-bot[bot] merged 1 commit intolinuxdeepin:develop/eaglefrom
dengzhongyuan365-dev:develop/eagle

Conversation

@dengzhongyuan365-dev
Copy link
Contributor

@dengzhongyuan365-dev dengzhongyuan365-dev commented Feb 4, 2026

Refactor the way QProcess is used to execute package management commands by replacing string concatenation with QStringList for better readability and reliability. This change affects the installation, uninstallation, and status checking of packages in both the device manager and driver manager components.

bug: https://pms.uniontech.com/task-view-386519.html

Summary by Sourcery

Update package management QProcess invocations to use argument-based syntax instead of concatenated command strings for more robust driver and device management operations.

Bug Fixes:

  • Correct package installation, removal, and status-check commands by avoiding shell-concatenated strings and invoking dpkg/apt directly with arguments.

Enhancements:

  • Align printer and generic package management routines across device and driver managers to use consistent QProcess argument handling.

Refactor the way QProcess is used to execute package management commands by replacing string concatenation with QStringList for better readability and reliability. This change affects the installation, uninstallation, and status checking of packages in both the device manager and driver manager components.

bug: https://pms.uniontech.com/task-view-386519.html
@sourcery-ai
Copy link

sourcery-ai bot commented Feb 4, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors QProcess usage for package management commands to use explicit program and argument lists instead of concatenated command strings, improving clarity, safety, and consistency across driver and device manager components.

Sequence diagram for updated printer installation flow using QProcess arguments

sequenceDiagram
    actor User
    participant DriverManager
    participant QProcess_install
    participant QProcess_check
    participant apt
    participant dpkg

    User ->> DriverManager: installPrinter(packageName)
    activate DriverManager

    DriverManager ->> QProcess_install: start("apt", ["install", packageName])
    activate QProcess_install
    QProcess_install ->> apt: run install packageName
    apt -->> QProcess_install: exit status
    QProcess_install -->> DriverManager: finished
    deactivate QProcess_install

    DriverManager ->> QProcess_check: start("dpkg", ["-s", packageName])
    activate QProcess_check
    QProcess_check ->> dpkg: query status packageName
    dpkg -->> QProcess_check: package status output
    QProcess_check -->> DriverManager: finished
    deactivate QProcess_check

    DriverManager -->> User: return printerHasInstalled(packageName)
    deactivate DriverManager
Loading

Class diagram for updated QProcess-based package operations

classDiagram
    class DriverManager {
        +bool printerHasInstalled(QString packageName)
        +bool installPrinter(QString packageName)
        +bool unInstallPrinter(QString packageName)
    }

    class PageInfo {
        +bool getMultiFlag()
        +bool packageHasInstalled(QString packageName)
    }

    class Utils {
        +bool addModBlackList(QString moduleName)
        +bool unInstallPackage(QString packageName)
    }

    class QProcess {
        +void start(QString program, QStringList arguments)
        +bool waitForFinished(int msecs)
        +QByteArray readAll()
    }

    DriverManager ..> QProcess : uses
    PageInfo ..> QProcess : uses
    Utils ..> QProcess : uses
Loading

File-Level Changes

Change Details Files
Standardize QProcess calls in DriverManager printer package operations to use explicit program and argument lists for dpkg and apt.
  • Change printerHasInstalled to start QProcess with program "dpkg" and arguments ["-s", packageName] instead of a concatenated "sudo dpkg -s" string
  • Change installPrinter to start QProcess with program "apt" and arguments ["install", packageName] instead of a concatenated "sudo apt install" string
  • Change unInstallPrinter to start QProcess with program "dpkg" and arguments ["-r", packageName] instead of a concatenated "sudo dpkg -r" string
deepin-devicemanager-server/deepin-devicecontrol/src/drivercontrol/drivermanager.cpp
Align package status checking in PageInfo with the new QProcess argument-list style.
  • Change packageHasInstalled to start QProcess with program "dpkg" and arguments ["-s", packageName] instead of a concatenated "dpkg -s" string
deepin-devicemanager/src/Page/PageInfo.cpp
Update Utils package uninstallation to use QProcess with separate program and arguments for apt remove.
  • Change unInstallPackage to start QProcess with program "apt" and arguments ["remove", packageName] instead of a formatted "apt remove " command string
deepin-devicemanager-server/deepin-devicecontrol/src/drivercontrol/utils.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的修改主要是将命令行字符串拼接的方式改为使用 QProcess 的列表参数形式。这是一个很好的改进,以下是详细的代码审查意见:

1. 语法逻辑

现状
修改后的代码将 p.start("cmd string") 改为了 p.start("program", QStringList() << "arg1" << "arg2")
评价
改进正确且必要。之前的字符串拼接方式(如 "sudo dpkg -s " + packageName)虽然能工作,但在逻辑上存在缺陷。如果 packageName 中包含空格,命令会被错误解析。使用参数列表形式可以确保参数被正确传递给子进程,避免了 Shell 解析的歧义。

2. 代码质量

现状
代码中移除了 sudo 命令,直接调用 dpkgapt 等系统命令。
评价
存在潜在风险

  1. 权限问题dpkgapt 通常需要 root 权限才能安装/卸载软件包。如果调用此代码的服务或应用程序没有以 root 权限运行(例如通过 systemd 服务配置了 User=root 或通过 polkit 授权),这些命令将会执行失败。
  2. 上下文缺失:从代码片段看,这是一个名为 deepin-devicecontrol 的服务器组件。通常这类服务后端会以 root 权限运行,或者通过 DBus/polkits 提升权限。如果该服务本身运行在 root 下,移除 sudo 是合理的(避免多余的进程)。建议确认运行环境是否具备 root 权限

3. 代码性能

现状
使用了 p.waitForFinished(-1) 阻塞等待进程结束。
评价
性能一般,建议异步化

  1. QProcess 是同步阻塞调用,会阻塞当前线程。如果这是在 GUI 线程(UI 线程)中调用,会导致界面卡死,用户体验极差。
  2. 如果这是在服务器后端的工作线程中调用,阻塞是可接受的,但仍需注意超时控制。
  3. 建议:如果可能,考虑使用信号槽机制(QProcess::finished)进行异步处理,或者至少设置一个合理的超时时间(例如 waitForFinished(30000)),而不是无限等待 -1,防止进程僵死导致线程永久挂起。

4. 代码安全

现状
使用了参数列表形式传递 packageName
评价
安全性有所提升,但仍需注意输入验证

  1. 命令注入防护:改为参数列表形式后,基本防止了 Shell 命令注入攻击(例如 packageNamepackage; rm -rf /)。这是此次修改最大的安全收益。
  2. 参数验证:虽然避免了 Shell 注入,但仍建议对 packageName 进行格式校验。例如,检查是否包含非法字符(如 ../ 或控制字符),防止向底层包管理器传递恶意构造的包名,尽管 dpkg 自身有一定的校验机制,但防御性编程总是好的。

5. 其他改进意见

  1. 错误处理缺失
    代码中调用了 startwaitForFinished,但没有检查 QProcess 的错误状态(如 error()exitStatus())。

    • 建议:在执行完命令后,检查 process.exitCode()process.error()。例如,apt install 失败可能有多种原因(网络问题、依赖冲突),仅通过 printerHasInstalled 来判断成功与否可能不够精确。
    // 示例改进
    p.start("apt", QStringList() << "install" << packageName);
    if (!p.waitForFinished(30000)) { // 设置30秒超时
        qWarning() << "Install timeout or failed:" << p.errorString();
        return false;
    }
    if (p.exitCode() != 0) {
        qWarning() << "Install failed with exit code:" << p.exitCode();
        qDebug() << "Error output:" << p.readAllStandardError();
        return false;
    }
  2. 环境变量与路径
    直接调用 aptdpkg 依赖于系统 PATH 环境变量。在某些极简或安全加固的环境中,PATH 可能不包含这些路径。

    • 建议:使用绝对路径,例如 /usr/bin/apt/usr/bin/dpkg,以增加健壮性。

总结

这次修改在防止命令注入参数解析方面做得很好,是正确的重构方向。但在权限管理异步处理错误处理方面还有进一步优化的空间。请务必确认服务运行权限,并补充对命令执行结果的详细检查。

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, 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

@dengzhongyuan365-dev
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Feb 4, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 6e16e08 into linuxdeepin:develop/eagle Feb 4, 2026
16 of 18 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