From f64fdd51dc2fac159e31a2d972ed69bddb581c43 Mon Sep 17 00:00:00 2001 From: Kevin Bracey Date: Thu, 7 Jun 2018 14:23:38 +0300 Subject: [PATCH 1/4] Add FileHandle::truncate and ftruncate Add support for file truncation (or extension) to the abstract API. No hooks to actual implementations in this commit. --- platform/FileHandle.h | 15 +++++++++++++++ platform/mbed_retarget.cpp | 17 +++++++++++++++++ platform/mbed_retarget.h | 1 + 3 files changed, 33 insertions(+) diff --git a/platform/FileHandle.h b/platform/FileHandle.h index 03b6399a66e..7c03bbcfd49 100644 --- a/platform/FileHandle.h +++ b/platform/FileHandle.h @@ -137,6 +137,21 @@ class FileHandle : private NonCopyable { */ virtual off_t size(); + /** Truncate or extend a file. + * + * The file's length is set to the specified value. The seek pointer is + * not changed. If the file is extended, the extended area appears as if + * it were zero-filled. + * + * @param length The requested new length for the file + * + * @return Zero on success, negative error code on failure + */ + virtual int truncate(off_t length) + { + return -EINVAL; + } + /** Move the file position to a given offset from a given location. * * @param offset The offset from whence to move to diff --git a/platform/mbed_retarget.cpp b/platform/mbed_retarget.cpp index 711ba0c6ae0..f406671ae11 100644 --- a/platform/mbed_retarget.cpp +++ b/platform/mbed_retarget.cpp @@ -841,6 +841,23 @@ extern "C" off_t lseek(int fildes, off_t offset, int whence) return off; } +extern "C" int ftruncate(int fildes, off_t length) +{ + FileHandle* fhc = get_fhc(fildes); + if (fhc == NULL) { + errno = EBADF; + return -1; + } + + int err = fhc->truncate(length); + if (err < 0) { + errno = -err; + return -1; + } else { + return 0; + } +} + #ifdef __ARMCC_VERSION extern "C" int PREFIX(_ensure)(FILEHANDLE fh) { diff --git a/platform/mbed_retarget.h b/platform/mbed_retarget.h index d61f3c8c653..38c0103fda9 100644 --- a/platform/mbed_retarget.h +++ b/platform/mbed_retarget.h @@ -523,6 +523,7 @@ extern "C" { ssize_t write(int fildes, const void *buf, size_t nbyte); ssize_t read(int fildes, void *buf, size_t nbyte); off_t lseek(int fildes, off_t offset, int whence); + int ftruncate(int fildes, off_t length); int isatty(int fildes); int fsync(int fildes); int fstat(int fildes, struct stat *st); From 025ac2389d3deacb42b320016a79d60ac68505ed Mon Sep 17 00:00:00 2001 From: Kevin Bracey Date: Tue, 30 Oct 2018 17:13:45 +0200 Subject: [PATCH 2/4] Rework fseek/ftell tests ARM C library is really good at optimising out calls to underlying seek. The only ones we were actually detecting in the empty file case were the ones that the default FileHandle::size() made itself during the SEEK_END case. When we implement TestFile::size() directly, we will no longer see a single seek call from the ARM C library in the empty file case, so remove those tests. Beef up the non-empty file case, adding checks that we are making underlying read+write calls in the correct position, as a proxy for direct checks for underlying seek being called. --- TESTS/mbed_platform/FileHandle/main.cpp | 34 ++++++++++--------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/TESTS/mbed_platform/FileHandle/main.cpp b/TESTS/mbed_platform/FileHandle/main.cpp index 7d5a2c21f59..0bf9d2c18b2 100644 --- a/TESTS/mbed_platform/FileHandle/main.cpp +++ b/TESTS/mbed_platform/FileHandle/main.cpp @@ -384,17 +384,15 @@ void test_fprintf_fscanf() /** Test fseek and ftell * - * Given already opened file is empty - * - * When set the file position indicator via fseek - * Then underneath retargeting layer seek function is called - * fseek return with succeed and ftell return already set position + * ARM library is quite good at optimising out unnecessary calls to underlying + * seek, so only test real non empty files. * * Given already opened file is not empty * * When set the file position indicator via fseek * Then underneath retargeting layer seek function is called * fseek return with succeed and ftell return already set position + * Check actual character read or written. * */ void test_fseek_ftell() @@ -413,19 +411,6 @@ void test_fseek_ftell() ftell_ret = std::ftell(file); TEST_ASSERT_EQUAL(0, ftell_ret); - TestFile::resetFunctionCallHistory(); - fssek_ret = std::fseek(file, 0, SEEK_CUR); - TEST_ASSERT_EQUAL(0, fssek_ret); - - TestFile::resetFunctionCallHistory(); - fssek_ret = std::fseek(file, 0, SEEK_SET); - TEST_ASSERT_EQUAL(0, fssek_ret); - - TestFile::resetFunctionCallHistory(); - fssek_ret = std::fseek(file, 0, SEEK_END); - TEST_ASSERT_TRUE(TestFile::functionCalled(TestFile::fnSeek)); - TEST_ASSERT_EQUAL(0, fssek_ret); - const char *str = "Hello world"; const std::size_t size = std::strlen(str); @@ -440,19 +425,28 @@ void test_fseek_ftell() TEST_ASSERT_EQUAL(0, fssek_ret); ftell_ret = std::ftell(file); TEST_ASSERT_EQUAL(5, ftell_ret); + int c = std::fgetc(file); + TEST_ASSERT_TRUE(TestFile::functionCalled(TestFile::fnRead)); + TEST_ASSERT_EQUAL(c, str[5]); TestFile::resetFunctionCallHistory(); - fssek_ret = std::fseek(file, -5, SEEK_CUR); + fssek_ret = std::fseek(file, -6, SEEK_CUR); TEST_ASSERT_EQUAL(0, fssek_ret); ftell_ret = std::ftell(file); TEST_ASSERT_EQUAL(0, ftell_ret); + c = std::fgetc(file); + TEST_ASSERT_TRUE(TestFile::functionCalled(TestFile::fnRead)); + TEST_ASSERT_EQUAL(c, str[0]); TestFile::resetFunctionCallHistory(); fssek_ret = std::fseek(file, 0, SEEK_END); - TEST_ASSERT_TRUE(TestFile::functionCalled(TestFile::fnSeek)); TEST_ASSERT_EQUAL(0, fssek_ret); ftell_ret = std::ftell(file); TEST_ASSERT_EQUAL(size, ftell_ret); + c = std::fputc('!', file); + TEST_ASSERT_TRUE(TestFile::functionCalled(TestFile::fnWrite)); + TEST_ASSERT_EQUAL(c, '!'); + TEST_ASSERT_EQUAL(fh.size(), size + 1); std::fclose(file); } From 571a771cd63791afe971395572f1545484db1da8 Mon Sep 17 00:00:00 2001 From: Kevin Bracey Date: Mon, 2 Jul 2018 13:39:47 +0300 Subject: [PATCH 3/4] Add ftruncate / fstat(st_size) unit test --- TESTS/mbed_platform/FileHandle/TestFile.h | 20 ++++++- TESTS/mbed_platform/FileHandle/main.cpp | 65 ++++++++++++++++++++++- 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/TESTS/mbed_platform/FileHandle/TestFile.h b/TESTS/mbed_platform/FileHandle/TestFile.h index 1c8ce7273e0..cca56c5bcfa 100644 --- a/TESTS/mbed_platform/FileHandle/TestFile.h +++ b/TESTS/mbed_platform/FileHandle/TestFile.h @@ -30,7 +30,7 @@ class TestFile : public mbed::FileHandle { ~TestFile() {} enum FunctionName { - fnNone, fnRead, fnWrite, fnSeek, fnClose, fnIsatty + fnNone, fnRead, fnWrite, fnSeek, fnClose, fnIsatty, fnTruncate }; virtual ssize_t read(void *buffer, size_t size) @@ -107,6 +107,24 @@ class TestFile : public mbed::FileHandle { return 0; } + virtual off_t size() + { + return _end; + } + + virtual int truncate(off_t length) + { + _fnCalled = fnTruncate; + if (!NEW_POS_IS_VALID(length)) { + return -EINVAL; + } + while (_end < length) { + _data[_end++] = 0; + } + _end = length; + return 0; + } + static void resetFunctionCallHistory() { diff --git a/TESTS/mbed_platform/FileHandle/main.cpp b/TESTS/mbed_platform/FileHandle/main.cpp index 0bf9d2c18b2..65d670e6f50 100644 --- a/TESTS/mbed_platform/FileHandle/main.cpp +++ b/TESTS/mbed_platform/FileHandle/main.cpp @@ -451,6 +451,68 @@ void test_fseek_ftell() std::fclose(file); } +/** Test ftruncate and fstat (st_size) + * + * Check we get EBADF for illegal handles + * + * Given already opened file is empty + * + * Check initial size is returned as 0 + * Call ftruncate with negative value - check our EINVAL is passed back + * Call ftruncate with positive value to increase size - check no error return + * Check fstat st_size now reads back as the value we set. + * Call ftruncate with smaller positive value to decrease size - check no error return + * Check fstat st_size now reads back as the value we set. + */ +void test_ftruncate_fstat() +{ + int fildes; + int ret; + struct stat st; + const uint32_t FS = 128; + TestFile fh; + + ret = ftruncate(12345678, 24); + TEST_ASSERT_EQUAL(-1, ret); + TEST_ASSERT_EQUAL(EBADF, errno); + + ret = fstat(12345678, &st); + TEST_ASSERT_EQUAL(-1, ret); + TEST_ASSERT_EQUAL(EBADF, errno); + + fildes = bind_to_fd(&fh); + TEST_ASSERT_TRUE(fildes >= 0); + + ret = fstat(fildes, &st); + TEST_ASSERT_EQUAL(0, ret); + TEST_ASSERT_EQUAL(0, st.st_size); + + TestFile::resetFunctionCallHistory(); + ret = ftruncate(fildes, -3); + TEST_ASSERT_TRUE(TestFile::functionCalled(TestFile::fnTruncate)); + TEST_ASSERT_EQUAL(-1, ret); + TEST_ASSERT_EQUAL(EINVAL, errno); + + TestFile::resetFunctionCallHistory(); + ret = ftruncate(fildes, 24); + TEST_ASSERT_TRUE(TestFile::functionCalled(TestFile::fnTruncate)); + TEST_ASSERT_EQUAL(0, ret); + + ret = fstat(fildes, &st); + TEST_ASSERT_EQUAL(0, ret); + TEST_ASSERT_EQUAL(24, st.st_size); + + ret = ftruncate(fildes, 12); + TEST_ASSERT_TRUE(TestFile::functionCalled(TestFile::fnTruncate)); + TEST_ASSERT_EQUAL(0, ret); + + ret = fstat(fildes, &st); + TEST_ASSERT_EQUAL(0, ret); + TEST_ASSERT_EQUAL(12, st.st_size); + + close(fildes); +} + utest::v1::status_t test_setup(const size_t number_of_cases) { GREENTEA_SETUP(10, "default_auto"); @@ -463,7 +525,8 @@ Case cases[] = { Case("Test fputc/fgetc", test_fputc_fgetc), Case("Test fputs/fgets", test_fputs_fgets), Case("Test fprintf/fscanf", test_fprintf_fscanf), - Case("Test fseek/ftell", test_fseek_ftell) + Case("Test fseek/ftell", test_fseek_ftell), + Case("Test ftruncate/fstat", test_ftruncate_fstat) }; utest::v1::Specification specification(test_setup, cases); From e22e9cd44105483b75d7b8887d362830f5cc50e1 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 2 Jul 2018 12:14:21 -0500 Subject: [PATCH 4/4] Added filesystem implementations of truncate - File::truncate - FileSystem::file_truncate - FATFileSystem::file_truncate - LittleFileSystem::file_truncate --- features/storage/filesystem/File.cpp | 6 ++++ features/storage/filesystem/File.h | 12 +++++++ features/storage/filesystem/FileSystem.cpp | 5 +++ features/storage/filesystem/FileSystem.h | 13 ++++++++ .../storage/filesystem/fat/FATFileSystem.cpp | 31 +++++++++++++++++++ .../storage/filesystem/fat/FATFileSystem.h | 13 ++++++++ .../filesystem/littlefs/LittleFileSystem.cpp | 11 +++++++ .../filesystem/littlefs/LittleFileSystem.h | 13 ++++++++ 8 files changed, 104 insertions(+) diff --git a/features/storage/filesystem/File.cpp b/features/storage/filesystem/File.cpp index 76bf66aebbd..eefce9fc1ee 100644 --- a/features/storage/filesystem/File.cpp +++ b/features/storage/filesystem/File.cpp @@ -110,4 +110,10 @@ off_t File::size() return _fs->file_size(_file); } +int File::truncate(off_t length) +{ + MBED_ASSERT(_fs); + return _fs->file_truncate(_file, length); +} + } // namespace mbed diff --git a/features/storage/filesystem/File.h b/features/storage/filesystem/File.h index d08cf7c0d77..8fb098b5a52 100644 --- a/features/storage/filesystem/File.h +++ b/features/storage/filesystem/File.h @@ -126,6 +126,18 @@ class File : public FileHandle { */ virtual off_t size(); + /** Truncate or extend a file. + * + * The file's length is set to the specified value. The seek pointer is + * not changed. If the file is extended, the extended area appears as if + * it were zero-filled. + * + * @param length The requested new length for the file + * + * @return Zero on success, negative error code on failure + */ + virtual int truncate(off_t length); + private: FileSystem *_fs; fs_file_t _file; diff --git a/features/storage/filesystem/FileSystem.cpp b/features/storage/filesystem/FileSystem.cpp index 047f3d7eb08..da8f23e4f39 100644 --- a/features/storage/filesystem/FileSystem.cpp +++ b/features/storage/filesystem/FileSystem.cpp @@ -84,6 +84,11 @@ off_t FileSystem::file_size(fs_file_t file) return size; } +int FileSystem::file_truncate(fs_file_t file, off_t length) +{ + return -ENOSYS; +} + int FileSystem::dir_open(fs_dir_t *dir, const char *path) { return -ENOSYS; diff --git a/features/storage/filesystem/FileSystem.h b/features/storage/filesystem/FileSystem.h index 504d220167e..e9a0e74bab3 100644 --- a/features/storage/filesystem/FileSystem.h +++ b/features/storage/filesystem/FileSystem.h @@ -220,6 +220,19 @@ class FileSystem : public FileSystemLike { */ virtual off_t file_size(fs_file_t file); + /** Truncate or extend a file. + * + * The file's length is set to the specified value. The seek pointer is + * not changed. If the file is extended, the extended area appears as if + * it were zero-filled. + * + * @param file File handle + * @param length The requested new length for the file + * + * @return Zero on success, negative error code on failure + */ + virtual int file_truncate(fs_file_t file, off_t length); + /** Open a directory on the filesystem * * @param dir Destination for the handle to the directory diff --git a/features/storage/filesystem/fat/FATFileSystem.cpp b/features/storage/filesystem/fat/FATFileSystem.cpp index 91d4f430e0e..f679dd3b138 100644 --- a/features/storage/filesystem/fat/FATFileSystem.cpp +++ b/features/storage/filesystem/fat/FATFileSystem.cpp @@ -720,6 +720,37 @@ off_t FATFileSystem::file_size(fs_file_t file) return res; } +int FATFileSystem::file_truncate(fs_file_t file, off_t length) +{ + FIL *fh = static_cast(file); + + lock(); + // save current position + FSIZE_t oldoff = f_tell(fh); + + // seek to new file size and truncate + FRESULT res = f_lseek(fh, length); + if (res) { + unlock(); + return fat_error_remap(res); + } + + res = f_truncate(fh); + if (res) { + unlock(); + return fat_error_remap(res); + } + + // restore old position + res = f_lseek(fh, oldoff); + if (res) { + unlock(); + return fat_error_remap(res); + } + + return 0; +} + ////// Dir operations ////// int FATFileSystem::dir_open(fs_dir_t *dir, const char *path) diff --git a/features/storage/filesystem/fat/FATFileSystem.h b/features/storage/filesystem/fat/FATFileSystem.h index b12fc3044b6..ccc7d8e1e3d 100644 --- a/features/storage/filesystem/fat/FATFileSystem.h +++ b/features/storage/filesystem/fat/FATFileSystem.h @@ -217,6 +217,19 @@ class FATFileSystem : public mbed::FileSystem { */ virtual off_t file_size(mbed::fs_file_t file); + /** Truncate or extend a file. + * + * The file's length is set to the specified value. The seek pointer is + * not changed. If the file is extended, the extended area appears as if + * it were zero-filled. + * + * @param file File handle + * @param length The requested new length for the file + * + * @return Zero on success, negative error code on failure + */ + virtual int file_truncate(mbed::fs_file_t file, off_t length); + /** Open a directory on the filesystem * * @param dir Destination for the handle to the directory diff --git a/features/storage/filesystem/littlefs/LittleFileSystem.cpp b/features/storage/filesystem/littlefs/LittleFileSystem.cpp index d5d9982cbcc..1b5457218fd 100644 --- a/features/storage/filesystem/littlefs/LittleFileSystem.cpp +++ b/features/storage/filesystem/littlefs/LittleFileSystem.cpp @@ -474,6 +474,17 @@ off_t LittleFileSystem::file_size(fs_file_t file) return lfs_toerror(res); } +int LittleFileSystem::file_truncate(fs_file_t file, off_t length) +{ + lfs_file_t *f = (lfs_file_t *)file; + _mutex.lock(); + LFS_INFO("file_truncate(%p)", file); + int err = lfs_file_truncate(&_lfs, f, length); + LFS_INFO("file_truncate -> %d", lfs_toerror(err)); + _mutex.unlock(); + return lfs_toerror(err); +} + ////// Dir operations ////// int LittleFileSystem::dir_open(fs_dir_t *dir, const char *path) diff --git a/features/storage/filesystem/littlefs/LittleFileSystem.h b/features/storage/filesystem/littlefs/LittleFileSystem.h index ac65d06fedc..4d29bd0ebb8 100644 --- a/features/storage/filesystem/littlefs/LittleFileSystem.h +++ b/features/storage/filesystem/littlefs/LittleFileSystem.h @@ -220,6 +220,19 @@ class LittleFileSystem : public mbed::FileSystem { */ virtual off_t file_size(mbed::fs_file_t file); + /** Truncate or extend a file. + * + * The file's length is set to the specified value. The seek pointer is + * not changed. If the file is extended, the extended area appears as if + * it were zero-filled. + * + * @param file File handle + * @param length The requested new length for the file + * + * @return Zero on success, negative error code on failure + */ + virtual int file_truncate(mbed::fs_file_t file, off_t length); + /** Open a directory on the filesystem * * @param dir Destination for the handle to the directory