Fix: Refactor long file name handling in common interface#323
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:develop/snipefrom Nov 21, 2025
Merged
Conversation
-- Renamed functions and updated logic to improve clarity and consistency in handling long file names across the codebase. The function `isSubpathOfDlnfs` is now `isSubpathOfLnfs`, and `findDlnfsPath` has been changed to `findLnfsPath`. This refactoring enhances the readability of the code and ensures that the correct file system types are checked for long file name support. Log: Refactor long file name handling in common interface Bug: https://pms.uniontech.com/bug-view-326015.html
deepin pr auto review我来分析一下这个diff的修改内容：
代码质量改进建议：
这些修改提高了代码的可维护性和扩展性，但还可以进一步优化以提高代码质量和安全性。 |
Reviewer's GuideThis PR systematically renames DLNFS-related identifiers to LNFS and overhauls long-file-name support checks by switching from a hardcoded source match to filesystem-type detection (fuse.dlnfs or ulnfs), while propagating these changes across the common interface, UI tools, and archive plugins. Sequence diagram for long file name support check in extraction processsequenceDiagram
participant Plugin
participant Common
Plugin->>Common: isSubpathOfLnfs(strTargetPath)
Common->>Common: findLnfsPath(target, func)
Common->>"mnt_table": iterate filesystems
Common->>Common: Check fsType ("fuse.dlnfs" or "ulnfs")
Common-->>Plugin: return true/false
Plugin->>Plugin: Use bLnfs to determine extraction logic
Class diagram for updated long file name handling in Common interfaceclassDiagram
class Common {
+QString handleLongNameforPath(strFilePath, entryName, mapLongDirName, mapRealDirValue)
+bool isSubpathOfLnfs(path)
+bool isSupportSeek(sFileName)
-bool findLnfsPath(target, func)
}
Class diagram for updated long file name support in LibzipPluginclassDiagram
class LibzipPlugin {
QMap<QString, int> m_mapLongDirName
QMap<QString, int> m_mapRealDirValue
QSet<QString> m_setLongName
bool m_bLnfs
}
Class diagram for updated long file name support in CliInterfaceclassDiagram
class CliInterface {
+PluginFinishType extractFiles(files, options, bLnfs)
}
Class diagram for updated local device file check in UiToolsclassDiagram
class UiTools {
+bool isLocalDeviceFile(strFileName)
+QStringList removeSameFileName(listFiles)
}
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 there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `3rdparty/interface/common.cpp:487` </location>
<code_context>
}
-bool Common::findDlnfsPath(const QString &target, Compare func)
+bool Common::findLnfsPath(const QString &target, Compare func)
{
Q_ASSERT(func);
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the loop to use RAII for resource management, move invariant computations outside the loop, and simplify type comparisons to reduce complexity.
You can dramatically flatten this by
• pulling `unifyPath(target)` out of the loop
• comparing raw `const char*` against a tiny static list (no `QString` temp)
• using RAII for your table/iter so you don’t need to sprinkle frees everywhere
• removing the per-fs logging (or guard it behind a `DEBUG`-only flag)
For example:
```cpp
// small RAII wrappers for auto‐freeing
struct MntTable {
libmnt_table *ptr{ mnt_new_table() };
~MntTable() { if (ptr) mnt_free_table(ptr); }
};
struct MntIter {
libmnt_iter *ptr{ mnt_new_iter(MNT_ITER_BACKWARD) };
~MntIter() { if (ptr) mnt_free_iter(ptr); }
};
bool Common::findLnfsPath(const QString &target, Compare func) {
Q_ASSERT(func);
MntTable table;
MntIter iter;
if (mnt_table_parse_mtab(table.ptr, nullptr) != 0)
return false;
const QString targetPath = unifyPath(target);
static constexpr const char* kLnfsTypes[] = {"fuse.dlnfs", "ulnfs"};
libmnt_fs *fs = nullptr;
while (mnt_table_next_fs(table.ptr, iter.ptr, &fs) == 0 && fs) {
const char* mountPoint = mnt_fs_get_target(fs);
if (!mountPoint) continue;
// raw C‐string compare, no QString temp
for (auto type : kLnfsTypes) {
if (strcmp(mnt_fs_get_fstype(fs), type) == 0 &&
func(targetPath, unifyPath(mountPoint))) {
return true;
}
}
}
return false;
}
```
This
1. moves the `unifyPath(target)` call out of the hot loop
2. drops all `QString` conversions of `fstype` into raw `strcmp`
3. eliminates deep nesting by inverting and combining conditions
4. uses RAII (`MntTable`/`MntIter`) so you never forget a free—or need to null-check before freeing.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
|
|
||
| bool Common::findDlnfsPath(const QString &target, Compare func) | ||
| bool Common::findLnfsPath(const QString &target, Compare func) |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the loop to use RAII for resource management, move invariant computations outside the loop, and simplify type comparisons to reduce complexity.
You can dramatically flatten this by
• pulling unifyPath(target) out of the loop
• comparing raw const char* against a tiny static list (no QString temp)
• using RAII for your table/iter so you don’t need to sprinkle frees everywhere
• removing the per-fs logging (or guard it behind a DEBUG-only flag)
For example:
// small RAII wrappers for auto‐freeing
struct MntTable {
libmnt_table *ptr{ mnt_new_table() };
~MntTable() { if (ptr) mnt_free_table(ptr); }
};
struct MntIter {
libmnt_iter *ptr{ mnt_new_iter(MNT_ITER_BACKWARD) };
~MntIter() { if (ptr) mnt_free_iter(ptr); }
};
bool Common::findLnfsPath(const QString &target, Compare func) {
Q_ASSERT(func);
MntTable table;
MntIter iter;
if (mnt_table_parse_mtab(table.ptr, nullptr) != 0)
return false;
const QString targetPath = unifyPath(target);
static constexpr const char* kLnfsTypes[] = {"fuse.dlnfs", "ulnfs"};
libmnt_fs *fs = nullptr;
while (mnt_table_next_fs(table.ptr, iter.ptr, &fs) == 0 && fs) {
const char* mountPoint = mnt_fs_get_target(fs);
if (!mountPoint) continue;
// raw C‐string compare, no QString temp
for (auto type : kLnfsTypes) {
if (strcmp(mnt_fs_get_fstype(fs), type) == 0 &&
func(targetPath, unifyPath(mountPoint))) {
return true;
}
}
}
return false;
}This
- moves the
unifyPath(target)call out of the hot loop - drops all
QStringconversions offstypeinto rawstrcmp - eliminates deep nesting by inverting and combining conditions
- uses RAII (
MntTable/MntIter) so you never forget a free—or need to null-check before freeing.
lzwind
approved these changes
Nov 21, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, 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 |
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.
-- Renamed functions and updated logic to improve clarity and consistency in handling long file names across the codebase. The function
isSubpathOfDlnfsis nowisSubpathOfLnfs, andfindDlnfsPathhas been changed tofindLnfsPath. This refactoring enhances the readability of the code and ensures that the correct file system types are checked for long file name support.Log: Refactor long file name handling in common interface
Bug: https://pms.uniontech.com/bug-view-326015.html
Summary by Sourcery
Refactor long file name handling by renaming related functions to ‘Lnfs’, enhancing detection logic to check fuse.dlnfs and ulnfs filesystem types, improving logging, and updating all plugin callers accordingly
Bug Fixes:
Enhancements: