Repository navigation
fix(dfm-io): prevent readlink blocking on GVFS paths in resolveSymlink - #409
Conversation
Add DFMUtils::isGvfsFile() guard in DLocalHelper::resolveSymlink() to stop symlink resolution when the target is a GVFS (remote) path, avoiding synchronous readlink() calls that block on network I/O. Also extend isGvfsFile() regex to match /media/ smbmounts paths in addition to /run/media/.
Reviewer's guide (collapsed on small PRs)Reviewer's GuidePrevents resolveSymlink from issuing further synchronous readlink/stat operations after reaching a GVFS-backed target, while expanding GVFS detection to recognize SMB mounts under both /run/media and /media. Flow diagram for GVFS-safe symlink resolutionflowchart TD
A[resolveSymlink] --> B[symlinkTarget]
B --> C{isGvfsFile}
C -->|yes| D[Return remote target]
C -->|no| E{nextTarget exists}
E -->|no| F[Return target]
E -->|yes| G{isGvfsFile nextTarget}
G -->|yes| D
G -->|no| H[Continue symlink resolution]
H --> I{Cycle detected}
I -->|yes| J[Return empty]
I -->|no| B
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 found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/dfm-io/dfm-io/utils/dlocalhelper.cpp" line_range="1287-1288" />
<code_context>
{
QSet<QString> visited;
QString target = DLocalHelper::symlinkTarget(url);
+ if (DFMUtils::isGvfsFile(QUrl::fromLocalFile(target)))
+ return target; // Remote target: stop resolving, further readlink/stat would block on network I/O
while (!target.isEmpty()) {
if (visited.contains(target))
</code_context>
<issue_to_address>
**Remote directory links appear as files**
When a symlink resolves to a directory under a GVFS path, `createSortFileInfo` skips `stat()` for the returned target and leaves `isDir` false, so `insertSortFileInfoList` treats the directory symlink as a file and directory-first ordering and filters misclassify it.
Set the directory/file classification for GVFS targets before returning them from either target check.
Also at `src/dfm-io/dfm-io/utils/dlocalhelper.cpp:1297-1298`.
</issue_to_address>| if (DFMUtils::isGvfsFile(QUrl::fromLocalFile(target))) | ||
| return target; // Remote target: stop resolving, further readlink/stat would block on network I/O |
There was a problem hiding this comment.
🟡 Medium · Remote directory links appear as files
When a symlink resolves to a directory under a GVFS path, createSortFileInfo skips stat() for the returned target and leaves isDir false, so insertSortFileInfoList treats the directory symlink as a file and directory-first ordering and filters misclassify it.
Set the directory/file classification for GVFS targets before returning them from either target check.
Also at src/dfm-io/dfm-io/utils/dlocalhelper.cpp:1297-1298.
Prompt for AI agents
In `src/dfm-io/dfm-io/utils/dlocalhelper.cpp` at lines 1287-1288:
**Remote directory links appear as files**
When a symlink resolves to a directory under a GVFS path, `createSortFileInfo` skips `stat()` for the returned target and leaves `isDir` false, so `insertSortFileInfoList` treats the directory symlink as a file and directory-first ordering and filters misclassify it.
Set the directory/file classification for GVFS targets before returning them from either target check.
Also at `src/dfm-io/dfm-io/utils/dlocalhelper.cpp:1297-1298`.
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 代码语法正确,逻辑清晰。正则表达式修改正确,(?:run/)? 非捕获组语法有效。边界条件处理完善:空 target 安全通过(正则不匹配空字符串,while 循环条件捕获),环检测逻辑不受影响。 2. 代码质量 ✅评价: 优秀 ✅ 通过 潜在问题:
建议: 建议将 static const QString *gvfsMatch = new QString{...} 改为 static const QString gvfsMatch{...}(栈分配静态变量),避免堆内存泄漏。同时建议将 QRegularExpression 也设为 static,避免每次调用重新编译正则。 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 本次修改净性能提升显著:通过在 resolveSymlink 中添加 GVFS 守卫,避免了在远程路径上调用阻塞式 readlink(),isGvfsFile() 的正则匹配开销远小于网络 I/O 阻塞。 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: 存在0个安全漏洞。本次修改未引入安全风险,无用户输入处理变更,无硬编码密钥,无注入风险。 💡 改进建议代码示例// 建议优化 isGvfsFile() 中的静态变量声明
bool DFMUtils::isGvfsFile(const QUrl &url)
{
if (!url.isValid())
return false;
const QString &path = url.toLocalFile();
// 使用栈分配的静态变量,避免堆内存泄漏
static const QString gvfsMatch = QStringLiteral("(^/run/user/\\d+/gvfs/|^/root/.gvfs/|^/(?:run/)?media/[\\s\\S]*/smbmounts)");
// 将 QRegularExpression 也设为 static,避免每次调用重新编译
static const QRegularExpression re(gvfsMatch);
QRegularExpressionMatch match = re.match(path);
return match.hasMatch();
}本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: liyigang1, 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 |
|
/forcemerge |
1 similar comment
|
/forcemerge |
7d87834
into
linuxdeepin:develop/meagle-20260526
|
This pr force merged! (status: unknown) |
fix(dfm-io): prevent readlink blocking on GVFS paths in resolveSymlink
Add DFMUtils::isGvfsFile() guard in DLocalHelper::resolveSymlink() to
stop symlink resolution when the target is a GVFS (remote) path,
avoiding synchronous readlink() calls that block on network I/O.
Also extend isGvfsFile() regex to match /media/ smbmounts paths in
addition to /run/media/.
Summary by Sourcery
Avoid blocking symlink resolution for GVFS paths and broaden detection of remote SMB mounts.
Bug Fixes: