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
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
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.
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.
Thanks to yamt, GCC-specific flags should now be disabled if compiling
with clang. Dropping the explicit flags also doubles as a test that the
NO_GCC inference works.
This is the same as the implicit Clang => NO_GCC behavior introduced by
yamt, but with an explicit variable that can be assigned by users using
other, non-gcc, compilers:
$ NO_GCC=1 make
Note, stack measurements are currently GCC specific:
$ NO_GCC=1 make stack
... snip ...
FileNotFoundError: [Errno 2] No such file or directory: 'lfs.ci'
make: *** [Makefile:494: lfs.stack.csv] Error 1
Long story short: There is a specific case where removing a directory
can trigger a deorphan pass, but lfs_remove did not check for this,
would try to clean up the (already cleaned) directory orphan, and
trigger an assert:
lfs.c:4890:assert: assert failed with false, expected eq true
LFS_ASSERT(lfs_tag_size(lfs->gstate.tag) > 0x000 || orphans >= 0);
The specific case being a remove commit that triggers a relocation that
creates an orphan.
This is also possible in lfs_rename, but only if you're renaming a
directory that implies a remove, which is a pretty rare operation.
---
This was probably an oversight introduced in the non-recursive commit
logic rework.
Fortunately the fix is to just check if we even have an orphan before
trying to remove it. We can rely on this instead of the file type, so
this fix shouldn't even increase the code size.
Found and root-caused by Hugh-Baoa
These are the same as the related reentrant variants, but by opting out
of powerloss testing, we can test a much larger number of states without
having to worry about the impact on powerloss testing runtime.
Bumped CYCLES from 20 -> 2000.
This reveals an orphan remove bug found by Hugh-Baoa.
This PR adds a new `lfs_fs_shrink`, which functions similarly to
`lfs_fs_grow`, but supports reducing the block count.
This functions first checks that none of the removed block are in use.
If it is the case, it will fail.
lfs_gstate_t was assumed to be a packed array of uint32_t,
but this is not always guaranteed. Access the fields directly
instead of attempting to loop over an array of uint32_t
Fixes clang tidy warnings about use of uninitialized memory
accessed.
