Fix: improve QProcess usage for Qt6 compatibility by GongHeng2017 · Pull Request #621 · linuxdeepin/deepin-devicemanager

GongHeng2017 · 2026-03-20T12:28:06Z

Refactored QProcess::start() calls in parseEDID() to use the modern API by separating program name and arguments instead of shell string concatenation.
Added return value validation for waitForFinished() to handle process execution failures gracefully.
Applied QString::fromLocal8Bit() to standard output reading to ensure proper text encoding handling.
These changes improve code robustness, security (avoiding shell injection risks), and compatibility with Qt6 best practices.

Log: fix issue
Bug: https://pms.uniontech.com/bug-view-353603.html

Summary by Sourcery

Improve EDID parsing by hardening external hexdump invocation and output handling for Qt6 compatibility.

Bug Fixes:

Prevent potential failures and undefined behavior when invoking hexdump by using the QProcess program/arguments API instead of shell command strings and by checking process completion results.

Enhancements:

Convert hexdump output using QString::fromLocal8Bit() and add logging for empty EDID files and failed hexdump calls to improve robustness and diagnostics.

sourcery-ai · 2026-03-20T12:28:34Z

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors QProcess usage in CommonTools::parseEDID to use the program/arguments API instead of shell command strings, adds basic error/empty-output handling, and ensures standard output is decoded via QString::fromLocal8Bit for more robust and Qt6-aligned process execution.

Sequence diagram for updated parseEDID QProcess interaction

sequenceDiagram
    participant Caller as Caller
    participant CommonTools as CommonTools
    participant QProcess as QProcess

    Caller->>CommonTools: parseEDID(allEDIDS, input, isHW)
    loop for each edid in allEDIDS
        alt isHW
            CommonTools->>QProcess: start("hexdump", [edid])
        else not isHW
            CommonTools->>QProcess: start("hexdump", ["-C", edid])
        end

        CommonTools->>QProcess: waitForFinished(-1)
        alt waitForFinished indicates failure
            CommonTools-->>Caller: log qCritical("Failed to hexdump edid file!")
            CommonTools-->>CommonTools: continue to next edid
        else process finished successfully
            CommonTools->>QProcess: readAllStandardOutput()
            QProcess-->>CommonTools: QByteArray rawOutput
            CommonTools-->>CommonTools: deviceInfo = QString::fromLocal8Bit(rawOutput)
            alt deviceInfo is empty
                CommonTools-->>Caller: log qWarning("The edid file is empty!", edid)
                CommonTools-->>CommonTools: continue to next edid
            else deviceInfo not empty
                CommonTools-->>CommonTools: parse lines and update edidStr
            end
        end
    end
    CommonTools-->>Caller: return

Class diagram for CommonTools::parseEDID and QProcess usage

classDiagram
    class CommonTools {
        +void parseEDID(QStringList allEDIDS, QString input, bool isHW)
    }

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

    CommonTools ..> QProcess : uses

File-Level Changes

