]> granicus.if.org Git - esp-idf/commitdiff
fatfs: Do not log from critical sections
authorRoland Dobai <dobai.roland@gmail.com>
Fri, 9 Nov 2018 13:15:00 +0000 (14:15 +0100)
committerRoland Dobai <dobai.roland@gmail.com>
Mon, 26 Nov 2018 08:21:44 +0000 (09:21 +0100)
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

components/fatfs/src/vfs_fat.c

index 6d7019a1e25b3f6b83e17de7ffef9ec6e696067c..e4f63fbffa60f8ce90227d0b8e5a1a7c4b4960b9 100644 (file)
@@ -305,18 +305,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()
@@ -325,7 +325,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;
 }
@@ -376,13 +375,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;
 }
 
@@ -393,13 +392,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;
 }
 
@@ -515,12 +514,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);
@@ -529,11 +528,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;
@@ -768,6 +766,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;
@@ -777,6 +776,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;
@@ -786,6 +786,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;
@@ -794,6 +795,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;
@@ -801,6 +803,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);
@@ -821,6 +824,5 @@ close:
 
 out:
     free(file);
-    _lock_release(&fat_ctx->lock);
     return ret;
 }