From 93ffc009ef8f1ed75731094e2e113539a020aa44 Mon Sep 17 00:00:00 2001
From: Felipe Neves <felipe.neves@espressif.com>
Date: Mon, 26 Oct 2020 16:06:12 -0300
Subject: [PATCH] vfs: restrict the fast seek for read-only files

Since the files under fast-seek cannot be expanded with further writes, it does not
make sense to enable fast-seek which may fail in write-mode files
---
 components/fatfs/test/test_fatfs_common.c | 18 +++++++-
 components/fatfs/vfs/vfs_fat.c            | 52 +++++++++++++----------
 tools/ci/config/target-test.yml           |  4 +-
 3 files changed, 49 insertions(+), 25 deletions(-)

diff --git a/components/fatfs/test/test_fatfs_common.c b/components/fatfs/test/test_fatfs_common.c
index c1dd3f1319..342d55a4fd 100644
--- a/components/fatfs/test/test_fatfs_common.c
+++ b/components/fatfs/test/test_fatfs_common.c
@@ -217,11 +217,27 @@ void test_fatfs_lseek(const char* filename)
     TEST_ASSERT_EQUAL(18, ftell(f));
     TEST_ASSERT_EQUAL(0, fseek(f, 0, SEEK_SET));
     char buf[20];
+
     TEST_ASSERT_EQUAL(18, fread(buf, 1, sizeof(buf), f));
     const char ref_buf[] = "0123456789\n\0\0\0abc\n";
     TEST_ASSERT_EQUAL_INT8_ARRAY(ref_buf, buf, sizeof(ref_buf) - 1);
-
     TEST_ASSERT_EQUAL(0, fclose(f));
+
+#ifdef CONFIG_FATFS_USE_FASTSEEK
+    f = fopen(filename, "rb+");
+    TEST_ASSERT_NOT_NULL(f);
+    TEST_ASSERT_EQUAL(0, fseek(f, 0, SEEK_END));
+    TEST_ASSERT_EQUAL(18, ftell(f));
+    TEST_ASSERT_EQUAL(0, fseek(f, -4, SEEK_CUR));
+    TEST_ASSERT_EQUAL(14, ftell(f));
+    TEST_ASSERT_EQUAL(0, fseek(f, -14, SEEK_CUR));
+    TEST_ASSERT_EQUAL(0, ftell(f));
+
+    TEST_ASSERT_EQUAL(18, fread(buf, 1, sizeof(buf), f));
+    TEST_ASSERT_EQUAL_INT8_ARRAY(ref_buf, buf, sizeof(ref_buf) - 1);
+    TEST_ASSERT_EQUAL(0, fclose(f));
+#endif
+
 }
 
 void test_fatfs_truncate_file(const char* filename)
diff --git a/components/fatfs/vfs/vfs_fat.c b/components/fatfs/vfs/vfs_fat.c
index 34de1bad94..54bc1194b9 100644
--- a/components/fatfs/vfs/vfs_fat.c
+++ b/components/fatfs/vfs/vfs_fat.c
@@ -319,29 +319,35 @@ static int vfs_fat_open(void* ctx, const char * path, int flags, int mode)
 
 #ifdef CONFIG_FATFS_USE_FASTSEEK
     FIL* file = &fat_ctx->files[fd];
-    DWORD *clmt_mem =  ff_memalloc(sizeof(DWORD) * CONFIG_FATFS_FAST_SEEK_BUFFER_SIZE);
-    if (clmt_mem == NULL) {
-        f_close(file);
-        file_cleanup(fat_ctx, fd);
-        _lock_release(&fat_ctx->lock);
-        ESP_LOGE(TAG, "open: Failed to pre-allocate CLMT buffer for fast-seek");
-        errno = ENOMEM;
-        return -1;
-    } 
+    //fast-seek is only allowed in read mode, since file cannot be expanded
+    //to use it.
+    if(!(fat_mode_conv(flags) & (FA_WRITE))) {
+        DWORD *clmt_mem =  ff_memalloc(sizeof(DWORD) * CONFIG_FATFS_FAST_SEEK_BUFFER_SIZE);
+        if (clmt_mem == NULL) {
+            f_close(file);
+            file_cleanup(fat_ctx, fd);
+            _lock_release(&fat_ctx->lock);
+            ESP_LOGE(TAG, "open: Failed to pre-allocate CLMT buffer for fast-seek");
+            errno = ENOMEM;
+            return -1;
+        } 
 
-    file->cltbl = clmt_mem;
-    file->cltbl[0] = CONFIG_FATFS_FAST_SEEK_BUFFER_SIZE;
-    res = f_lseek(file, CREATE_LINKMAP);
-    ESP_LOGD(TAG, "%s: fast-seek has: %s",
-            __func__,
-            (res == FR_OK) ? "activated" : "failed");
-    if(res != FR_OK) {
-        ESP_LOGW(TAG, "%s: fast-seek not activated reason code: %d",
-                __func__, res);
-        //If linkmap creation fails, fallback to the non fast seek.
-        ff_memfree(file->cltbl);
+        file->cltbl = clmt_mem;
+        file->cltbl[0] = CONFIG_FATFS_FAST_SEEK_BUFFER_SIZE;
+        res = f_lseek(file, CREATE_LINKMAP);
+        ESP_LOGD(TAG, "%s: fast-seek has: %s",
+                __func__,
+                (res == FR_OK) ? "activated" : "failed");
+        if(res != FR_OK) {
+            ESP_LOGW(TAG, "%s: fast-seek not activated reason code: %d",
+                    __func__, res);
+            //If linkmap creation fails, fallback to the non fast seek.
+            ff_memfree(file->cltbl);
+            file->cltbl = NULL;
+        }
+    } else {
         file->cltbl = NULL;
-    }
+    } 
 #endif
 
     // O_APPEND need to be stored because it is not compatible with FA_OPEN_APPEND:
@@ -404,6 +410,7 @@ static ssize_t vfs_fat_pread(void *ctx, int fd, void *dst, size_t size, off_t of
     const off_t prev_pos = f_tell(file);
 
     FRESULT f_res = f_lseek(file, offset);
+
     if (f_res != FR_OK) {
         ESP_LOGD(TAG, "%s: fresult=%d", __func__, f_res);
         errno = fresult_to_errno(f_res);
@@ -443,6 +450,7 @@ static ssize_t vfs_fat_pwrite(void *ctx, int fd, const void *src, size_t size, o
     const off_t prev_pos = f_tell(file);
 
     FRESULT f_res = f_lseek(file, offset);
+
     if (f_res != FR_OK) {
         ESP_LOGD(TAG, "%s: fresult=%d", __func__, f_res);
         errno = fresult_to_errno(f_res);
@@ -497,6 +505,7 @@ static int vfs_fat_close(void* ctx, int fd)
 
 #ifdef CONFIG_FATFS_USE_FASTSEEK
     ff_memfree(file->cltbl);
+    file->cltbl = NULL;
 #endif
 
     FRESULT res = f_close(file);
@@ -530,7 +539,6 @@ static off_t vfs_fat_lseek(void* ctx, int fd, off_t offset, int mode)
     }
 
     ESP_LOGD(TAG, "%s: offset=%ld, filesize:=%d", __func__, new_pos, f_size(file));
-
     FRESULT res = f_lseek(file, new_pos);
     if (res != FR_OK) {
         ESP_LOGD(TAG, "%s: fresult=%d", __func__, res);
diff --git a/tools/ci/config/target-test.yml b/tools/ci/config/target-test.yml
index e0a9c30225..6e330cc475 100644
--- a/tools/ci/config/target-test.yml
+++ b/tools/ci/config/target-test.yml
@@ -328,7 +328,7 @@ component_ut_test_001:
 
 UT_001:
   extends: .unit_test_template
-  parallel: 43
+  parallel: 44
   tags:
     - ESP32_IDF
     - UT_T1_1
@@ -480,7 +480,7 @@ UT_034:
 
 UT_035:
   extends: .unit_test_s2_template
-  parallel: 44
+  parallel: 45
   tags:
     - ESP32S2_IDF
     - UT_T1_1