Repository navigation
fix(data-transfer): fix moveFile failure when source and target are on different filesystems - #787
Merged
deepin-bot[bot] merged 1 commit intoSep 8, 2026
Conversation
…n different filesystems - Move directories with QDir::rename instead of QFile::rename, which fails with EISDIR on directories - Fall back to recursive copy plus source removal when rename fails with EXDEV across filesystems: copyDirectory() for directories and QFile::copy for single files - Fix rename-collision handling: dst.remove(fileName) deleted every occurrence of the name and corrupted paths with repeated segments; only the trailing segment is cut now - Ensure the destination parent directory exists before moving, for targets such as ~/D:/ derived from Windows drive letters 修复(数据迁移): 修复源和目标位于不同文件系统时 moveFile 搬运失败的问题 - 目录改用 QDir::rename 移动，避免 QFile::rename 移动目录时报 EISDIR 错误 - rename 跨文件系统失败(EXDEV)时退化为复制后删除源：目录走 copyDirectory() 递归复制，单文件走 QFile::copy - 修复重名避让逻辑: dst.remove(fileName) 会误删路径中所有同名段导致路径破坏，改为只截取结尾段 - 移动前确保目标父目录存在，兼容 ~/D:/ 这类源自 Windows 盘符的目标路径 Log: 修复数据迁移搬运阶段接收目录与主目录不在同一文件系统时 rename 跨设备失败(EXDEV)导致搬运失败的问题 Bug: https://pms.uniontech.com/bug-view-375881.html
Reviewer's GuideUpdates moveFile to correctly move directories and files across different filesystems via copy-and-delete fallbacks, while fixing path truncation in rename avoidance and ensuring destination directories exist. Sequence diagram for cross-filesystem file and directory movessequenceDiagram
participant Caller
participant SettingHelper
participant Filesystem
Caller->>SettingHelper: moveFile(src, dst)
SettingHelper->>Filesystem: QDir().mkpath(destination parent)
alt source is directory
SettingHelper->>Filesystem: QDir::rename(src, dst)
alt rename succeeds
Filesystem-->>SettingHelper: success
else rename fails across filesystems
SettingHelper->>Filesystem: copyDirectory(src, dst)
SettingHelper->>Filesystem: QDir(src).removeRecursively()
Filesystem-->>SettingHelper: success
end
else source is file
SettingHelper->>Filesystem: QFile::rename(dst)
alt rename succeeds
Filesystem-->>SettingHelper: success
else rename fails across filesystems
SettingHelper->>Filesystem: QFile::copy(src, dst)
SettingHelper->>Filesystem: QFile::remove(src)
Filesystem-->>SettingHelper: success
end
end
SettingHelper-->>Caller: true or false
Flow diagram for cross-filesystem moveFile fallbackflowchart TD
A["moveFile(src, dst)"] --> B["QDir().mkpath(destination parent)"]
B --> C{Source is directory?}
C -->|Yes| D["QDir::rename(src, dst)"]
D -->|Success| E[Return true]
D -->|Failure| F["copyDirectory(src, dst)"]
F -->|Success| G["QDir(src).removeRecursively()"]
G --> E
F -->|Failure| H[Return false]
C -->|No| I["QFile::rename(dst)"]
I -->|Success| E
I -->|Failure| J["QFile::copy(src, dst)"]
J -->|Success| K["QFile::remove(src)"]
K --> E
J -->|Failure| H
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 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/lib/data-transfer/core/utils/settinghepler.cpp" line_range="392-406" />
<code_context>
+ if (dir.rename(src, dst))
+ return true;
+ if (copyDirectory(src, dst)) {
+ QDir(src).removeRecursively(); // 数据已就位, 源清理失败不影响结果
+ return true;
+ }
+
+ WLOG << "moveFile error: rename dir failed: " << src.toStdString() << " -> " << dst.toStdString();
+ return false;
+ }
+ // ---------- 文件处理 ----------
+ // 优先 rename 移动; 跨设备时退化为复制后删除源
QFile f(src);
LOG << "moveFile dst: " << src.toStdString() << " " << dst.toStdString();
if (f.rename(dst))
return true;
+ if (QFile::copy(src, dst)) { // 跨设备时退化为复制后删除源
+ QFile::remove(src);
+ return true;
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** After a cross-filesystem copy succeeds, failure to delete the source is ignored and `moveFile` returns true, so the same data remains in both locations even though callers are told the move completed. This applies to both directory cleanup at `ck` and file cleanup at `cy`.
**Triggers:** When the source cannot be removed because of permissions, read-only media, or a concurrent filesystem change.
**Suggested fix:** Check the removal result and return failure, or report the operation as a copy rather than a completed move when cleanup fails.
</issue_to_address>
### Comment 2
<location path="src/lib/data-transfer/core/utils/settinghepler.cpp" line_range="345-356" />
<code_context>
+ if (!QDir().mkpath(dst))
+ return false;
+
+ const auto entries = srcDir.entryInfoList(
+ QDir::Files | QDir::Dirs | QDir::NoDotAndDotDot | QDir::Hidden | QDir::System);
+ for (const auto &entry : entries) {
+ const QString target = dst + QLatin1Char('/') + entry.fileName();
+ if (entry.isDir()) {
+ if (!copyDirectory(entry.absoluteFilePath(), target))
+ return false;
+ } else if (!QFile::copy(entry.absoluteFilePath(), target)) {
+ WLOG << "copy file failed: " << entry.absoluteFilePath().toStdString()
+ << " -> " << target.toStdString();
+ return false;
+ }
+ }
+ return true;
</code_context>
<issue_to_address>
**issue (bug_risk):** If copying any entry fails, `copyDirectory` returns false after creating the destination and copying earlier entries, leaving a partial destination tree while the source remains. A retry then treats that partial tree as an existing destination and applies rename-conflict logic, producing additional suffixed directories instead of retrying cleanly.
**Triggers:** When a directory contains an unreadable file, unsupported entry, or a copy error after at least one entry has already been transferred.
**Suggested fix:** Remove the partial destination tree before returning failure, or copy into a temporary destination and rename it into place only after the complete copy succeeds.
</issue_to_address>
### Comment 3
<location path="src/lib/data-transfer/core/utils/settinghepler.cpp" line_range="347-350" />
<code_context>
+ for (const auto &entry : entries) {
+ const QString target = dst + QLatin1Char('/') + entry.fileName();
+ if (entry.isDir()) {
+ if (!copyDirectory(entry.absoluteFilePath(), target))
+ return false;
+ } else if (!QFile::copy(entry.absoluteFilePath(), target)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** A directory symlink is classified as a directory and recursively passed to `copyDirectory`; because the recursion follows the symlink and does not track visited paths, a symlink to an ancestor causes unbounded recursive copying until path/resource exhaustion instead of preserving or rejecting the symlink.
**Triggers:** When the source directory contains a symlink to a directory, especially a symlink pointing to the source or one of its ancestors.
**Suggested fix:** Handle `entry.isSymLink()` before `entry.isDir()` and either copy the link itself or explicitly reject symlinks during fallback copying.
```suggestion
for (const auto &entry : entries) {
const QString target = dst + QLatin1Char('/') + entry.fileName();
if (entry.isSymLink())
return false;
if (entry.isDir()) {
if (!copyDirectory(entry.absoluteFilePath(), target))
```
</issue_to_address>
Comment on lines
+392
to
+406
| QDir(src).removeRecursively(); // 数据已就位, 源清理失败不影响结果 | ||
| return true; | ||
| } | ||
|
|
||
| WLOG << "moveFile error: rename dir failed: " << src.toStdString() << " -> " << dst.toStdString(); | ||
| return false; | ||
| } | ||
| // ---------- 文件处理 ---------- | ||
| // 优先 rename 移动; 跨设备时退化为复制后删除源 | ||
| QFile f(src); | ||
| LOG << "moveFile dst: " << src.toStdString() << " " << dst.toStdString(); | ||
| if (f.rename(dst)) | ||
| return true; | ||
| if (QFile::copy(src, dst)) { // 跨设备时退化为复制后删除源 | ||
| QFile::remove(src); |
There was a problem hiding this comment.
issue (bug_risk): After a cross-filesystem copy succeeds, failure to delete the source is ignored and moveFile returns true, so the same data remains in both locations even though callers are told the move completed. This applies to both directory cleanup at ck and file cleanup at cy.
Triggers: When the source cannot be removed because of permissions, read-only media, or a concurrent filesystem change.
Suggested fix: Check the removal result and return failure, or report the operation as a copy rather than a completed move when cleanup fails.
Comment on lines
+345
to
+356
| const auto entries = srcDir.entryInfoList( | ||
| QDir::Files | QDir::Dirs | QDir::NoDotAndDotDot | QDir::Hidden | QDir::System); | ||
| for (const auto &entry : entries) { | ||
| const QString target = dst + QLatin1Char('/') + entry.fileName(); | ||
| if (entry.isDir()) { | ||
| if (!copyDirectory(entry.absoluteFilePath(), target)) | ||
| return false; | ||
| } else if (!QFile::copy(entry.absoluteFilePath(), target)) { | ||
| WLOG << "copy file failed: " << entry.absoluteFilePath().toStdString() | ||
| << " -> " << target.toStdString(); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): If copying any entry fails, copyDirectory returns false after creating the destination and copying earlier entries, leaving a partial destination tree while the source remains. A retry then treats that partial tree as an existing destination and applies rename-conflict logic, producing additional suffixed directories instead of retrying cleanly.
Triggers: When a directory contains an unreadable file, unsupported entry, or a copy error after at least one entry has already been transferred.
Suggested fix: Remove the partial destination tree before returning failure, or copy into a temporary destination and rename it into place only after the complete copy succeeds.
Comment on lines
+347
to
+350
| for (const auto &entry : entries) { | ||
| const QString target = dst + QLatin1Char('/') + entry.fileName(); | ||
| if (entry.isDir()) { | ||
| if (!copyDirectory(entry.absoluteFilePath(), target)) |
There was a problem hiding this comment.
issue (bug_risk): A directory symlink is classified as a directory and recursively passed to copyDirectory; because the recursion follows the symlink and does not track visited paths, a symlink to an ancestor causes unbounded recursive copying until path/resource exhaustion instead of preserving or rejecting the symlink.
Triggers: When the source directory contains a symlink to a directory, especially a symlink pointing to the source or one of its ancestors.
Suggested fix: Handle entry.isSymLink() before entry.isDir() and either copy the link itself or explicitly reject symlinks during fallback copying.
Suggested change
| for (const auto &entry : entries) { | |
| const QString target = dst + QLatin1Char('/') + entry.fileName(); | |
| if (entry.isDir()) { | |
| if (!copyDirectory(entry.absoluteFilePath(), target)) | |
| for (const auto &entry : entries) { | |
| const QString target = dst + QLatin1Char('/') + entry.fileName(); | |
| if (entry.isSymLink()) | |
| return false; | |
| if (entry.isDir()) { | |
| if (!copyDirectory(entry.absoluteFilePath(), target)) |
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 良好 ✅ 通过 潜在问题：
建议： 建议使用 dstDir.endsWith('/') ? dstDir + baseName : dstDir + '/' + baseName 的方式拼接路径，或使用 QDir::toNativeSeparators 规范化路径 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 建议在 copyDirectory 失败后，清理目标目录中的部分复制数据（QDir(dst).removeRecursively()），或在函数注释中明确说明失败时的数据状态 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 建议在 copyDirectory 中添加 symlink 检测（entry.isSymLink()），对符号链接直接复制链接本身或跳过；如需保留文件属性，可考虑使用系统调用或 QFileInfo 获取并恢复权限 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 建议增加递归深度限制（如 maxDepth 参数），或检测符号链接后跳过/特殊处理 💡 改进建议代码示例// 修复路径拼接双斜杠问题
QString sep = dstDir.endsWith('/') ? "" : "/";
dst = dstDir + sep + baseName + "(" + QString::number(i) + ")" + suffix;
// copyDirectory 增加符号链接防护和递归深度限制
static bool copyDirectory(const QString &src, const QString &dst, int depth = 0)
{
if (depth > 100) // 防止无限递归
return false;
QDir srcDir(src);
if (!srcDir.exists())
return false;
if (!QDir().mkpath(dst))
return false;
const auto entries = srcDir.entryInfoList(
QDir::Files | QDir::Dirs | QDir::NoDotAndDotDot | QDir::Hidden | QDir::System);
for (const auto &entry : entries) {
if (entry.isSymLink()) // 跳过符号链接，避免循环
continue;
const QString target = dst + QLatin1Char('/') + entry.fileName();
if (entry.isDir()) {
if (!copyDirectory(entry.absoluteFilePath(), target, depth + 1))
return false;
} else if (!QFile::copy(entry.absoluteFilePath(), target)) {
WLOG << "copy file failed: " << entry.absoluteFilePath().toStdString()
<< " -> " << target.toStdString();
return false;
}
}
return true;
}本报告由 AI 代码审查工具自动生成 |
re2zero
approved these changes
Sep 8, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pppanghu77, re2zero 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 |
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.
问题
数据迁移搬运阶段失败，日志报：
根因：接收目录（~/Downloads）与主目录可能不在同一文件系统（例如接收目录是 fuse 挂载、主目录是 ext4）。
rename(2)不允许跨文件系统，返回 EXDEV（Errno 18），搬运必然失败；且移动目录时QFile::rename会直接报 EISDIR。修复内容（moveFile）
QDir::rename移动：QFile::rename移动目录报 EISDIR（"打开的文件是个目录"）copyDirectory()递归复制（含隐藏文件）→removeRecursively()删源QFile::copy→QFile::removedst.remove(fileName)会删除路径中所有出现的同名段（QString::remove是全局删除），路径含重复段时被截断成错误路径；改为只截结尾段~/D:这类源自 Windows 盘符的目标目录，mkpath 确保存在验证
stat -c %d设备号不同）三场景实测通过：~/D:路径（含子目录、隐藏文件、无后缀文件）os.rename实测复现 Errno 18 EXDEV 确认根因Bug: https://pms.uniontech.com//bug-view-375881.html
Summary by Sourcery
Make data-transfer file moves reliable across filesystem boundaries while preserving destination path and naming behavior.
Bug Fixes:
Enhancements: