feat:[littelfs]更新至V.2.11.2 #27
Merged
Conversation
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
- block_cycles is signed and should use PRId32 - flags is signed (which is a bit weird) and should be cast for %x Unfortunately exactly what PRI* expands to is dependant on both the compiler and the underlying architecture, so I don't think it's possible for us to catch these mistakes with CI... Found by stefano-zanotti
Turns out major versions break things. Old behavior: Artifacts with same name are merged New behavior: Artifacts with same name error Using a pattern and merging on download should fix this at least on the job-side. Though I do wonder if we'll start running into artifact limit issues with the new way artifacts are handled...
Looks like cross-workflow downloads has finally been added to the standard download-artifact action, so we might as well switch to it to reduce dependencies. dawidd6's version was also missing the merge-multiple feature which is necessary to work around breaking changes in download-artifact's v4 bump. Weirdly it needs GITHUB_TOKEN for some reason? Not sure why this couldn't be implicit.
With GitHub forcibly deprecating old versions of actions, pinning the minor/patch version is more likely to cause breakage than not.
Update github actions to the latest versions
With the existing method, (-DLFS_MALLOC=my_malloc) users often had to use compiler options like -include, which was not so portable. This change introduces another way to provide partial overrides of lfs_util.h using a user-provided header.
Original tests provided by m-kostrzewa, these identify signed overflow (undefined behavior) when compiled with -fsanitize=undefined.
In the previous implementation of lfs_file_seek, we calculated the new offset using signed arithmetic before checking for possible overflow/underflow conditions. This results in undefined behavior in C. Fortunately for us, littlefs is now limited to 31-bit file sizes for API reasons, so we don't have to be too clever here. Doing the arithmetic with unsigned integers and just checking if we're in a valid range afterwards should work. Found by m-kostrzewa and lucic71
- Added METADATA_MAX to test_runner. - Added METADATA_MAX to bench_runner. - Added a simple metadata_max test to test_superblocks, for lack of better location. There have been several issues floating around related to metadata_max and LFS_ERR_NOSPC which makes me think there's a bug in our metadata_max logic. metadata_max was a quick patch and is relatively untested, so an undetected bug isn't too surprising. This commit adds at least some testing over metadata_max. Sure enough, the new test_superblocks_metadata_max test reveals a curious LFS_ERR_NAMETOOLONG error that shouldn't be there. More investigation needed.
The inconsistency here between the use of block_size vs metadata_max was suspicious. Turns out there's a bug when metadata_max == prog_size. We correctly use metadata_max for the block_size/2 check, but we weren't using it for the block_size-40 check. The second check seems unnecessary after the first, but it protects against running out of space in a commit for commit-related metadata (checksums, tail pointers, etc) when we can't program half-blocks. Turns out this is also needed when limiting metadata_max to a single prog, otherwise we risk erroring with LFS_ERR_NOSPC early. Found by ajheck, dpkristensen, NLLK, and likely others.
Like the read/prog/block_size checks, these are just asserts. If these invariants are broken the filesystem will break in surprising ways.
These two small libraries provide examples of error-correction compatible with littlefs (or any filesystem really). It would be nice to eventually provide these as drop-in solutions, but right now it's not really possible without breaking changes to littlefs's block device API. In the meantime, ramcrc32bd and ramrsbd at least provide example implementations that can be adapted to users' own block devices.
This should be a superset of the previous test_paths test suite, while covering a couple more things (more APIs, more path synonyms, utf8, non-printable ascii, non-utf8, etc). Not yet tested are some corner cases with known bugs, mainly around trailing slashes.
As expected these are failing and will need some work to pass. The issue with lfs_file_open allowing trailing slashes was found by rob-zeno, and the issue with lfs_mkdir disallowing trailing slashes was found by XinStellaris, PoppaChubby, pavel-kirienko, inf265, Xywzel, steverpalmer, and likely others.
- lfs_mkdir now accepts trailing slashes:
- before: lfs_mkdir("a/") => LFS_ERR_NOENT
- after: lfs_mkdir("a/") => 0
- lfs_stat, lfs_getattr, etc, now reject trailing slashes if the file is
not a directory:
- before: lfs_stat("reg_a/") => 0
- after: lfs_stat("reg_a/") => LFS_ERR_NOTDIR
Note trailing slashes are accepted if the file is a directory:
- before: lfs_stat("dir_a/") => 0
- after: lfs_stat("dir_a/") => 0
- lfs_file_open now returns LFS_ERR_NOTDIR if the file exists but the
path contains trailing slashes:
- before: lfs_file_open("reg_a/") => LFS_ERR_NOENT
- after: lfs_file_open("reg_a/") => LFS_ERR_NOTDIR
To make these work, the internal lfs_dir_find API required some
interesting changes:
- lfs_dir_find no longer sets id=0x3ff on not finding a parent entry in
the path. Instead, lfs_path_islast can be used to determine if the
modified path references a parent entry or child entry based on the
remainder of the path string.
Note this is only necessary for functions that create new entries
(lfs_mkdir, lfs_rename, lfs_file_open).
- Trailing slashes mean we can no longer rely on the modified path being
NULL-terminated. lfs_path_namelen provides an alternative to strlen
that stops at slash or NULL.
- lfs_path_isdir also tells you if the modified path must reference a
dir (contains trailing slashes). I considered handling this entirely
in lfs_dir_find, but the behavior of entry-creating functions is too
nuanced.
At least lfs_dir_find returns LFS_ERR_NOTDIR if the file exists on
disk.
Like strlen, lfs_path_namelen/islast/isdir are all O(n) where n is the
name length. This isn't great, but if you're using filenames large
enough for this to actually matter... uh... open an issue on GitHub and
we might improve this in the future.
---
There are a couple POSIX incompatibilities that I think are not
worth fixing:
- Root modifications return EINVAL instead of EBUSY:
- littlefs: remove("/") => EINVAL
- POSIX: remove("/") => EBUSY
Reason: This would be the only use of EBUSY in the system.
- We accept modifications of directories with trailing dots:
- littlefs: remove("a/.") => 0
- POSIX: remove("a/.") => EBUSY
Reason: Not worth implementing.
- We do not check for existence of directories followed by dotdots:
- littlefs: stat("a/missing/..") => 0
- POSIX: stat("a/missing/..") => ENOENT
Reason: Difficult to implement non-recursively.
- We accept modifications of directories with trailing dotdots:
- littlefs: rename("a/b/..", "c") => 0
- POSIX: rename("a/b/..", "c") => EBUSY
Reason: Not worth implementing.
These are at least now documented in tests/test_paths.toml, which isn't
the greatest location, but it's at least something until a better
document is created.
Note that these don't really belong in SPEC.md because path parsing is
a function of the driver and has no impact on disk.
This changes the behavior of paths that attempt to navigate above root
to now return LFS_ERR_INVAL:
- before: lfs_stat("/../a") => 0
- after: lfs_stat("/../a") => LFS_ERR_INVAL
This is a bit of an opinionated change while making other path
resolution tweaks.
In terms of POSIX-compatibility, it's a bit unclear exactly what dotdots
above the root should do.
POSIX notes:
> As a special case, in the root directory, dot-dot may refer to the
> root directory itself.
But the word choice of "may" implies it is up to the implementation.
I originally implement this as a root-loop simply because that is what
my Linux machine does, but I now think that's not the best option. Since
we're making other path-related tweaks, we might as well try to adopt
behavior that is, in my opinion, safer and less... weird...
This should also help make paths more consistent with future theoretical
openat-list APIs, where saturating at the current directory is sort of
the least expected behavior.
Unlike normal files, dots (".") should not change the depth when
attempting to skip dotdot ("..") entries.
A weird nuance in the path parser, but at least it had a relatively easy
fix.
Added test_paths_dot_dotdots to prevent a regression.
Before this, the empty path ("") was treated as an alias for the root.
This was unintentional and just a side-effect of how the path parser
worked.
Now, the empty path should always result in LFS_ERR_INVAL:
- before: lfs_stat("") => 0
- after: lfs_stat("") => LFS_ERR_INVAL
These flags change the behavior of open quite significantly. It's useful to cover these in our path tests so the behavior is locked down.
- test_paths_noent_trailing_slashes - test_paths_noent_trailing_dots - test_paths_noent_trailing_dotdots These managed to slip through our path testing but should be tested, if anything just to know exactly what errors these return.
- before: lfs_file_open("missing/") => LFS_ERR_ISDIR
- after: lfs_file_open("missing/") => LFS_ERR_NOTDIR
As noted by bmcdonnell-fb, returning LFS_ERR_ISDIR here was inconsistent
with the case where the file exists:
case before after
lfs_file_open("dir_a") => LFS_ERR_ISDIR LFS_ERR_ISDIR
lfs_file_open("dir_a/") => LFS_ERR_ISDIR LFS_ERR_ISDIR
lfs_file_open("reg_a/") => LFS_ERR_NOTDIR LFS_ERR_NOTDIR
lfs_file_open("missing_a/") => LFS_ERR_ISDIR LFS_ERR_NOTDIR
Note this is consistent with the behavior of lfs_stat:
lfs_file_open("reg_a/") => LFS_ERR_NOTDIR
lfs_stat("reg_a/") => LFS_ERR_NOTDIR
And the only other function that can "create" files, lfs_rename:
lfs_file_open("missing_a/") => LFS_ERR_NOTDIR
lfs_rename("reg_a", "missing_a/") => LFS_ERR_NOTDIR
There is some ongoing discussion about if these should return NOTDIR,
ISDIR, or INVAL, but this is at least an improvement over the
rename/open mismatch.
Write the detect cycles function as a function to optimize code
Add an alternative way to override LFS_MALLOC etc
…ce-format-again Fix some more LFS_TRACE format specifiers
…ek-overflow-ub Fix seek undefined behavior on signed integer overflow
…ospc-issues Fix metadata_max==prog_size commit->end calculation
Minor release: v2.11
This was preventing bench modifications from triggering relevant bench-runner rebuilds.
…ke-build-dep make: Add missing BUILD_DEP include
refactor: value stored to 'diff' is never read
perf: gc might try to populate the lookahead buffer each time
Add asserts on file system reads to make sure no positive values are returned, which would make assumptions on error checks invalid. This fixes clang tidy warnings on uninitialized reads in uses of lfs_dir_get where only negative returns are considered errors.
If lfs_bd_read fails, lfs_fcrc_fromle32 will read uninitialized memory, and hasfcrc will be set to true. This may end up in a "working" state later due to crcs not matching. but it's hard to follow if that woud be the case.
This matches the logic originally implemented in 48bd2bf, which was lost during the big no-recursion refactor 84da4c0. Other notes: - Checking >= 0xff matches the split logic during compaction (line 2158): end - split < 0xff - Grouping dir->erased || dir->count >= 0xff together makes it clear these share a common code path. - Checking for dir->count >= 0xff early avoids committing >8-bit ids to disk. The cat may already be out-of-the bag on this one, but opening the id space up to the full 10-bits should probably be on a non-patch release. Found by dschendt
Curiously, the logic from 48bd2bf was incorrect, and would allow a commit to be tried if erased _or_ dir->count was at risk of overflow. That is clearly wrong, we should only try to commit if both conditions are met... Found again by dschendt
fix: false uninitialized read warning
fix: add missing return causing uninitialized reads
…ir-count fix: compact when dir count hits 0x3ff
Add littlefs-toy to the related projects section.
Closed
Collaborator
Author
|
感觉这个ci改炸了，作者看下 |
Rbb666
approved these changes
Oct 20, 2025
Collaborator
Author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
17 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.
简单验证测试
Littlefs 从 v2.9.9 到 v2.11.2 版本涵盖了多个更新，主要集中在 v2.10.x 和 v2.11.x 版本系列中。这些更新带来了重要的新功能、问题修复以及一些可能影响现有实现的重大变更。
v2.10 系列
此系列版本主要围绕路径解析的重构和一些错误修复。
v2.10.0
LFS_DEFINES作为LFS_CONFIG的简化替代方案，方便用户部分覆盖lfs_util.h中的定义。metadata_max时可能导致过早出现LFS_ERR_NOSPC（无空间）错误的元数据压缩问题。lfs_tortoise_detectcycles函数优化了代码大小。v2.10.1
v2.10.2
v2.11 系列
v2.11 系列引入了对文件系统收缩的有限支持，并包含了一些重要的错误修复。此外，从 v2.11.1 版本开始，littlefs v2 进入了功能冻结状态，以便将开发重点转移到 v3 版本。
v2.11.0
LFS_SHRINKNONRELOCATING编译选项。 如果启用，lfs_fs_grow函数可以用来减少文件系统的block_count，前提是不需要进行块的重定位。 这对于从旧的文件系统镜像迁移非常有用。make test命令现在可以直接在 Mac 上运行。LFS_CRC相关的错误。lfs_fromle32/lfs_frombe32函数在 16 位设备上可能出现的溢出风险。v2.11.1
v2.11.2
dir->count检查逻辑。dir->count检查，并将其限制为小于0xff。return的问题。