Fix: improve QProcess usage for Qt6 compatibility#622
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors QProcess usage in CommonTools::parseEDID() to use the argument-based start() API, adds robust error handling and logging around process execution and empty output, and ensures stdout is decoded with QString::fromLocal8Bit() for better Qt6 compatibility and robustness. Sequence diagram for updated CommonTools_parseEDID QProcess handlingsequenceDiagram
participant CommonTools
participant QProcess
participant hexdump
participant FileSystem
CommonTools->>CommonTools: parseEDID(allEDIDS, input, isHW)
loop for each edid in allEDIDS
alt isHW is true
CommonTools->>QProcess: QProcess()
CommonTools->>QProcess: start(hexdump, [edid])
else isHW is false
CommonTools->>QProcess: QProcess()
CommonTools->>QProcess: start(hexdump, [-C, edid])
end
QProcess->>hexdump: execute with arguments
hexdump->>FileSystem: read edid file
FileSystem-->>hexdump: edid bytes
hexdump-->>QProcess: hex dump output
CommonTools->>QProcess: waitForFinished(-1)
alt waitForFinished returns false
CommonTools-->>CommonTools: qCritical Failed to hexdump edid file
CommonTools-->>CommonTools: continue to next edid
else waitForFinished returns true
CommonTools->>QProcess: readAllStandardOutput()
QProcess-->>CommonTools: rawLocal8BitData
CommonTools-->>CommonTools: deviceInfo = QString_fromLocal8Bit(rawLocal8BitData)
alt deviceInfo is empty
CommonTools-->>CommonTools: qWarning edid file is empty
CommonTools-->>CommonTools: continue to next edid
else deviceInfo is nonempty
CommonTools-->>CommonTools: split deviceInfo into lines
CommonTools-->>CommonTools: parse lines for EDID data
end
end
end
Class diagram for CommonTools_parseEDID and QProcess interactionclassDiagram
class CommonTools {
+void parseEDID(QStringList allEDIDS, QString input, bool isHW)
}
class QProcess {
+QProcess()
+void start(QString program, QStringList arguments)
+bool waitForFinished(int msecs)
+QByteArray readAllStandardOutput()
}
class QString {
+static QString fromLocal8Bit(QByteArray byteArray)
}
class Logger {
+static void qCritical(QString message)
+static void qWarning(QString message)
}
CommonTools --> QProcess : uses
CommonTools --> QString : decodesOutput
CommonTools --> Logger : logsErrorsAndWarnings
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note
详情{
"deepin-devicemanager/src/Tool/commontools.cpp": [
{
"line": " return \"https://driver.uniontech.com/api/v1/drive/search\";",
"line_number": 165,
"rule": "S35",
"reason": "Url link | 162cef9d4a"
},
{
"line": " return \"https://driver.uniontech.com/api/v1/drive/search\";",
"line_number": 169,
"rule": "S35",
"reason": "Url link | 162cef9d4a"
},
{
"line": " return \"https://drive-pre.uniontech.com/api/v1/drive/search\";",
"line_number": 171,
"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
0c00178 to
f54df7c
Compare
deepin pr auto review这段代码的修改主要针对 1. 语法与逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结这次修改显著提升了代码的健壮性(超时控制)和安全性(参数分离),并改善了兼容性(字符编码处理)。 进一步优化建议示例代码: 为了消除重复代码,建议重构如下: // 在 CommonTools 类中添加私有辅助函数声明
// QString readHexDumpOutput(const QString &filePath, const QStringList &extraArgs);
QString CommonTools::readHexDumpOutput(const QString &filePath, const QStringList &extraArgs) {
QProcess process;
QStringList args;
args << extraArgs << filePath;
process.start("hexdump", args);
if (!process.waitForFinished(3000)) {
qWarning() << "Hexdump timed out or failed for file:" << filePath;
process.kill();
return QString();
}
QByteArray output = process.readAllStandardOutput();
if (output.isEmpty()) {
qWarning() << "Hexdump output is empty for file:" << filePath;
return QString();
}
return QString::fromLocal8Bit(output);
}
// 在 parseEDID 中调用
void CommonTools::parseEDID(...) {
// ...
for (auto edid : allEDIDS) {
// ...
QString deviceInfo;
if (isHW) {
deviceInfo = readHexDumpOutput(edid, QStringList()); // 不带 -C
} else {
deviceInfo = readHexDumpOutput(edid, QStringList() << "-C"); // 带 -C
}
if (deviceInfo.isEmpty()) {
continue;
}
// 后续解析逻辑...
}
} |
|
Note
详情{
"deepin-devicemanager/src/Tool/commontools.cpp": [
{
"line": " return \"https://driver.uniontech.com/api/v1/drive/search\";",
"line_number": 165,
"rule": "S35",
"reason": "Url link | 162cef9d4a"
},
{
"line": " return \"https://driver.uniontech.com/api/v1/drive/search\";",
"line_number": 169,
"rule": "S35",
"reason": "Url link | 162cef9d4a"
},
{
"line": " return \"https://drive-pre.uniontech.com/api/v1/drive/search\";",
"line_number": 171,
"rule": "S35",
"reason": "Url link | 0e010283c1"
}
]
} |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, max-lvs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/merge |
|
/forcemerge |
|
This pr cannot be merged! (status: unstable) |
|
This pr force merged! (status: unstable) |
894f7e2
into
linuxdeepin:develop/eagle
Log: fix issue
Bug: https://pms.uniontech.com/bug-view-353603.html
Summary by Sourcery
Improve EDID parsing by modernizing QProcess usage and adding robustness around hexdump output handling.
Bug Fixes:
Enhancements: