From d04b07750619f1a21f0ecf8e8e3c7cd21ff2ebb4 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 17 Feb 2020 12:00:47 -0600 Subject: [PATCH] Fixed minor things to get CI passing again - Added caching to Travis install dirs, because otherwise pip3 install fails randomly - Increased size of littlefs-fuse disk because test script has a larger footprint now - Skip a couple of reentrant tests under byte-level writes because the tests just take too long and cause Travis to bail due to no output for 10m - Fixed various Valgrind errors - Suppressed uninit checks for tests where LFS_BLOCK_ERASE_VALUE == -1. In this case rambd goes uninitialized, which is fine for rambd's purposes. Note I couldn't figure out how to limit this suppression to only the malloc in rambd, this doesn't seem possible with Valgrind. - Fixed memory leaks in exhaustion tests - Fixed off-by-1 string null-terminator issue in paths tests - Fixed lfs_file_sync issue caused by revealed by fixing memory leaks in exhaustion tests. Getting ENOSPC during a file write puts the file in a bad state where littlefs doesn't know how to write it out safely. In this case, lfs_file_sync and lfs_file_close return 0 without writing out state so that device-side resources can still be cleaned up. To recover from ENOSPC, the file needs to be reopened and the writes recreated. Not sure if there is a better way to handle this. - Added some quality-of-life improvements to Valgrind testing - Fit Valgrind messages into truncated output when not in verbose mode - Turned on origin tracking --- .travis.yml | 32 ++++++++++++++------------------ lfs.c | 7 ++++++- scripts/test.py | 6 ++++++ tests/test_dirs.toml | 2 ++ tests/test_exhaustion.toml | 20 ++++++++++++++++++++ tests/test_paths.toml | 5 ++--- 6 files changed, 50 insertions(+), 22 deletions(-) diff --git a/.travis.yml b/.travis.yml index 5d10d4e..0ee90a2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,10 +4,16 @@ env: - CFLAGS=-Werror - MAKEFLAGS=-j +# cache installation dirs +cache: + pip: true + directories: + - $HOME/.cache/apt + # common installation _: &install-common # need toml, also pip3 isn't installed by default? - - sudo apt-get install python3-pip + - sudo apt-get install python3 python3-pip - sudo pip3 install toml # setup a ram-backed disk to speed up reentrant tests - mkdir disks @@ -189,22 +195,12 @@ jobs: stage: test env: - NAME=littlefs-valgrind - - TFLAGS="$TFLAGS --valgrind" install: - *install-common - sudo apt-get install valgrind - valgrind --version - script: *test-example - - {<<: *valgrind, script: *test-default} - - {<<: *valgrind, script: *test-nor} - - {<<: *valgrind, script: *test-emmc} - - {<<: *valgrind, script: *test-nand} - - {<<: *valgrind, script: *test-no-intrinsics} - - {<<: *valgrind, script: *test-no-inline} - - {<<: *valgrind, script: *test-byte-writes} - - {<<: *valgrind, script: *test-block-cycles} - - {<<: *valgrind, script: *test-odd-block-count} - - {<<: *valgrind, script: *test-odd-block-size} + script: + - make test TFLAGS+="-k --valgrind" # self-host with littlefs-fuse for fuzz test - stage: test @@ -224,7 +220,7 @@ jobs: - mkdir mount - sudo chmod a+rw /dev/loop0 - - dd if=/dev/zero bs=512 count=4096 of=disk + - dd if=/dev/zero bs=512 count=128K of=disk - losetup /dev/loop0 disk script: # self-host test @@ -239,7 +235,7 @@ jobs: - cd mount/littlefs - stat . - ls -flh - - make -B test_dirs test_files QUIET=1 + - make -B test # test migration using littlefs-fuse - stage: test @@ -260,7 +256,7 @@ jobs: - mkdir mount - sudo chmod a+rw /dev/loop0 - - dd if=/dev/zero bs=512 count=4096 of=disk + - dd if=/dev/zero bs=512 count=128K of=disk - losetup /dev/loop0 disk script: # compile v1 and v2 @@ -277,7 +273,7 @@ jobs: - cd mount/littlefs - stat . - ls -flh - - make -B test_dirs test_files QUIET=1 + - make -B test # attempt to migrate - cd ../.. @@ -291,7 +287,7 @@ jobs: - cd mount/littlefs - stat . - ls -flh - - make -B test_dirs test_files QUIET=1 + - make -B test # automatically create releases - stage: deploy diff --git a/lfs.c b/lfs.c index 354cebc..7190312 100644 --- a/lfs.c +++ b/lfs.c @@ -2707,6 +2707,12 @@ int lfs_file_sync(lfs_t *lfs, lfs_file_t *file) { LFS_TRACE("lfs_file_sync(%p, %p)", (void*)lfs, (void*)file); LFS_ASSERT(file->flags & LFS_F_OPENED); + if (file->flags & LFS_F_ERRED) { + // it's not safe to do anything if our file errored + LFS_TRACE("lfs_file_sync -> %d", 0); + return 0; + } + int err = lfs_file_flush(lfs, file); if (err) { file->flags |= LFS_F_ERRED; @@ -2715,7 +2721,6 @@ int lfs_file_sync(lfs_t *lfs, lfs_file_t *file) { } if ((file->flags & LFS_F_DIRTY) && - !(file->flags & LFS_F_ERRED) && !lfs_pair_isnull(file->m.pair)) { // update dir entry uint16_t type; diff --git a/scripts/test.py b/scripts/test.py index 018d0b2..6187028 100755 --- a/scripts/test.py +++ b/scripts/test.py @@ -299,10 +299,16 @@ class ValgrindTestCase(TestCase): return not self.leaky and super().shouldtest(**args) def test(self, exec=[], **args): + verbose = args.get('verbose', False) + uninit = (self.defines.get('LFS_ERASE_VALUE', None) == -1) exec = [ 'valgrind', '--leak-check=full', + ] + (['--undef-value-errors=no'] if uninit else []) + [ + ] + (['--track-origins=yes'] if not uninit else []) + [ '--error-exitcode=4', + '--error-limit=no', + ] + (['--num-callers=1'] if not verbose else []) + [ '-q'] + exec return super().test(exec=exec, **args) diff --git a/tests/test_dirs.toml b/tests/test_dirs.toml index 270f4f8..6cfd790 100644 --- a/tests/test_dirs.toml +++ b/tests/test_dirs.toml @@ -157,6 +157,7 @@ code = ''' [[case]] # reentrant many directory creation/rename/removal define.N = [5, 11] reentrant = true +if = 'LFS_CACHE_SIZE >= 4' # these just take too long with byte-level writes code = ''' err = lfs_mount(&lfs, &cfg); if (err) { @@ -383,6 +384,7 @@ code = ''' [[case]] # reentrant file creation/rename/removal define.N = [5, 25] reentrant = true +if = 'LFS_CACHE_SIZE >= 4' # these just take too long with byte-level writes code = ''' err = lfs_mount(&lfs, &cfg); if (err) { diff --git a/tests/test_exhaustion.toml b/tests/test_exhaustion.toml index 26eeeba..569611c 100644 --- a/tests/test_exhaustion.toml +++ b/tests/test_exhaustion.toml @@ -33,6 +33,9 @@ code = ''' lfs_ssize_t res = lfs_file_write(&lfs, &file, &c, 1); assert(res == 1 || res == LFS_ERR_NOSPC); if (res == LFS_ERR_NOSPC) { + err = lfs_file_close(&lfs, &file); + assert(err == 0 || err == LFS_ERR_NOSPC); + lfs_unmount(&lfs) => 0; goto exhausted; } } @@ -40,6 +43,7 @@ code = ''' err = lfs_file_close(&lfs, &file); assert(err == 0 || err == LFS_ERR_NOSPC); if (err == LFS_ERR_NOSPC) { + lfs_unmount(&lfs) => 0; goto exhausted; } } @@ -111,6 +115,9 @@ code = ''' lfs_ssize_t res = lfs_file_write(&lfs, &file, &c, 1); assert(res == 1 || res == LFS_ERR_NOSPC); if (res == LFS_ERR_NOSPC) { + err = lfs_file_close(&lfs, &file); + assert(err == 0 || err == LFS_ERR_NOSPC); + lfs_unmount(&lfs) => 0; goto exhausted; } } @@ -118,6 +125,7 @@ code = ''' err = lfs_file_close(&lfs, &file); assert(err == 0 || err == LFS_ERR_NOSPC); if (err == LFS_ERR_NOSPC) { + lfs_unmount(&lfs) => 0; goto exhausted; } } @@ -198,6 +206,9 @@ code = ''' lfs_ssize_t res = lfs_file_write(&lfs, &file, &c, 1); assert(res == 1 || res == LFS_ERR_NOSPC); if (res == LFS_ERR_NOSPC) { + err = lfs_file_close(&lfs, &file); + assert(err == 0 || err == LFS_ERR_NOSPC); + lfs_unmount(&lfs) => 0; goto exhausted; } } @@ -205,6 +216,7 @@ code = ''' err = lfs_file_close(&lfs, &file); assert(err == 0 || err == LFS_ERR_NOSPC); if (err == LFS_ERR_NOSPC) { + lfs_unmount(&lfs) => 0; goto exhausted; } } @@ -283,6 +295,9 @@ code = ''' lfs_ssize_t res = lfs_file_write(&lfs, &file, &c, 1); assert(res == 1 || res == LFS_ERR_NOSPC); if (res == LFS_ERR_NOSPC) { + err = lfs_file_close(&lfs, &file); + assert(err == 0 || err == LFS_ERR_NOSPC); + lfs_unmount(&lfs) => 0; goto exhausted; } } @@ -290,6 +305,7 @@ code = ''' err = lfs_file_close(&lfs, &file); assert(err == 0 || err == LFS_ERR_NOSPC); if (err == LFS_ERR_NOSPC) { + lfs_unmount(&lfs) => 0; goto exhausted; } } @@ -364,6 +380,9 @@ code = ''' lfs_ssize_t res = lfs_file_write(&lfs, &file, &c, 1); assert(res == 1 || res == LFS_ERR_NOSPC); if (res == LFS_ERR_NOSPC) { + err = lfs_file_close(&lfs, &file); + assert(err == 0 || err == LFS_ERR_NOSPC); + lfs_unmount(&lfs) => 0; goto exhausted; } } @@ -371,6 +390,7 @@ code = ''' err = lfs_file_close(&lfs, &file); assert(err == 0 || err == LFS_ERR_NOSPC); if (err == LFS_ERR_NOSPC) { + lfs_unmount(&lfs) => 0; goto exhausted; } } diff --git a/tests/test_paths.toml b/tests/test_paths.toml index 480dd9a..a7474c0 100644 --- a/tests/test_paths.toml +++ b/tests/test_paths.toml @@ -247,14 +247,14 @@ code = ''' lfs_mkdir(&lfs, "coffee/coldcoffee") => 0; memset(path, 'w', LFS_NAME_MAX+1); - path[LFS_NAME_MAX+2] = '\0'; + path[LFS_NAME_MAX+1] = '\0'; lfs_mkdir(&lfs, path) => LFS_ERR_NAMETOOLONG; lfs_file_open(&lfs, &file, path, LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NAMETOOLONG; memcpy(path, "coffee/", strlen("coffee/")); memset(path+strlen("coffee/"), 'w', LFS_NAME_MAX+1); - path[strlen("coffee/")+LFS_NAME_MAX+2] = '\0'; + path[strlen("coffee/")+LFS_NAME_MAX+1] = '\0'; lfs_mkdir(&lfs, path) => LFS_ERR_NAMETOOLONG; lfs_file_open(&lfs, &file, path, LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NAMETOOLONG; @@ -270,7 +270,6 @@ code = ''' lfs_mkdir(&lfs, "coffee/warmcoffee") => 0; lfs_mkdir(&lfs, "coffee/coldcoffee") => 0; - lfs_mount(&lfs, &cfg) => 0; memset(path, 'w', LFS_NAME_MAX); path[LFS_NAME_MAX] = '\0'; lfs_mkdir(&lfs, path) => 0;