From: Roland Dobai <dobai.roland@gmail.com>
Date: Fri, 9 Nov 2018 13:15:00 +0000 (+0100)
Subject: fatfs: Do not log from critical sections
X-Git-Tag: v3.3-beta1~106^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ad2cdeb476c86f678505c0782f142f2f331d0fe0;p=esp-idf

fatfs: Do not log from critical sections

Logging in the critical section can result in a deadlock when the logger
is redirected to FATFS.

Closes https://github.com/espressif/esp-idf/issues/1693
---

diff --git a/components/fatfs/src/vfs_fat.c b/components/fatfs/src/vfs_fat.c
index 9f139f6eb4..974e0651ac 100644
--- a/components/fatfs/src/vfs_fat.c
+++ b/components/fatfs/src/vfs_fat.c
@@ -307,18 +307,18 @@ static int vfs_fat_open(void* ctx, const char * path, int flags, int mode)
     prepend_drive_to_path(fat_ctx, &path, NULL);
     int fd = get_next_fd(fat_ctx);
     if (fd < 0) {
+        _lock_release(&fat_ctx->lock);
         ESP_LOGE(TAG, "open: no free file descriptors");
         errno = ENFILE;
-        fd = -1;
-        goto out;
+        return -1;
     }
     FRESULT res = f_open(&fat_ctx->files[fd], path, fat_mode_conv(flags));
     if (res != FR_OK) {
-        ESP_LOGD(TAG, "%s: fresult=%d", __func__, res);
         file_cleanup(fat_ctx, fd);
+        _lock_release(&fat_ctx->lock);
+        ESP_LOGD(TAG, "%s: fresult=%d", __func__, res);
         errno = fresult_to_errno(res);
-        fd = -1;
-        goto out;
+        return -1;
     }
     // O_APPEND need to be stored because it is not compatible with FA_OPEN_APPEND:
     //  - FA_OPEN_APPEND means to jump to the end of file only after open()
@@ -327,7 +327,6 @@ static int vfs_fat_open(void* ctx, const char * path, int flags, int mode)
     // therefore this flag is stored here (at this VFS level) in order to save
     // memory.
     fat_ctx->o_append[fd] = (flags & O_APPEND) == O_APPEND;
-out:
     _lock_release(&fat_ctx->lock);
     return fd;
 }
@@ -378,13 +377,13 @@ static int vfs_fat_fsync(void* ctx, int fd)
     _lock_acquire(&fat_ctx->lock);
     FIL* file = &fat_ctx->files[fd];
     FRESULT res = f_sync(file);
+    _lock_release(&fat_ctx->lock);
     int rc = 0;
     if (res != FR_OK) {
         ESP_LOGD(TAG, "%s: fresult=%d", __func__, res);
         errno = fresult_to_errno(res);
         rc = -1;
     }
-    _lock_release(&fat_ctx->lock);
     return rc;
 }
 
@@ -395,13 +394,13 @@ static int vfs_fat_close(void* ctx, int fd)
     FIL* file = &fat_ctx->files[fd];
     FRESULT res = f_close(file);
     file_cleanup(fat_ctx, fd);
+    _lock_release(&fat_ctx->lock);
     int rc = 0;
     if (res != FR_OK) {
         ESP_LOGD(TAG, "%s: fresult=%d", __func__, res);
         errno = fresult_to_errno(res);
         rc = -1;
     }
-    _lock_release(&fat_ctx->lock);
     return rc;
 }
 
@@ -517,12 +516,12 @@ static int vfs_fat_link(void* ctx, const char* n1, const char* n2)
     FIL* pf2 = calloc(1, sizeof(FIL));
     void* buf = malloc(copy_buf_size);
     if (buf == NULL || pf1 == NULL || pf2 == NULL) {
+        _lock_release(&fat_ctx->lock);
         ESP_LOGD(TAG, "alloc failed, pf1=%p, pf2=%p, buf=%p", pf1, pf2, buf);
         free(pf1);
         free(pf2);
         free(buf);
         errno = ENOMEM;
-        _lock_release(&fat_ctx->lock);
         return -1;
     }
     res = f_open(pf1, n1, FA_READ | FA_OPEN_EXISTING);
@@ -531,11 +530,10 @@ static int vfs_fat_link(void* ctx, const char* n1, const char* n2)
         goto fail1;
     }
     res = f_open(pf2, n2, FA_WRITE | FA_CREATE_NEW);
+    _lock_release(&fat_ctx->lock);
     if (res != FR_OK) {
-        _lock_release(&fat_ctx->lock);
         goto fail2;
     }
-    _lock_release(&fat_ctx->lock);
     size_t size_left = f_size(pf1);
     while (size_left > 0) {
         size_t will_copy = (size_left < copy_buf_size) ? size_left : copy_buf_size;
@@ -770,6 +768,7 @@ static int vfs_fat_truncate(void* ctx, const char *path, off_t length)
 
     file = (FIL*) calloc(1, sizeof(FIL));
     if (file == NULL) {
+        _lock_release(&fat_ctx->lock);
         ESP_LOGD(TAG, "truncate alloc failed");
         errno = ENOMEM;
         ret = -1;
@@ -779,6 +778,7 @@ static int vfs_fat_truncate(void* ctx, const char *path, off_t length)
     res = f_open(file, path, FA_WRITE);
 
     if (res != FR_OK) {
+        _lock_release(&fat_ctx->lock);
         ESP_LOGD(TAG, "%s: fresult=%d", __func__, res);
         errno = fresult_to_errno(res);
         ret = -1;
@@ -788,6 +788,7 @@ static int vfs_fat_truncate(void* ctx, const char *path, off_t length)
     res = f_size(file);
 
     if (res < length) {
+        _lock_release(&fat_ctx->lock);
         ESP_LOGD(TAG, "truncate does not support extending size");
         errno = EPERM;
         ret = -1;
@@ -796,6 +797,7 @@ static int vfs_fat_truncate(void* ctx, const char *path, off_t length)
 
     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;
@@ -803,6 +805,7 @@ static int vfs_fat_truncate(void* ctx, const char *path, off_t length)
     }
 
     res = f_truncate(file);
+    _lock_release(&fat_ctx->lock);
 
     if (res != FR_OK) {
         ESP_LOGD(TAG, "%s: fresult=%d", __func__, res);
@@ -823,7 +826,6 @@ close:
 
 out:
     free(file);
-    _lock_release(&fat_ctx->lock);
     return ret;
 }