Merge pull request #8615 from gebart/pr/fatfs-paths

pkg/fatfs: Refactor path handling, minor fixes

pkg/fatfs/fatfs_vfs/fatfs_vfs.c

@@ -36,6 +36,17 @@
 static int fatfs_err_to_errno(int32_t err);
 static void _fatfs_time_to_timespec(WORD fdate, WORD ftime, time_t *time);
 
+/**
+ * @brief Concatenate drive number and path into the buffer provided by fs_desc
+ *
+ * Most FatFs library file operations need an absolute path.
+ */
+static void _build_abs_path(fatfs_desc_t *fs_desc, const char *name)
+{
+    snprintf(fs_desc->abs_path_str_buff, FATFS_MAX_ABS_PATH_SIZE, "%u:/%s",
+             fs_desc->vol_idx, name);
+}
+
 static int _mount(vfs_mount_t *mountp)
 {
     /* if one of the lines below fail to compile you probably need to adjust
@@ -45,13 +56,12 @@ static int _mount(vfs_mount_t *mountp)
 
     fatfs_desc_t *fs_desc = (fatfs_desc_t *)mountp->private_data;
 
-    char volume_str[FATFS_MAX_VOL_STR_LEN];
+    _build_abs_path(fs_desc, "");
-    snprintf(volume_str, sizeof(volume_str), "%d:/", fs_desc->vol_idx);
 
     memset(&fs_desc->fat_fs, 0, sizeof(fs_desc->fat_fs));
 
-    DEBUG("mounting file system of volume '%s'\n", volume_str);
+    DEBUG("mounting file system of volume '%s'\n", fs_desc->abs_path_str_buff);
-    FRESULT res = f_mount(&fs_desc->fat_fs, volume_str, 1);
+    FRESULT res = f_mount(&fs_desc->fat_fs, fs_desc->abs_path_str_buff, 1);
 
     if (res == FR_OK) {
         DEBUG("[OK]");
@@ -69,11 +79,10 @@ static int _umount(vfs_mount_t *mountp)
 
     DEBUG("fatfs_vfs.c: _umount: private_data = %p\n", mountp->private_data);
 
-    char volume_str[FATFS_MAX_VOL_STR_LEN];
+    _build_abs_path(fs_desc, "");
-    snprintf(volume_str, sizeof(volume_str), "%d:/", fs_desc->vol_idx);
 
-    DEBUG("unmounting file system of volume '%s'\n", volume_str);
+    DEBUG("unmounting file system of volume '%s'\n", fs_desc->abs_path_str_buff);
-    FRESULT res = f_unmount(volume_str);
+    FRESULT res = f_unmount(fs_desc->abs_path_str_buff);
 
     if (res == FR_OK) {
         DEBUG("[OK]");
@@ -90,8 +99,7 @@ static int _unlink(vfs_mount_t *mountp, const char *name)
 {
     fatfs_desc_t *fs_desc = (fatfs_desc_t *)mountp->private_data;
 
-    snprintf(fs_desc->abs_path_str_buff, FATFS_MAX_ABS_PATH_SIZE, "%d:/%s",
+    _build_abs_path(fs_desc, name);
-             fs_desc->vol_idx, name);
 
     return fatfs_err_to_errno(f_unlink(fs_desc->abs_path_str_buff));
 }
@@ -102,10 +110,9 @@ static int _rename(vfs_mount_t *mountp, const char *from_path,
     char fatfs_abs_path_to[FATFS_MAX_ABS_PATH_SIZE];
     fatfs_desc_t *fs_desc = (fatfs_desc_t *)mountp->private_data;
 
-    snprintf(fs_desc->abs_path_str_buff, FATFS_MAX_ABS_PATH_SIZE, "%d:/%s",
+    _build_abs_path(fs_desc, from_path);
-             fs_desc->vol_idx, from_path);
 
-    snprintf(fatfs_abs_path_to, sizeof(fatfs_abs_path_to), "%d:/%s",
+    snprintf(fatfs_abs_path_to, sizeof(fatfs_abs_path_to), "%u:/%s",
              fs_desc->vol_idx, to_path);
 
     return fatfs_err_to_errno(f_rename(fs_desc->abs_path_str_buff,
@@ -117,8 +124,7 @@ static int _open(vfs_file_t *filp, const char *name, int flags, mode_t mode,
 {
     fatfs_file_desc_t *fd = (fatfs_file_desc_t *)&filp->private_data.buffer[0];
     fatfs_desc_t *fs_desc = (fatfs_desc_t *)filp->mp->private_data;
-    snprintf(fs_desc->abs_path_str_buff, FATFS_MAX_ABS_PATH_SIZE, "%d:/%s",
+    _build_abs_path(fs_desc, name);
-             fs_desc->vol_idx, name);
 
     (void) abs_path;
     (void) mode; /* fatfs can't use mode param with f_open*/
@@ -251,8 +257,7 @@ static int _fstat(vfs_file_t *filp, struct stat *buf)
     FILINFO fi;
     FRESULT res;
 
-    snprintf(fs_desc->abs_path_str_buff, FATFS_MAX_ABS_PATH_SIZE, "%d:/%s",
+    _build_abs_path(fs_desc, fd->fname);
-             fs_desc->vol_idx, fd->fname);
 
     memset(buf, 0, sizeof(*buf));
 
@@ -295,8 +300,7 @@ static int _opendir(vfs_DIR *dirp, const char *dirname, const char *abs_path)
     fatfs_desc_t *fs_desc = (fatfs_desc_t *)dirp->mp->private_data;
     (void) abs_path;
 
-    snprintf(fs_desc->abs_path_str_buff, FATFS_MAX_ABS_PATH_SIZE, "%d:/%s",
+    _build_abs_path(fs_desc, dirname);
-             fs_desc->vol_idx, dirname);
 
     return fatfs_err_to_errno(f_opendir(dir, fs_desc->abs_path_str_buff));
 }
@@ -334,8 +338,7 @@ static int _mkdir (vfs_mount_t *mountp, const char *name, mode_t mode)
     fatfs_desc_t *fs_desc = (fatfs_desc_t *)mountp->private_data;
     (void) mode;
 
-    snprintf(fs_desc->abs_path_str_buff, FATFS_MAX_ABS_PATH_SIZE, "%d:/%s",
+    _build_abs_path(fs_desc, name);
-             fs_desc->vol_idx, name);
 
     return fatfs_err_to_errno(f_mkdir(fs_desc->abs_path_str_buff));
 }
@@ -344,8 +347,7 @@ static int _rmdir (vfs_mount_t *mountp, const char *name)
 {
     fatfs_desc_t *fs_desc = (fatfs_desc_t *)mountp->private_data;
 
-    snprintf(fs_desc->abs_path_str_buff, FATFS_MAX_ABS_PATH_SIZE, "%d:/%s",
+    _build_abs_path(fs_desc, name);
-             fs_desc->vol_idx, name);
 
     return fatfs_err_to_errno(f_unlink(fs_desc->abs_path_str_buff));
 }

sys/include/fs/fatfs.h

@@ -35,35 +35,22 @@ extern "C" {
 /** The epoch offset is used to convert between FatFs and time_t timestamps */
 #define EPOCH_YEAR_OFFSET (1970)
 
-/** Size of the buffer needed for a directory entry -> @attention this should be: sizeof(DIR).
-    sizeof(DIR) currently isn't used directly because it's not possible to use that within
-    preprocessor-if (see below) */
-#define FATFS_DIR_SIZE (44)
-
-/** Size of the buffer needed for directory
-    -> should be: sizeof(fatfs_file_desc_t) */
-#define FATFS_FILE_SIZE (72)
-
 /** size needed for volume strings like "n:/" where n is the volume id */
-#define FATFS_MAX_VOL_STR_LEN (4)
+#define FATFS_MAX_VOL_STR_LEN (6)
 
 /** 0:mount on first file access, 1 mount in f_mount() call */
 #define FATFS_MOUNT_OPT       (1)
 
-/** most FatFs file operations need an absolute path. This defines the size of the
+/**
-    needed buffer to circumvent stack allocation within vfs-wrappers */
+ * @brief Size of path buffer for absolute paths
+ *
+ * Most FatFs file operations need an absolute path. This defines the size of
+ * the needed buffer to circumvent stack allocation within vfs-wrappers
+ */
 #define FATFS_MAX_ABS_PATH_SIZE (FATFS_MAX_VOL_STR_LEN + VFS_NAME_MAX + 1)
 
-#if (VFS_DIR_BUFFER_SIZE < FATFS_DIR_SIZE)
-#error "VFS_DIR_BUFFER_SIZE too small"
-#endif
-
-#if (VFS_FILE_BUFFER_SIZE < FATFS_FILE_SIZE)
-#error "VFS_FILE_BUFFER_SIZE too small"
-#endif
-
 /**
- * needed info to run a FatFs instance
+ * @brief FatFs instance descriptor
  */
 typedef struct fatfs_desc {
     FATFS fat_fs;       /**< FatFs work area needed for each volume */
@@ -75,7 +62,7 @@ typedef struct fatfs_desc {
 } fatfs_desc_t;
 
 /**
- * info of a single opened file
+ * @brief FatFs file instance descriptor
  */
 typedef struct fatfs_file_desc {
     FIL file;                     /**< FatFs work area for a single file */

tests/pkg_fatfs/README.md

@@ -3,15 +3,25 @@ Using FatFs on RIOT
 
 # native
 
-To use this test on native you can either use a FAT-formatted image file or directly use the mkfs command from the RIOT shell.
+To use this test on native you can either use a FAT-formatted image file or
-Use `make image` to extract a prepared image file that already contains a simple test.txt file.
+directly use the mkfs command from the RIOT shell. Use `make image` to extract
-This is only a convinience function to allow testing against a "default linux" formatted fat volume without the need to call mount or other stuff that may require super user privileges.
+a prepared image file that already contains a simple test.txt file. This is
-Optionally `make compressed-image` can be used to generate the compressed image that is in turn used by `make image`.
+only a convinience function to allow testing against a "default linux"
+formatted fat volume without the need to call mount or other stuff that may
+require super user privileges. Optionally `make compressed-image` can be used
+to generate the compressed image that is in turn used by `make image`.
 
-To tell RIOT where your image file is located you can use the define `MTD_NATIVE_FILENAME`.
+To tell RIOT where your image file is located you can use the define
+`MTD_NATIVE_FILENAME`.
 
-	NOTE: You shouldn't leave the image mounted while you use it in RIOT, the abstraction layer between FatFs and the image file mimics a dumb block device (i.e. behaves much like the devices that are actually meant to be used with FAT) That implies it doesn't show any modifications in RIOT that you perform on your OS and the other way round. So always remember to mount/unmount correctly or your FS will probably get damaged.
+NOTE: You shouldn't leave the image mounted while you use it in RIOT, the
+abstraction layer between FatFs and the image file mimics a dumb block device
+(i.e. behaves much like the devices that are actually meant to be used with
+FAT) That implies it doesn't show any modifications in RIOT that you perform on
+your OS and the other way round. So always remember to mount/unmount correctly
+or your FS will probably get damaged.
 
 # Real Hardware
 
-Currently the test defaults to sdcard_spi on real hardware. But generally any device that supports the mtd-interface can be used with FatFs.
+Currently the test defaults to sdcard_spi on real hardware. But generally any
+device that supports the mtd-interface can be used with FatFs.

tests/pkg_fatfs_vfs/README.md

@@ -3,18 +3,30 @@ Using FatFs (with VFS) on RIOT
 
 # native
 
-To use this test on native you can either use a FAT-formatted image file or directly use the mkfs command from the RIOT shell.
+To use this test on native you can either use a FAT-formatted image file or
-Use `make image` to extract a prepared image file that already contains a simple test.txt file.
+directly use the mkfs command from the RIOT shell. Use `make image` to extract
-This is only a convinience function to allow testing against a "default linux" formatted fat volume without the need to call mount or other stuff that may require super user privileges.
+a prepared image file that already contains a simple test.txt file. This is
-Optionally `make compressed-image` can be used to generate the compressed image that is in turn used by `make image`.
+only a convinience function to allow testing against a "default linux"
+formatted fat volume without the need to call mount or other stuff that may
+require super user privileges. Optionally `make compressed-image` can be used
+to generate the compressed image that is in turn used by `make image`.
 
-To tell RIOT where your image file is located you can use the define `MTD_NATIVE_FILENAME`.
+To tell RIOT where your image file is located you can use the define
+`MTD_NATIVE_FILENAME`.
 
-	NOTE: You shouldn't leave the image mounted while you use it in RIOT, the abstraction layer between FatFs and the image file mimics a dumb block device (i.e. behaves much like the devices that are actually meant to be used with FAT) That implies it doesn't show any modifications in RIOT that you perform on your OS and the other way round. So always remember to mount/unmount correctly or your FS will probably get damaged.
+NOTE: You shouldn't leave the image mounted while you use it in RIOT, the
+abstraction layer between FatFs and the image file mimics a dumb block device
+(i.e. behaves much like the devices that are actually meant to be used with
+FAT) That implies it doesn't show any modifications in RIOT that you perform on
+your OS and the other way round. So always remember to mount/unmount correctly
+or your FS will probably get damaged.
 
 # Real Hardware
 
-Currently the test defaults to sdcard_spi on real hardware. But generally any device that supports the mtd-interface can be used with FatFs.
+Currently the test defaults to sdcard_spi on real hardware. But generally any
-To use the automated test in pkg_fatfs_vfs you need to copy the generated image to your storage device (e.g. your SD-card).
+device that supports the mtd-interface can be used with FatFs. To use the
-To copy the image onto the card you can use something like `make image && dd if=bin/riot_fatfs_disk.img of=/dev/<your_sdcard>`.
+automated test in pkg_fatfs_vfs you need to copy the generated image to your
-After that you can connect the card to your RIOT device and check the test output via terminal.
+storage device (e.g. your SD-card). To copy the image onto the card you can use
+something like `make image && dd if=bin/riot_fatfs_disk.img
+of=/dev/<your_sdcard>`. After that you can connect the card to your RIOT device
+and check the test output via terminal.