Change	Details	Files
Refactor QProcess invocations in parseEDID to use program/arguments API and add error handling and encoding fixes.	Replace shell-concatenated QProcess::start("hexdump ...") calls with QProcess::start("hexdump", QStringList() << args) to avoid shell invocation and align with Qt6 best practices. Guard waitForFinished(-1) with a return-value check to detect hexdump failures and skip EDID parsing on error, logging a critical message. Decode process standard output with QString::fromLocal8Bit(readAllStandardOutput()) instead of using the raw QByteArray to ensure proper text encoding handling. Add warnings and early-continue when the hexdump output for an EDID is empty, distinguishing between process failure (critical) and empty content (warning).	`deepin-devicemanager/src/Tool/commontools.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

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey - I've found 1 issue, and left some high level feedback:

The if (process.waitForFinished(-1)) checks are inverted: waitForFinished returns true on success, so the error path should be taken when it returns false.
When logging failures to run hexdump, consider including process.error() or readAllStandardError() to aid debugging instead of only a generic message.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `if (process.waitForFinished(-1))` checks are inverted: `waitForFinished` returns `true` on success, so the error path should be taken when it returns `false`.
- When logging failures to run `hexdump`, consider including `process.error()` or `readAllStandardError()` to aid debugging instead of only a generic message.

## Individual Comments

### Comment 1
<location path="deepin-devicemanager/src/Tool/commontools.cpp" line_range="202" />
<code_context>
-            process.start(QString("hexdump %1").arg(edid));
-            process.waitForFinished(-1);
+            process.start("hexdump", QStringList() << edid);
+            if (process.waitForFinished(-1)) {
+                qCritical() << "Failed to hexdump edid file!";
+                continue;
</code_context>
<issue_to_address>
**issue (bug_risk):** The success check for waitForFinished is inverted, so failures are silently ignored and successes are treated as errors.

`QProcess::waitForFinished(-1)` returns true on success and false on failure. Here, a successful finish logs an error and continues, while failures are ignored. This should be inverted, e.g. `if (!process.waitForFinished(-1)) { ... }`, so actual hexdump failures are handled.
</issue_to_address>

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-devicemanager/src/Tool/commontools.cpp

github-actions · 2026-03-20T12:29:18Z

Note

[静态代码检查]

敏感词检查失败, 检测到1个文件存在敏感词

详情

{
    "deepin-devicemanager/src/Tool/commontools.cpp": [
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 177,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 182,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://drive-pre.uniontech.com/api/v1/drive/search\";",
            "line_number": 185,
            "rule": "S35",
            "reason": "Url link | 0e010283c1"
        }
    ]
}

github-actions · 2026-03-20T12:48:00Z

Note

[静态代码检查]

敏感词检查失败, 检测到1个文件存在敏感词

详情

{
    "deepin-devicemanager/src/Tool/commontools.cpp": [
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 177,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 182,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://drive-pre.uniontech.com/api/v1/drive/search\";",
            "line_number": 185,
            "rule": "S35",
            "reason": "Url link | 0e010283c1"
        }
    ]
}

- Refactored QProcess::start() calls in parseEDID() to use the modern API by separating program name and arguments instead of shell string concatenation. - Added return value validation for waitForFinished() to handle process execution failures gracefully. - Applied QString::fromLocal8Bit() to standard output reading to ensure proper text encoding handling. - These changes improve code robustness, security (avoiding shell injection risks), and compatibility with Qt6 best practices. Log: fix issue Bug: https://pms.uniontech.com/bug-view-353603.html

deepin-ci-robot · 2026-03-20T13:12:11Z

deepin pr auto review

这段代码的修改主要针对 QProcess 的使用方式进行了优化，提高了代码的安全性和健壮性。以下是对代码的详细审查意见：

1. 语法逻辑

改进点：
- 将 process.start(QString("hexdump %1").arg(edid)) 改为 process.start("hexdump", QStringList() << edid)。
- 原因：原代码使用字符串拼接构造命令，容易受到命令注入攻击。改进后使用参数列表形式，QProcess 会自动处理参数，避免了注入风险。
改进点：
- 将 process.waitForFinished(-1) 改为 process.waitForFinished(3000)。
- 原因：原代码无限等待进程结束，可能导致程序卡死。改进后设置超时时间为 3 秒，避免长时间阻塞。

2. 代码质量

改进点：
- 添加了错误日志输出（qCritical 和 qWarning）。
- 原因：原代码在失败时静默跳过，难以排查问题。改进后输出错误信息，便于调试。
改进点：
- 使用 QString::fromLocal8Bit 处理 process.readAllStandardOutput() 的结果。
- 原因：hexdump 的输出可能是本地编码（如 UTF-8 或 GBK），直接转换为 QString 可能导致乱码。改进后显式指定编码，提高兼容性。

3. 代码性能

改进点：
- 设置了超时时间（3000 毫秒），避免无限等待。
- 原因：原代码可能因 hexdump 进程卡死而阻塞主线程。改进后超时机制提高了程序的响应性。

4. 代码安全

改进点：
- 使用参数列表形式启动 hexdump，避免命令注入。
- 原因：原代码通过字符串拼接构造命令，如果 edid 文件路径包含恶意字符（如 ; rm -rf /），可能导致严重安全问题。改进后 QProcess 会正确转义参数，避免注入。
改进点：
- 检查 deviceInfo 是否为空，并输出警告。
- 原因：原代码未检查输出是否为空，可能导致后续逻辑异常。改进后提前跳过无效数据，提高健壮性。

5. 进一步优化建议

统一错误处理：

可以将 hexdump 的调用封装为一个单独的函数，避免重复代码。例如：

QString runHexdump(const QString &filePath, bool useFormatC) {
    QProcess process;
    QStringList args;
    if (useFormatC) args << "-C";
    args << filePath;
    process.start("hexdump", args);
    if (!process.waitForFinished(3000)) {
        qCritical() << "Failed to hexdump file:" << filePath;
        return QString();
    }
    QString output = QString::fromLocal8Bit(process.readAllStandardOutput());
    if (output.isEmpty()) {
        qWarning() << "Hexdump output is empty for file:" << filePath;
    }
    return output;
}

然后在 parseEDID 中调用：

QString deviceInfo = runHexdump(edid, !isHW);
if (deviceInfo.isEmpty()) continue;

超时时间可配置：
- 可以将超时时间定义为常量或通过参数传入，避免硬编码：
```
const int HEXDUMP_TIMEOUT_MS = 3000;
```

检查 hexdump 是否存在：

在调用 hexdump 前，可以检查命令是否存在：

QProcess which;
which.start("which", QStringList() << "hexdump");
if (!which.waitForFinished(1000) || which.readAllStandardOutput().isEmpty()) {
    qCritical() << "hexdump command not found!";
    return;
}

避免频繁创建 QProcess：
- 如果 parseEDID 被频繁调用，可以考虑复用 QProcess 对象，减少资源开销。

总结

这段代码的改进显著提升了安全性、健壮性和可维护性。通过使用参数列表启动进程、设置超时时间、显式处理编码和添加错误日志，代码的质量和安全性得到了明显提升。进一步优化可以减少重复代码并提高灵活性。

github-actions · 2026-03-20T13:13:40Z

Note

[静态代码检查]

敏感词检查失败, 检测到1个文件存在敏感词

详情

{
    "deepin-devicemanager/src/Tool/commontools.cpp": [
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 177,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 182,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://drive-pre.uniontech.com/api/v1/drive/search\";",
            "line_number": 185,
            "rule": "S35",
            "reason": "Url link | 0e010283c1"
        }
    ]
}

deepin-ci-robot · 2026-03-23T01:33:59Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

GongHeng2017 · 2026-03-23T01:37:53Z

/forcemerge

sourcery-ai bot reviewed Mar 20, 2026

View reviewed changes

deepin-devicemanager/src/Tool/commontools.cpp Outdated Show resolved Hide resolved

GongHeng2017 force-pushed the 202603201959-master-fix branch from e933af3 to f03c2cf Compare March 20, 2026 12:46

GongHeng2017 force-pushed the 202603201959-master-fix branch from f03c2cf to 949a9dd Compare March 20, 2026 13:11

lzwind approved these changes Mar 23, 2026

View reviewed changes

deepin-bot bot merged commit 0769ab6 into linuxdeepin:master Mar 23, 2026
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: improve QProcess usage for Qt6 compatibility#621

Fix: improve QProcess usage for Qt6 compatibility#621
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
GongHeng2017:202603201959-master-fix

GongHeng2017 commented Mar 20, 2026 •

edited by sourcery-ai bot

Loading

Uh oh!

sourcery-ai bot commented Mar 20, 2026

Reviewer's Guide

Sequence diagram for updated parseEDID QProcess interaction

Class diagram for CommonTools::parseEDID and QProcess usage

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Uh oh!

Uh oh!

github-actions bot commented Mar 20, 2026

Uh oh!

github-actions bot commented Mar 20, 2026

Uh oh!

deepin-ci-robot commented Mar 20, 2026

Uh oh!

github-actions bot commented Mar 20, 2026

Uh oh!

deepin-ci-robot commented Mar 23, 2026

Uh oh!

GongHeng2017 commented Mar 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

GongHeng2017 commented Mar 20, 2026 • edited by sourcery-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Mar 20, 2026

Reviewer's Guide

Sequence diagram for updated parseEDID QProcess interaction

Class diagram for CommonTools::parseEDID and QProcess usage

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Mar 20, 2026

Uh oh!

github-actions bot commented Mar 20, 2026

Uh oh!

deepin-ci-robot commented Mar 20, 2026

deepin pr auto review

1. 语法逻辑

2. 代码质量

3. 代码性能

4. 代码安全

5. 进一步优化建议

总结

Uh oh!

github-actions bot commented Mar 20, 2026

Uh oh!

deepin-ci-robot commented Mar 23, 2026

Uh oh!

GongHeng2017 commented Mar 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GongHeng2017 commented Mar 20, 2026 •

edited by sourcery-ai bot

Loading