Added a couple more fwrite litmus tests

- test_fwrite_reversed_litmus_fragments
- test_fwrite_reversed_litmus_blocks
- test_fwrite_freversed
- test_fwrite_freversed_litmus_fragments
- test_fwrite_freversed_litmus_blocks
- test_fwrite_truncate_pos
- test_fwrite_fruncate_pos

And hey, they found some bugs:

- crystal_thresh=-1 was broken due to integer overflow in some signed
  math.

  Fortunately when crystal_thresh=-1 we can just skip the crystal
  lookups entirely. This saves a btree lookup in fully-fragmented files.

- We were including empty fragments in our crystal size, when we should
  only use them to determine crystal boundaries, like bptrs.

  This is a common case for the first entry in a sparse file.

- We weren't updating pos on fruncate. fruncate's effect on pos was
  actually not tested at all.

  Which raises the question, what should the behavior be? Match
  lfsr_file_truncate and leave the pos unaffected?

  I ended up having fruncate update the file pos to keep the same pos
  relative to the end, as I figured this would have the least surprise
  for users. So lfsr_file_read should return the same bytes unless
  clobbered.

  This is almost a mirror image of lfsr_file_truncate, except we don't
  allow negative positions, so fruncating more than pos forces pos to 0.

  ---

  This behavior is now covered in a couple tests:

  - test_fwrite_truncate_pos
  - test_fwrite_fruncate_pos
  - test_fwrite_freversed
  - test_fwrite_freversed_litmus_fragments
  - test_fwrite_freversed_litmus_blocks

Code changes:

           code          stack          ctx
  before: 35688           2440          640
  after:  35692 (+0.0%)   2440 (+0.0%)  640 (+0.0%)
This commit is contained in:
Christopher Haster
2025-04-21 19:01:25 -05:00
parent b3669f02c2
commit a0b3eccf15
2 changed files with 900 additions and 14 deletions
+11 -2
View File
@@ -11989,7 +11989,8 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file,
// within our tree? find left crystal neighbor
if (pos > 0
&& lfs->cfg->crystal_thresh > 0
// if crystal_thresh is 0 or -1, we can skip these
&& (lfs_soff_t)lfs->cfg->crystal_thresh > 0
&& (lfs_soff_t)(pos - (lfs->cfg->crystal_thresh-1))
< (lfs_soff_t)file->b.shrub.weight
&& file->b.shrub.weight > 0
@@ -12010,6 +12011,7 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file,
// obvious hole between our own crystal and our neighbor,
// include as a part of our crystal
if (!lfsr_bptr_isbptr(&bptr)
&& lfsr_data_size(bptr.data) > 0
// hole? holes can be quite large and shouldn't trigger
// crystallization
&& (lfs_soff_t)(bid-(weight-1)+lfsr_data_size(bptr.data))
@@ -12066,7 +12068,8 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file,
// if right crystal neighbor is a fragment, include as a part
// of our crystal
if (!lfsr_bptr_isbptr(&bptr)) {
if (!lfsr_bptr_isbptr(&bptr)
&& lfsr_data_size(bptr.data) > 0) {
crystal_end = lfs_max(
bid-(weight-1)+lfsr_data_size(bptr.data),
crystal_end);
@@ -13091,6 +13094,12 @@ int lfsr_file_fruncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size_) {
size - size_,
file->cache.pos);
// fruncate _does_ update pos, to keep the same pos relative to end
// of file, though we can't let pos go negative
file->pos -= lfs_smin(
size - size_,
file->pos);
// sync if requested
if (lfsr_o_issync(file->b.o.flags)) {
err = lfsr_file_sync(lfs, file);