fix: optimize handleLongNameExtract by batching extraction#416
Merged
deepin-bot[bot] merged 1 commit intoMay 22, 2026
Conversation
Reviewer's GuideRefactors handleLongNameExtract to decouple long‑name renaming from extraction, batching all 7z extract calls into a single process while preserving password handling, directory creation, and long-name error logic. Sequence diagram for batched handleLongNameExtract extractionsequenceDiagram
participant CliInterface
participant KPtyProcess
participant moveProgram
participant extractProgram
participant PasswordNeededQuery
CliInterface->>CliInterface: handleLongNameExtract(files)
CliInterface->>CliInterface: classify files into renameEntries, normalFileList
loop for each entry in renameEntries
CliInterface->>KPtyProcess: setProgram(moveProgram, moveArgs(...))
CliInterface->>KPtyProcess: start()
KPtyProcess->>moveProgram: 7z rn
KPtyProcess-->>CliInterface: waitForFinished(-1)
end
CliInterface->>CliInterface: build allFileList from renameEntries and normalFileList
CliInterface->>KPtyProcess: setProgram(extractProgram, extractArgs(allFileList,...))
CliInterface->>KPtyProcess: start()
KPtyProcess->>extractProgram: 7z x (batch)
alt password is empty
loop until finished
KPtyProcess-->>CliInterface: output
CliInterface->>CliInterface: isPasswordPrompt(line)
alt isPasswordPrompt
CliInterface->>KPtyProcess: kill()
KPtyProcess-->>CliInterface: waitForFinished(3000)
CliInterface->>PasswordNeededQuery: PasswordNeededQuery(m_strArchiveName)
CliInterface-->>PasswordNeededQuery: signalQuery(&query)
PasswordNeededQuery-->>CliInterface: waitForResponse()
alt user cancelled
CliInterface->>CliInterface: set m_eErrorType = ET_NeedPassword
CliInterface-->>KPtyProcess: deleteLater()
CliInterface-->>CliInterface: return false
else user entered password
CliInterface->>CliInterface: update archiveData.strPassword
CliInterface->>KPtyProcess: setProgram(extractProgram, extractArgs(allFileList,..., password))
CliInterface->>KPtyProcess: start()
KPtyProcess-->>CliInterface: waitForFinished(-1)
alt exitCode != 0 and output contains Wrong password
CliInterface->>CliInterface: set m_eErrorType = ET_WrongPassword
CliInterface-->>KPtyProcess: deleteLater()
CliInterface-->>CliInterface: return false
end
end
end
end
else password provided
KPtyProcess-->>CliInterface: waitForFinished(-1)
end
CliInterface-->>KPtyProcess: deleteLater()
CliInterface-->>CliInterface: return true
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="3rdparty/interface/archiveinterface/cliinterface.cpp" line_range="1334-1321" />
<code_context>
- }
+ // 第二步：对需要重命名的文件逐个执行 7z rn
+ qInfo() << "handleLongNameExtract: rename entries:" << renameEntries.count() << ", normal files:" << normalFileList.count();
+ for (const FileEntry &entry : renameEntries) {
+ qInfo() << "handleLongNameExtract: rename" << entry.strFullPath << "->" << entry.strAlias;
+ QList<FileEntry> lstFile;
+ lstFile.append(entry);
+ pProcess->setPtyChannels(KPtyProcess::StdinChannel);
+ pProcess->setOutputChannelMode(KProcess::MergedChannels);
+ pProcess->setNextOpenMode(QIODevice::ReadWrite | QIODevice::Unbuffered | QIODevice::Text);
+ pProcess->setProgram(m_cliProps->property("moveProgram").toString(), m_cliProps->moveArgs(absoluteDestinationPath, lstFile, DataManager::get_instance().archiveData(), password));
+ pProcess->start();
+ pProcess->waitForFinished(-1);
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider handling errors from the rename (moveProgram) step, including exit status and potential password issues.
This loop waits for `moveProgram` to finish but never checks `exitCode()` or its output. If a rename fails (invalid path, archive issues, or a password-required `rn`), the code still proceeds and the bulk extract may misbehave. Please inspect the process result here, reuse the password‑prompt handling if applicable, and set an appropriate `m_eErrorType` instead of continuing silently.
Suggested implementation:
```cpp
for (const FileEntry &entry : renameEntries) {
qInfo() << "handleLongNameExtract: rename" << entry.strFullPath << "->" << entry.strAlias;
QList<FileEntry> lstFile;
lstFile.append(entry);
pProcess->setPtyChannels(KPtyProcess::StdinChannel);
pProcess->setOutputChannelMode(KProcess::MergedChannels);
pProcess->setNextOpenMode(QIODevice::ReadWrite | QIODevice::Unbuffered | QIODevice::Text);
pProcess->setProgram(
m_cliProps->property("moveProgram").toString(),
m_cliProps->moveArgs(
absoluteDestinationPath,
lstFile,
DataManager::get_instance().archiveData(),
password));
pProcess->start();
// Wait for the rename process and inspect its result; abort on any failure so we don't continue silently.
if (!pProcess->waitForFinished(-1)) {
qWarning() << "handleLongNameExtract: moveProgram did not finish for"
<< entry.strFullPath
<< "error:" << pProcess->errorString();
return;
}
if (pProcess->exitStatus() != QProcess::NormalExit || pProcess->exitCode() != 0) {
const QByteArray procOutput = pProcess->readAll();
qWarning() << "handleLongNameExtract: moveProgram failed for"
<< entry.strFullPath
<< "exitStatus:" << pProcess->exitStatus()
<< "exitCode:" << pProcess->exitCode()
<< "output:" << procOutput;
return;
}
}
```
To fully align with your review comment and the rest of the codebase, the following adjustments are likely needed elsewhere in `cliinterface.cpp`:
1. **Password‑prompt reuse:**
- Identify the existing logic that handles password prompts / wrong password for other 7z invocations (e.g. extract or list).
- Before treating a non‑zero `exitCode()` as fatal, reuse that same handler for the `moveProgram` call, so cases like “password required for `rn`” are mapped to the same `m_eErrorType` and user interaction flow.
2. **Error type wiring (`m_eErrorType`):**
- Locate how `m_eErrorType` is set for other 7z failures (invalid path, archive issues, password errors).
- In the two `return` paths added above, set `m_eErrorType` (and any associated error message field) consistently with those existing patterns instead of just returning. For example, map a non‑zero exit code from `moveProgram` to the same error type used for extract failures.
3. **Includes (if needed):**
- If `QProcess` isn’t already included in this translation unit, add `#include <QProcess>` to the top of the file so `QProcess::NormalExit` compiles.
</issue_to_address>
### Comment 2
<location path="3rdparty/interface/archiveinterface/cliinterface.cpp" line_range="1357" />
<code_context>
+ }
+ }
+ allFileList.append(normalFileList);
+ qInfo() << "handleLongNameExtract: batch extract files:" << allFileList.count();
+
+ if (!allFileList.isEmpty()) {
</code_context>
<issue_to_address>
**suggestion:** Batch extraction with a potentially very large allFileList may hit command-line length limits.
This now builds a single `allFileList` and passes it to `extractArgs` once. For archives with many entries, the resulting command line may exceed OS length limits (particularly on Windows), whereas the previous per-file extraction avoided this. Consider chunking `allFileList` into safely sized batches or adding a fallback if the command line would be too long.
Suggested implementation:
```cpp
QStringList allFileList;
for (const FileEntry &entry : renameEntries) {
QFileInfo info(entry.strFullPath);
if (info.path() != ".") {
allFileList.append(info.path() + QDir::separator() + entry.strAlias);
} else {
allFileList.append(entry.strAlias);
}
}
allFileList.append(normalFileList);
// Estimate total command-line length contribution of file arguments to avoid OS limits (especially on Windows).
// This is conservative and counts path characters plus a separator per file.
qsizetype estimatedArgsLength = 0;
for (const QString &filePath : std::as_const(allFileList)) {
estimatedArgsLength += filePath.size() + 1; // +1 for space / separator
}
// Choose a safe limit well below typical Windows command-line limits (~32K), accounting for program/other args.
constexpr qsizetype kMaxSafeCmdArgLength = 24000;
QVector<QStringList> fileBatches;
if (estimatedArgsLength > kMaxSafeCmdArgLength) {
qInfo() << "handleLongNameExtract: total argument length" << estimatedArgsLength
<< "exceeds safe limit" << kMaxSafeCmdArgLength << "- chunking extraction.";
QStringList currentBatch;
qsizetype currentBatchLen = 0;
for (const QString &filePath : std::as_const(allFileList)) {
const qsizetype fileLen = filePath.size() + 1; // +1 for separator
// If adding this file would exceed the safe limit, start a new batch.
if (!currentBatch.isEmpty() && currentBatchLen + fileLen > kMaxSafeCmdArgLength) {
fileBatches.append(currentBatch);
currentBatch.clear();
currentBatchLen = 0;
}
currentBatch.append(filePath);
currentBatchLen += fileLen;
}
if (!currentBatch.isEmpty()) {
fileBatches.append(currentBatch);
}
qInfo() << "handleLongNameExtract: created" << fileBatches.size()
<< "batches for" << allFileList.count() << "files.";
} else {
// Single batch with all files is safely within command-line limits.
fileBatches.append(allFileList);
qInfo() << "handleLongNameExtract: batch extract files:" << allFileList.count();
}
if (!fileBatches.isEmpty()) {
// For now, start the first batch here. The surrounding logic should be extended to
// process remaining batches sequentially, reusing pProcess.
const QStringList firstBatch = fileBatches.first();
QDir::setCurrent(options.strTargetPath);
pProcess->setPtyChannels(KPtyProcess::StdinChannel);
pProcess->setOutputChannelMode(KProcess::MergedChannels);
pProcess->setNextOpenMode(QIODevice::ReadWrite | QIODevice::Unbuffered | QIODevice::Text);
pProcess->setProgram(m_cliProps->property("extractProgram").toString(),
m_cliProps->extractArgs(absoluteDestinationPath, firstBatch, false, password));
pProcess->start();
if (password.isEmpty()) {
bool bPasswordEntered = false;
```
To fully implement multi-batch extraction and avoid command-line length limits end-to-end, you should:
1. Extend the logic around this snippet to:
- Keep `fileBatches` (or an index into it) as state for the ongoing extraction.
- When the current `pProcess` finishes successfully, start the next batch with a new call to `setProgram(...)`/`start()`, reusing the same `pProcess`.
2. Ensure that error handling and progress reporting account for multiple batches:
- If any batch fails, abort remaining batches and surface the error.
- Progress UI (if any) should reflect progress across all batches, not just the first.
3. If this function currently assumes a single `pProcess` run per invocation, you may need to:
- Store `fileBatches` in a member of the owning class, and
- Trigger subsequent batches from the `finished`/`errorOccurred` slots connected to `pProcess`.
This will preserve the new batch behavior while safely handling very large archives by splitting them into multiple, OS-safe command-line invocations.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Instead of running separate 7z rn + 7z x for every file, only rename files that actually need it and do a single batch extraction for all files at once, reducing process launches from N*2 to M+1. log: fix bug Bug: https://pms.uniontech.com/bug-view-362685.html
lzwind
approved these changes
May 22, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LiHua000, lzwind 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 |
Contributor
Author
|
/merge |
deepin pr auto review这份代码的重构主要将原本“遍历文件时逐个启动进程进行移动和解压”的逻辑，优化为了“先统一处理重命名（7z rn），再批量解压”的逻辑。这大大减少了进程的创建和销毁次数，是一个很好的性能优化方向。 但在仔细审查后，发现在语法逻辑、代码性能和代码安全方面仍有一些关键问题需要修正。以下是详细的审查意见： 一、 语法与逻辑错误 (严重)1. 进程重复启动导致状态混乱
2. 密码检测逻辑中的变量遮蔽 QByteArray output = pProcess->readAllStandardOutput();
// ...
if (pProcess->exitCode() != 0) {
QByteArray output = pProcess->readAllStandardOutput(); // 内部遮蔽了外部的 output
if (output.contains("Wrong password")) {
3.
二、 代码性能1. for (const FileEntry &entry : renameEntries) {
QList<FileEntry> lstFile;
lstFile.append(entry);
// ... 启动进程
}
2. 字符串编码转换的重复计算 bool needRename = (NAME_MAX < QString(info.fileName()).toLocal8Bit().length() && ...);
三、 代码安全1. 临时文件/压缩包的拷贝未做安全校验 QFile tmpFile(absoluteDestinationPath);
if (tmpFile.exists()) tmpFile.remove();
QFile::copy(m_strArchiveName, absoluteDestinationPath);
2. pProcess->waitForFinished(-1);
3.
四、 代码质量与可读性1. 魔法字符串 if (output.contains("Wrong password")) {
2. 注释与代码逻辑的统一 改进后的代码建议综合以上意见，以下是改进后的代码参考： bool CliInterface::handleLongNameExtract(const QList<FileEntry> &files)
{
ExtractionOptions &options = m_extractOptions;
QString password;
if (options.password.isEmpty()) {
password = DataManager::get_instance().archiveData().isListEncrypted ? DataManager::get_instance().archiveData().strPassword : QString();
} else {
password = options.password;
}
QFile tmpFile(absoluteDestinationPath);
if (tmpFile.exists()) {
if (!tmpFile.remove()) {
m_eErrorType = ET_FileRemoveError; // 建议增加错误类型
return false;
}
}
if (!QFile::copy(m_strArchiveName, absoluteDestinationPath)) {
m_eErrorType = ET_FileCopyError; // 建议增加错误类型
return false;
}
QList<FileEntry> renameEntries;
QStringList normalFileList;
qInfo() << "handleLongNameExtract: total files:" << files.count();
// 第一遍：遍历所有文件，分离需要重命名的文件和普通文件，同时创建目录结构
for (const FileEntry &entry : files) {
QFileInfo info(entry.strFullPath);
QString strFilePath = info.path();
QString strFileName = entry.strFullPath;
if (strFilePath != ".") {
QString sDir = strFilePath + QDir::separator();
if (!strFileName.startsWith(sDir)) {
strFileName = sDir + info.fileName();
}
}
bool needRename = (NAME_MAX < info.fileName().toLocal8Bit().length() && !entry.strFullPath.endsWith(QDir::separator()));
if (needRename) {
QString strTemp = info.fileName().left(TRUNCATION_FILE_LONG);
QString tempFilePathName = strFilePath + QDir::separator() + strTemp;
if (m_mapLongName[tempFilePathName]++ >= LONGFILE_SAME_FILES) {
emit signalCurFileName(entry.strFullPath);
m_eErrorType = ET_LongNameError;
return false;
}
// 生成截断后的别名
QString strTempFileName = strTemp + QString("_%1").arg(m_mapLongName[tempFilePathName]);
if (strFilePath != ".") {
strFileName = strFilePath + QDir::separator() + strTempFileName;
} else {
strFileName = strTempFileName;
}
}
QString strDestFileName = options.strTargetPath + QDir::separator() + strFileName;
if (!m_extractOptions.bAllExtract) {
int nCnt = m_rootNode.count(QDir::separator());
QString destFileName = strFileName;
for (int i = 0; i < nCnt; i++) {
destFileName = destFileName.mid(destFileName.indexOf(QDir::separator()) + 1);
}
strDestFileName = options.strTargetPath + QDir::separator() + destFileName;
}
if (entry.strFullPath.endsWith(QDir::separator())) {
if (!QDir(strDestFileName).exists()) {
if (!QDir(strDestFileName).mkpath(strDestFileName)) {
return false;
}
}
} else {
FileEntry newEntry = entry;
newEntry.strAlias = QFileInfo(strFileName).fileName();
if (needRename) {
renameEntries.append(newEntry);
} else {
if (info.path() != ".") {
normalFileList.append(info.path() + QDir::separator() + newEntry.strAlias);
} else {
normalFileList.append(newEntry.strAlias);
}
}
}
}
// 第二步：对需要重命名的文件逐个执行 moveProgram (如 7z rn)
qInfo() << "handleLongNameExtract: rename entries:" << renameEntries.count() << ", normal files:" << normalFileList.count();
for (const FileEntry &entry : renameEntries) {
qInfo() << "handleLongNameExtract: rename" << entry.strFullPath << "->" << entry.strAlias;
// 每次重命名使用独立的进程，避免状态混乱
KPtyProcess renameProcess;
QList<FileEntry> lstFile;
lstFile.append(entry);
renameProcess.setPtyChannels(KPtyProcess::StdinChannel);
renameProcess.setOutputChannelMode(KProcess::MergedChannels);
renameProcess.setNextOpenMode(QIODevice::ReadWrite | QIODevice::Unbuffered | QIODevice::Text);
renameProcess.setProgram(m_cliProps->property("moveProgram").toString(),
m_cliProps->moveArgs(absoluteDestinationPath, lstFile, DataManager::get_instance().archiveData(), password));
renameProcess.start();
// 设置超时时间，避免死锁
if (!renameProcess.waitForFinished(30000)) {
renameProcess.kill();
renameProcess.waitForFinished(3000);
m_eErrorType = ET_ProcessTimeout; // 建议增加错误类型
return false;
}
}
// 第三步：一次性批量解压所有文件
QStringList allFileList;
allFileList.reserve(renameEntries.size() + normalFileList.size()); // 优化内存分配
for (const FileEntry &entry : renameEntries) {
QFileInfo info(entry.strFullPath);
if (info.path() != ".") {
allFileList.append(info.path() + QDir::separator() + entry.strAlias);
} else {
allFileList.append(entry.strAlias);
}
}
allFileList.append(normalFileList);
qInfo() << "handleLongNameExtract: batch extract files:" << allFileList.count();
if (!allFileList.isEmpty()) {
QScopedPointer<KPtyProcess> pProcess(new KPtyProcess);
pProcess->setWorkingDirectory(options.strTargetPath);
pProcess->setPtyChannels(KPtyProcess::StdinChannel);
pProcess->setOutputChannelMode(KProcess::MergedChannels);
pProcess->setNextOpenMode(QIODevice::ReadWrite | QIODevice::Unbuffered | QIODevice::Text);
pProcess->setProgram(m_cliProps->property("extractProgram").toString(),
m_cliProps->extractArgs(absoluteDestinationPath, allFileList, false, password));
pProcess->start();
if (password.isEmpty()) {
bool bPasswordEntered = false;
// 设置超时时间
while (!pProcess->waitForFinished(200)) {
if (pProcess->bytesAvailable() > 0) {
QByteArray output = pProcess->readAllStandardOutput();
QStringList lines = QString::fromLocal8Bit(output).split('\n');
for (const QString &line : lines) {
if (isPasswordPrompt(line)) {
pProcess->kill();
pProcess->waitForFinished(3000);
PasswordNeededQuery query(m_strArchiveName);
emit signalQuery(&query);
query.waitForResponse();
if (query.responseCancelled()) {
m_eErrorType = ET_NeedPassword;
return false;
}
password = query.password();
DataManager::get_instance().archiveData().strPassword = password;
pProcess->setPtyChannels(KPtyProcess::StdinChannel);
pProcess->setOutputChannelMode(KProcess::MergedChannels);
pProcess->setNextOpenMode(QIODevice::ReadWrite | QIODevice::Unbuffered | QIODevice::Text);
pProcess->setProgram(m_cliProps->property("extractProgram").toString(),
m_cliProps->extractArgs(absoluteDestinationPath, allFileList, false, password));
pProcess->start();
pProcess->waitForFinished(-1); // 输入密码后可等待
if (pProcess->exitCode() != 0) {
// 修复变量遮蔽问题，复用外层 output
output = pProcess->readAllStandardOutput();
if (output.contains("Wrong password")) {
m_eErrorType = ET_WrongPassword;
return false;
}
}
bPasswordEntered = true;
break;
}
}
if (bPasswordEntered)
break;
}
}
} else {
// 增加超时保护
if (!pProcess->waitForFinished(60000)) {
pProcess->kill();
pProcess->waitForFinished(3000);
m_eErrorType = ET_ProcessTimeout;
return false;
}
}
}
return true;
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instead of running separate 7z rn + 7z x for every file, only rename
files that actually need it and do a single batch extraction for all
files at once, reducing process launches from N*2 to M+1.
log: fix bug
Bug: https://pms.uniontech.com/bug-view-362685.html
Summary by Sourcery
Optimize long-filename extraction by separating rename handling from extraction and performing a single batch extract.
Bug Fixes:
Enhancements: