From 35e312c48ec44328f81d47520327a61f0cb51e54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20M=C3=BAdry?= Date: Wed, 7 Feb 2024 19:43:50 +0100 Subject: [PATCH] feat(fatfs): Allow expanding files with seeking and truncating functions Closes https://github.com/espressif/esp-idf/issues/13100 --- .../flash_wl/main/test_fatfs_flash_wl.c | 9 +- .../test_apps/sdcard/main/test_fatfs_sdmmc.c | 4 +- .../test_fatfs_common/test_fatfs_common.c | 153 +++++++++++---- .../test_fatfs_common/test_fatfs_common.h | 4 +- components/fatfs/vfs/vfs_fat.c | 181 ++++++++++++++---- components/vfs/include/esp_vfs.h | 8 +- 6 files changed, 277 insertions(+), 82 deletions(-) diff --git a/components/fatfs/test_apps/flash_wl/main/test_fatfs_flash_wl.c b/components/fatfs/test_apps/flash_wl/main/test_fatfs_flash_wl.c index 155769ceb0..66c9f6930d 100644 --- a/components/fatfs/test_apps/flash_wl/main/test_fatfs_flash_wl.c +++ b/components/fatfs/test_apps/flash_wl/main/test_fatfs_flash_wl.c @@ -190,7 +190,14 @@ TEST_CASE("(WL) can lseek", "[fatfs][wear_levelling]") TEST_CASE("(WL) can truncate", "[fatfs][wear_levelling]") { test_setup(); - test_fatfs_truncate_file("/spiflash/truncate.txt"); + test_fatfs_truncate_file("/spiflash/truncate.txt", true); + test_teardown(); +} + +TEST_CASE("(WL) can ftruncate", "[fatfs][wear_levelling]") +{ + test_setup(); + test_fatfs_ftruncate_file("/spiflash/ftrunc.txt", true); test_teardown(); } diff --git a/components/fatfs/test_apps/sdcard/main/test_fatfs_sdmmc.c b/components/fatfs/test_apps/sdcard/main/test_fatfs_sdmmc.c index 6d66dcd6de..3f709f0a38 100644 --- a/components/fatfs/test_apps/sdcard/main/test_fatfs_sdmmc.c +++ b/components/fatfs/test_apps/sdcard/main/test_fatfs_sdmmc.c @@ -186,7 +186,7 @@ TEST_CASE("(SD) can truncate", "[fatfs][sdmmc]") { sdmmc_card_t *card = NULL; test_setup_sdmmc(&card); - test_fatfs_truncate_file("/sdcard/truncate.txt"); + test_fatfs_truncate_file("/sdcard/truncate.txt", true); test_teardown_sdmmc(card); } @@ -194,7 +194,7 @@ TEST_CASE("(SD) can ftruncate", "[fatfs][sdmmc]") { sdmmc_card_t *card = NULL; test_setup_sdmmc(&card); - test_fatfs_ftruncate_file("/sdcard/ftrunc.txt"); + test_fatfs_ftruncate_file("/sdcard/ftrunc.txt", true); test_teardown_sdmmc(card); } diff --git a/components/fatfs/test_apps/test_fatfs_common/test_fatfs_common.c b/components/fatfs/test_apps/test_fatfs_common/test_fatfs_common.c index aa7be418c0..56b2609d6c 100644 --- a/components/fatfs/test_apps/test_fatfs_common/test_fatfs_common.c +++ b/components/fatfs/test_apps/test_fatfs_common/test_fatfs_common.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -20,6 +20,7 @@ #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "test_fatfs_common.h" +#include "ff.h" const char* fatfs_test_hello_str = "Hello, World!\n"; const char* fatfs_test_hello_str_utf = "世界，你好！\n"; @@ -252,7 +253,7 @@ void test_fatfs_lseek(const char* filename) } -void test_fatfs_truncate_file(const char* filename) +void test_fatfs_truncate_file(const char* filename, bool allow_expanding_files) { int read = 0; int truncated_len = 0; @@ -267,14 +268,44 @@ void test_fatfs_truncate_file(const char* filename) TEST_ASSERT_EQUAL(0, fclose(f)); + struct stat st; + size_t size; - // Extending file beyond size is not supported - TEST_ASSERT_EQUAL(-1, truncate(filename, strlen(input) + 1)); - TEST_ASSERT_EQUAL(errno, EPERM); + stat(filename, &st); + size = st.st_size; + TEST_ASSERT_EQUAL(strlen(input), size); - TEST_ASSERT_EQUAL(-1, truncate(filename, -1)); - TEST_ASSERT_EQUAL(errno, EINVAL); + if (allow_expanding_files) { + size_t trunc_add = 2; + size_t new_size = strlen(input) + trunc_add; + TEST_ASSERT_EQUAL(0, truncate(filename, new_size)); + stat(filename, &st); + size = st.st_size; + TEST_ASSERT_EQUAL(new_size, size); + + f = fopen(filename, "rb"); + TEST_ASSERT_NOT_NULL(f); + + char expanded_output[sizeof(input) + trunc_add]; + memset(expanded_output, 42, sizeof(expanded_output)); // set to something else than 0 (42) + + read = fread(expanded_output, 1, sizeof(input) + trunc_add, f); + TEST_ASSERT_EQUAL(new_size, read); + + TEST_ASSERT_EQUAL('Z', expanded_output[strlen(input) - 1]); // 'Z' character + TEST_ASSERT_EQUAL('\0', expanded_output[sizeof(input) + trunc_add - 3]); // zeroed expanded space + TEST_ASSERT_EQUAL('\0', expanded_output[sizeof(input) + trunc_add - 2]); // zeroed expanded space + TEST_ASSERT_EQUAL(42, expanded_output[sizeof(input) + trunc_add - 1]); // 42 set with memset, end of the array + + TEST_ASSERT_EQUAL(0, fclose(f)); + } else { + TEST_ASSERT_EQUAL(-1, truncate(filename, strlen(input) + 1)); + TEST_ASSERT_EQUAL(errno, EPERM); + + TEST_ASSERT_EQUAL(-1, truncate(filename, -1)); + TEST_ASSERT_EQUAL(errno, EINVAL); + } // Truncating should succeed const char truncated_1[] = "ABCDEFGHIJ"; @@ -282,6 +313,10 @@ void test_fatfs_truncate_file(const char* filename) TEST_ASSERT_EQUAL(0, truncate(filename, truncated_len)); + stat(filename, &st); + size = st.st_size; + TEST_ASSERT_EQUAL(strlen(truncated_1), size); + f = fopen(filename, "rb"); TEST_ASSERT_NOT_NULL(f); @@ -293,28 +328,34 @@ void test_fatfs_truncate_file(const char* filename) TEST_ASSERT_EQUAL(0, fclose(f)); + if (allow_expanding_files) { + TEST_ASSERT_EQUAL(0, truncate(filename, truncated_len + 1)); + } else { + // Once truncated, the new file size should be the basis + // whether truncation should succeed or not when `allow_expanding_files == false` + TEST_ASSERT_EQUAL(-1, truncate(filename, truncated_len + 1)); + TEST_ASSERT_EQUAL(EPERM, errno); - // Once truncated, the new file size should be the basis - // whether truncation should succeed or not - TEST_ASSERT_EQUAL(-1, truncate(filename, truncated_len + 1)); - TEST_ASSERT_EQUAL(EPERM, errno); + TEST_ASSERT_EQUAL(-1, truncate(filename, strlen(input))); + TEST_ASSERT_EQUAL(EPERM, errno); - TEST_ASSERT_EQUAL(-1, truncate(filename, strlen(input))); - TEST_ASSERT_EQUAL(EPERM, errno); - - TEST_ASSERT_EQUAL(-1, truncate(filename, strlen(input) + 1)); - TEST_ASSERT_EQUAL(EPERM, errno); + TEST_ASSERT_EQUAL(-1, truncate(filename, strlen(input) + 1)); + TEST_ASSERT_EQUAL(EPERM, errno); + } TEST_ASSERT_EQUAL(-1, truncate(filename, -1)); TEST_ASSERT_EQUAL(EINVAL, errno); - // Truncating a truncated file should succeed const char truncated_2[] = "ABCDE"; truncated_len = strlen(truncated_2); TEST_ASSERT_EQUAL(0, truncate(filename, truncated_len)); + stat(filename, &st); + size = st.st_size; + TEST_ASSERT_EQUAL(strlen(truncated_2), size); + f = fopen(filename, "rb"); TEST_ASSERT_NOT_NULL(f); @@ -327,29 +368,63 @@ void test_fatfs_truncate_file(const char* filename) TEST_ASSERT_EQUAL(0, fclose(f)); } -void test_fatfs_ftruncate_file(const char* filename) +void test_fatfs_ftruncate_file(const char* filename, bool allow_expanding_files) { int truncated_len = 0; const char input[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; char output[sizeof(input)]; - int fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC); + int fd = open(filename, O_RDWR | O_CREAT | O_TRUNC); TEST_ASSERT_NOT_EQUAL(-1, fd); TEST_ASSERT_EQUAL(strlen(input), write(fd, input, strlen(input))); - // Extending file beyond size is not supported - TEST_ASSERT_EQUAL(-1, ftruncate(fd, strlen(input) + 1)); - TEST_ASSERT_EQUAL(errno, EPERM); + struct stat st; + size_t size; - TEST_ASSERT_EQUAL(-1, ftruncate(fd, -1)); - TEST_ASSERT_EQUAL(errno, EINVAL); + fstat(fd, &st); + size = st.st_size; + TEST_ASSERT_EQUAL(strlen(input), size); + + if (allow_expanding_files) { + size_t trunc_add = 2; + size_t new_size = strlen(input) + trunc_add; + TEST_ASSERT_EQUAL(0, ftruncate(fd, new_size)); + + fstat(fd, &st); + size = st.st_size; + TEST_ASSERT_EQUAL(new_size, size); + + char expanded_output[sizeof(input) + trunc_add]; + memset(expanded_output, 42, sizeof(expanded_output)); // set to something else than 0 (42) + + lseek(fd, 0, SEEK_SET); + int r = read(fd, expanded_output, sizeof(input) + trunc_add); + TEST_ASSERT_NOT_EQUAL(-1, r); + TEST_ASSERT_EQUAL(new_size, r); + + TEST_ASSERT_EQUAL('Z', expanded_output[strlen(input) - 1]); // 'Z' character + TEST_ASSERT_EQUAL('\0', expanded_output[sizeof(input) + trunc_add - 3]); // zeroed expanded space + TEST_ASSERT_EQUAL('\0', expanded_output[sizeof(input) + trunc_add - 2]); // zeroed expanded space + TEST_ASSERT_EQUAL(42, expanded_output[sizeof(input) + trunc_add - 1]); // 42 set with memset, end of the array + } else { + TEST_ASSERT_EQUAL(-1, ftruncate(fd, strlen(input) + 1)); + TEST_ASSERT_EQUAL(errno, EPERM); + + TEST_ASSERT_EQUAL(-1, ftruncate(fd, -1)); + TEST_ASSERT_EQUAL(errno, EINVAL); + } // Truncating should succeed const char truncated_1[] = "ABCDEFGHIJ"; truncated_len = strlen(truncated_1); TEST_ASSERT_EQUAL(0, ftruncate(fd, truncated_len)); + + fstat(fd, &st); + size = st.st_size; + TEST_ASSERT_EQUAL(truncated_len, size); + TEST_ASSERT_EQUAL(0, close(fd)); // open file for reading and validate the content @@ -367,25 +442,35 @@ void test_fatfs_ftruncate_file(const char* filename) // further truncate the file fd = open(filename, O_WRONLY); TEST_ASSERT_NOT_EQUAL(-1, fd); - // Once truncated, the new file size should be the basis - // whether truncation should succeed or not - TEST_ASSERT_EQUAL(-1, ftruncate(fd, truncated_len + 1)); - TEST_ASSERT_EQUAL(EPERM, errno); - TEST_ASSERT_EQUAL(-1, ftruncate(fd, strlen(input))); - TEST_ASSERT_EQUAL(EPERM, errno); + if (allow_expanding_files) { + TEST_ASSERT_EQUAL(0, ftruncate(fd, truncated_len + 1)); + } else { + // Once truncated, the new file size should be the basis + // whether truncation should succeed or not when `allow_expanding_files == false` + TEST_ASSERT_EQUAL(-1, ftruncate(fd, truncated_len + 1)); + TEST_ASSERT_EQUAL(EPERM, errno); - TEST_ASSERT_EQUAL(-1, ftruncate(fd, strlen(input) + 1)); - TEST_ASSERT_EQUAL(EPERM, errno); + TEST_ASSERT_EQUAL(-1, ftruncate(fd, strlen(input))); + TEST_ASSERT_EQUAL(EPERM, errno); - TEST_ASSERT_EQUAL(-1, ftruncate(fd, -1)); - TEST_ASSERT_EQUAL(EINVAL, errno); + TEST_ASSERT_EQUAL(-1, ftruncate(fd, strlen(input) + 1)); + TEST_ASSERT_EQUAL(EPERM, errno); + + TEST_ASSERT_EQUAL(-1, ftruncate(fd, -1)); + TEST_ASSERT_EQUAL(EINVAL, errno); + } // Truncating a truncated file should succeed const char truncated_2[] = "ABCDE"; truncated_len = strlen(truncated_2); TEST_ASSERT_EQUAL(0, ftruncate(fd, truncated_len)); + + fstat(fd, &st); + size = st.st_size; + TEST_ASSERT_EQUAL(truncated_len, size); + TEST_ASSERT_EQUAL(0, close(fd)); // open file for reading and validate the content diff --git a/components/fatfs/test_apps/test_fatfs_common/test_fatfs_common.h b/components/fatfs/test_apps/test_fatfs_common/test_fatfs_common.h index e7366fe1e1..4ae67937dd 100644 --- a/components/fatfs/test_apps/test_fatfs_common/test_fatfs_common.h +++ b/components/fatfs/test_apps/test_fatfs_common/test_fatfs_common.h @@ -45,9 +45,9 @@ void test_fatfs_open_max_files(const char* filename_prefix, size_t files_count); void test_fatfs_lseek(const char* filename); -void test_fatfs_truncate_file(const char* path); +void test_fatfs_truncate_file(const char* path, bool allow_expanding_files); -void test_fatfs_ftruncate_file(const char* path); +void test_fatfs_ftruncate_file(const char* path, bool allow_expanding_files); void test_fatfs_stat(const char* filename, const char* root_dir); diff --git a/components/fatfs/vfs/vfs_fat.c b/components/fatfs/vfs/vfs_fat.c index 55cfbd1f0e..7c717c76c8 100644 --- a/components/fatfs/vfs/vfs_fat.c +++ b/components/fatfs/vfs/vfs_fat.c @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -16,6 +17,8 @@ #include "ff.h" #include "diskio_impl.h" +#define F_WRITE_MALLOC_ZEROING_BUF_SIZE_LIMIT 512 + typedef struct { char fat_drive[8]; /* FAT drive name */ char base_path[ESP_VFS_PATH_MAX]; /* base path in VFS where partition is registered */ @@ -981,6 +984,49 @@ static int vfs_fat_access(void* ctx, const char *path, int amode) return ret; } +static FRESULT f_write_zero_mem(FIL* fp, FSIZE_t data_size, FSIZE_t buf_size, UINT* bytes_written) +{ + if (fp == NULL || data_size <= 0 || buf_size <= 0) { + return FR_INVALID_PARAMETER; + } + + void* buf = ff_memalloc(buf_size); + if (buf == NULL) { + return FR_DISK_ERR; + } + memset(buf, 0, buf_size); + + FRESULT res = FR_OK; + UINT bw = 0; + FSIZE_t i = 0; + if (bytes_written != NULL) { + *bytes_written = 0; + } + + if (data_size > buf_size) { // prevent unsigned underflow + for (; i < (data_size - buf_size); i += buf_size) { // write chunks of buf_size + res = f_write(fp, buf, (UINT) buf_size, &bw); + if (res != FR_OK) { + goto out; + } + if (bytes_written != NULL) { + *bytes_written += bw; + } + } + } + + if (i < data_size) { // write the remaining data + res = f_write(fp, buf, (UINT) (data_size - i), &bw); + if (res == FR_OK && bytes_written != NULL) { + *bytes_written += bw; + } + } + +out: + ff_memfree(buf); + return res; +} + static int vfs_fat_truncate(void* ctx, const char *path, off_t length) { FRESULT res; @@ -1019,32 +1065,55 @@ static int vfs_fat_truncate(void* ctx, const char *path, off_t length) goto out; } - long sz = f_size(file); - if (sz < length) { - _lock_release(&fat_ctx->lock); - ESP_LOGD(TAG, "truncate does not support extending size"); - errno = EPERM; - ret = -1; - goto close; - } + FSIZE_t seek_ptr_pos = (FSIZE_t) f_tell(file); // current seek pointer position + FSIZE_t sz = (FSIZE_t) f_size(file); // current file size (end of file position) res = f_lseek(file, length); - if (res != FR_OK) { - _lock_release(&fat_ctx->lock); - ESP_LOGD(TAG, "%s: fresult=%d", __func__, res); - errno = fresult_to_errno(res); - ret = -1; - goto close; + if (res != FR_OK || f_tell(file) != length) { + goto lseek_or_write_fail; } - res = f_truncate(file); + if (sz < length) { + res = f_lseek(file, sz); // go to the previous end of file + if (res != FR_OK) { + goto lseek_or_write_fail; + } - if (res != FR_OK) { - _lock_release(&fat_ctx->lock); - ESP_LOGD(TAG, "%s: fresult=%d", __func__, res); - errno = fresult_to_errno(res); - ret = -1; - goto close; + FSIZE_t new_free_space = ((FSIZE_t) length) - sz; + UINT written; + + if (new_free_space > UINT32_MAX) { + _lock_release(&fat_ctx->lock); + ESP_LOGE(TAG, "%s: Cannot extend the file more than 4GB at once", __func__); + ret = -1; + goto close; + } + + FSIZE_t buf_size_limit = F_WRITE_MALLOC_ZEROING_BUF_SIZE_LIMIT; + FSIZE_t buf_size = new_free_space < buf_size_limit ? new_free_space : buf_size_limit; + res = f_write_zero_mem(file, new_free_space, buf_size, &written); + + if (res != FR_OK) { + goto lseek_or_write_fail; + } else if (written != (UINT) new_free_space) { + res = FR_DISK_ERR; + goto lseek_or_write_fail; + } + + res = f_lseek(file, seek_ptr_pos); // return to the original position + if (res != FR_OK) { + goto lseek_or_write_fail; + } + } else { + res = f_truncate(file); + + if (res != FR_OK) { + _lock_release(&fat_ctx->lock); + ESP_LOGD(TAG, "%s: fresult=%d", __func__, res); + errno = fresult_to_errno(res); + ret = -1; + goto close; + } } #if CONFIG_FATFS_IMMEDIATE_FSYNC @@ -1072,6 +1141,13 @@ close: out: free(file); return ret; + +lseek_or_write_fail: + _lock_release(&fat_ctx->lock); + ESP_LOGD(TAG, "%s: fresult=%d", __func__, res); + errno = fresult_to_errno(res); + ret = -1; + goto close; } static int vfs_fat_ftruncate(void* ctx, int fd, off_t length) @@ -1098,29 +1174,50 @@ static int vfs_fat_ftruncate(void* ctx, int fd, off_t length) goto out; } - long sz = f_size(file); - if (sz < length) { - ESP_LOGD(TAG, "ftruncate does not support extending size"); - errno = EPERM; - ret = -1; - goto out; - } + FSIZE_t seek_ptr_pos = (FSIZE_t) f_tell(file); // current seek pointer position + FSIZE_t sz = (FSIZE_t) f_size(file); // current file size (end of file position) res = f_lseek(file, length); - if (res != FR_OK) { - ESP_LOGD(TAG, "%s: fresult=%d", __func__, res); - errno = fresult_to_errno(res); - ret = -1; - goto out; + if (res != FR_OK || f_tell(file) != length) { + goto fail; } - res = f_truncate(file); + if (sz < length) { + res = f_lseek(file, sz); // go to the previous end of file + if (res != FR_OK) { + goto fail; + } - if (res != FR_OK) { - ESP_LOGD(TAG, "%s: fresult=%d", __func__, res); - errno = fresult_to_errno(res); - ret = -1; - goto out; + FSIZE_t new_free_space = ((FSIZE_t) length) - sz; + UINT written; + + if (new_free_space > UINT32_MAX) { + ESP_LOGE(TAG, "%s: Cannot extend the file more than 4GB at once", __func__); + ret = -1; + goto out; + } + + FSIZE_t buf_size_limit = F_WRITE_MALLOC_ZEROING_BUF_SIZE_LIMIT; + FSIZE_t buf_size = new_free_space < buf_size_limit ? new_free_space : buf_size_limit; + res = f_write_zero_mem(file, new_free_space, buf_size, &written); + + if (res != FR_OK) { + goto fail; + } else if (written != (UINT) new_free_space) { + res = FR_DISK_ERR; + goto fail; + } + + res = f_lseek(file, seek_ptr_pos); // return to the original position + if (res != FR_OK) { + goto fail; + } + } else { + res = f_truncate(file); + + if (res != FR_OK) { + goto fail; + } } #if CONFIG_FATFS_IMMEDIATE_FSYNC @@ -1135,6 +1232,12 @@ static int vfs_fat_ftruncate(void* ctx, int fd, off_t length) out: _lock_release(&fat_ctx->lock); return ret; + +fail: + ESP_LOGD(TAG, "%s: fresult=%d", __func__, res); + errno = fresult_to_errno(res); + ret = -1; + goto out; } static int vfs_fat_utime(void *ctx, const char *path, const struct utimbuf *times) diff --git a/components/vfs/include/esp_vfs.h b/components/vfs/include/esp_vfs.h index f3358d5076..f7cca42e71 100644 --- a/components/vfs/include/esp_vfs.h +++ b/components/vfs/include/esp_vfs.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -46,17 +46,17 @@ extern "C" { /** * Default value of flags member in esp_vfs_t structure. */ -#define ESP_VFS_FLAG_DEFAULT 0 +#define ESP_VFS_FLAG_DEFAULT (1 << 0) /** * Flag which indicates that FS needs extra context pointer in syscalls. */ -#define ESP_VFS_FLAG_CONTEXT_PTR 1 +#define ESP_VFS_FLAG_CONTEXT_PTR (1 << 1) /** * Flag which indicates that FS is located on read-only partition. */ -#define ESP_VFS_FLAG_READONLY_FS 2 +#define ESP_VFS_FLAG_READONLY_FS (1 << 2) /* * @brief VFS identificator used for esp_vfs_register_with_id()