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.
When removing a file, we mark all open handles as "removed" (
pair={-1,-1}) to avoid trying to later read metadata that no longer
exists. Unfortunately, this also includes open dir handles that happen
to be pointing at the removed file, causing them to return
LFS_ERR_CORRUPT on the next read.
The good news is this is _not_ actual filesystem corruption, only a
logic error in lfs_dir_read.
We actually already have logic in place to nudge the dir to the next id,
but it was unreachable with the existing logic. I suspect this worked at
one point but was broken during a refactor due to lack of testing.
---
Fortunately, all we need to do is _not_ clobber the handle if the
internal type is a dir. Then the dir-nudging logic can correctly take
over.
I've also added test_dirs_remove_read to test this and prevent another
regression, adapted from tests provided by tpwrules that identified the
original bug.
Found by tpwrules
In v2.5, we introduced an optimization to avoid rereading data when
seeking inside the file cache. Unfortunately this used a slightly
wrong condition to check if the cache was "live", which meant seeks from
end-of-blocks could end up with invalid caches and wrong data. Not
great.
The problem is the nuance of when a file's cache is "live":
1. The file is marked as LFS_F_READING or LFS_F_WRITING.
But we can't reuse the cache when writing, so we only care about
LFS_F_READING.
2. file->off != lfs->cfg->block_size (end-of-block).
This is an optimization to avoid eagerly reading blocks we may not
actually care about.
We weren't checking for the end-of-block case, which meant if you seeked
_from_ the end of a block to a seemingly valid location in the file
cache, you could end up with an invalid cache.
Note that end-of-block may not be powers-of-two due to CTZ skip-list
pointers.
---
The fix is to check for the end-of-block case in lfs_file_seek. Note
this now matches the need-new-block logic in lfs_file_flushedread.
This logic change may also make lfs_file_seek call lfs_file_flush more
often, but only in cases where lfs_file_flush is a noop.
I've also extended the test_seek tests to cover a few more boundary-read
cases and prevent a regression in the future.
Found by wjl and lrodorigo
