fix: isSupportSeek use QTemporaryFile directly instead of close/reopen#443
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors Common::isSupportSeek to use an already-open QTemporaryFile for seek detection instead of closing and reopening it, preventing permission errors on some filesystems. Sequence diagram for updated Common_isSupportSeek temporary file seek detectionsequenceDiagram
participant Common
participant QFileInfo
participant QTemporaryFile
Common->>QFileInfo: absoluteDir
QFileInfo-->>Common: tempDir
Common->>QTemporaryFile: QTemporaryFile(fileTemplate)
Common->>QTemporaryFile: setAutoRemove(true)
Common->>QTemporaryFile: open()
alt tempFile opened
Common->>QTemporaryFile: write("test\n")
Common->>QTemporaryFile: flush()
Common->>QTemporaryFile: seek(0)
alt seek successful
Common->>QTemporaryFile: close()
Common-->>Common: return true
else seek failed
Common->>QTemporaryFile: close()
Common-->>Common: return false
end
else open failed
Common-->>Common: return false
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The logic now calls tempFile.close() both inside the seek-success branch and again after the if block; consider removing the inner close to avoid redundant calls and keep the control flow simpler.
- Since the temporary file is only used to test seek support, you could early-return on tempFile.open() failure as well, which would make the intent clearer and avoid unnecessary work in the non-seekable case.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic now calls tempFile.close() both inside the seek-success branch and again after the if block; consider removing the inner close to avoid redundant calls and keep the control flow simpler.
- Since the temporary file is only used to test seek support, you could early-return on tempFile.open() failure as well, which would make the intent clearer and avoid unnecessary work in the non-seekable case.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Fix permission denied error when reopening temporary file for seek detection on certain filesystems. Log: isSupportSeek use QTemporaryFile directly Bug:https://pms.uniontech.com/bug-view-371275.html
deepin pr auto review★ 总体评分:100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 bool Common::isSupportSeek(QString sFileName)
{
// ... 前置代码 ...
} else {
QString tempDir = info.absoluteDir().path();
QString fileTemplate = tempDir + "/tempfile_XXXXXX";
QTemporaryFile tempFile(fileTemplate);
tempFile.setAutoRemove(true);
if (tempFile.open()) {
tempFile.write("test\n");
tempFile.flush();
if (tempFile.seek(0)) {
tempFile.close();
return true;
}
}
tempFile.close();
}
return false;
} |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LiHua000, 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 |
Fix permission denied error when reopening temporary file for seek detection on certain filesystems.
Log: isSupportSeek use QTemporaryFile directly
Bug:https://pms.uniontech.com/bug-view-371275.html
Summary by Sourcery
Bug Fixes: