From 539262b5c2f186154092d727863dc39cb0c2ceee Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Tue, 10 Oct 2017 16:11:12 +1100 Subject: [PATCH] vfs: Remove fd_offset member This was intended for integrating LWIP, but a different approach was used. --- components/lwip/port/vfs_lwip.c | 1 - components/vfs/README.rst | 49 ++++++++++++++------------------ components/vfs/include/esp_vfs.h | 8 ++---- components/vfs/vfs.c | 7 ++--- components/vfs/vfs_uart.c | 1 - 5 files changed, 28 insertions(+), 38 deletions(-) diff --git a/components/lwip/port/vfs_lwip.c b/components/lwip/port/vfs_lwip.c index b1f064c33..b3c626192 100644 --- a/components/lwip/port/vfs_lwip.c +++ b/components/lwip/port/vfs_lwip.c @@ -45,7 +45,6 @@ static int lwip_ioctl_r_wrapper(int fd, int cmd, va_list args); void esp_vfs_lwip_sockets_register() { esp_vfs_t vfs = { - .fd_offset = 0, .flags = ESP_VFS_FLAG_DEFAULT | ESP_VFS_FLAG_SHARED_FD_SPACE, .write = &lwip_write_r, .open = NULL, diff --git a/components/vfs/README.rst b/components/vfs/README.rst index 943705539..2b2a6bc9f 100644 --- a/components/vfs/README.rst +++ b/components/vfs/README.rst @@ -22,7 +22,6 @@ To register an FS driver, application needs to define in instance of esp_vfs_t s :: esp_vfs_t myfs = { - .fd_offset = 0, .flags = ESP_VFS_FLAG_DEFAULT, .write = &myfs_write, .open = &myfs_open, @@ -43,7 +42,7 @@ Case 1: API functions are declared without an extra context pointer (FS driver i .flags = ESP_VFS_FLAG_DEFAULT, .write = &myfs_write, // ... other members initialized - + // When registering FS, context pointer (third argument) is NULL: ESP_ERROR_CHECK(esp_vfs_register("/data", &myfs, NULL)); @@ -55,7 +54,7 @@ Case 2: API functions are declared with an extra context pointer (FS driver supp .flags = ESP_VFS_FLAG_CONTEXT_PTR, .write_p = &myfs_write, // ... other members initialized - + // When registering FS, pass the FS context pointer into the third argument // (hypothetical myfs_mount function is used for illustrative purposes) myfs_t* myfs_inst1 = myfs_mount(partition1->offset, partition1->size); @@ -100,37 +99,34 @@ File descriptors It is suggested that filesystem drivers should use small positive integers as file descriptors. VFS component assumes that ``CONFIG_MAX_FD_BITS`` bits (12 by default) are sufficient to represent a file descriptor. -If filesystem is configured with an option to offset all file descriptors by a constant value, such value should be passed to ``fd_offset`` field of ``esp_vfs_t`` structure. VFS component will then remove this offset when working with FDs of that specific FS, bringing them into the range of small positive integers. +While file descriptors returned by VFS component to newlib library are rarely seen by the application, the following details may be useful for debugging purposes. File descriptors returned by VFS component are composed of two parts: FS driver ID, and the actual file descriptor. Because newlib stores file descriptors as 16-bit integers, VFS component is also limited by 16 bits to store both parts. -While file descriptors returned by VFS component to newlib library are rarely seen by the application, the following details may be useful for debugging purposes. File descriptors returned by VFS component are composed of two parts: FS driver ID, and the actual file descriptor. Because newlib stores file descriptors as 16-bit integers, VFS component is also limited by 16 bits to store both parts. +Lower ``CONFIG_MAX_FD_BITS`` bits are used to store zero-based file descriptor. The per-filesystem FD obtained from the FS ``open`` call, and this result is stored in the lower bits of the FD. Higher bits are used to save the index of FS in the internal table of registered filesystems. -Lower ``CONFIG_MAX_FD_BITS`` bits are used to store zero-based file descriptor. If FS driver has a non-zero ``fd_offset`` field, this ``fd_offset`` is subtracted FDs obtained from the FS ``open`` call, and the result is stored in the lower bits of the FD. Higher bits are used to save the index of FS in the internal table of registered filesystems. - -When VFS component receives a call from newlib which has a file descriptor, this file descriptor is translated back to the FS-specific file descriptor. First, higher bits of FD are used to identify the FS. Then ``fd_offset`` field of the FS is added to the lower ``CONFIG_MAX_FD_BITS`` bits of the fd, and resulting FD is passed to the FS driver. +When VFS component receives a call from newlib which has a file descriptor, this file descriptor is translated back to the FS-specific file descriptor. First, higher bits of FD are used to identify the FS. Then only the lower ``CONFIG_MAX_FD_BITS`` bits of the fd are masked in, and resulting FD is passed to the FS driver. .. highlight:: none :: FD as seen by newlib FD as seen by FS driver - +-----+ - +-------+---------------+ | | +------------------------+ - | FS id | Zero—based FD | +---------------> sum +----> | - +---+---+------+--------+ | | | +------------------------+ - | | | +--^--+ - | +--------------+ | - | | - | +-------------+ | - | | Table of | | - | | registered | | - | | filesystems | | - | +-------------+ +-------------+ | - +-------> entry +----> esp_vfs_t | | - index +-------------+ | structure | | - | | | | | - | | | + fd_offset +---+ - +-------------+ | | - +-------------+ + + +-------+---------------+ +------------------------+ + | FS id | Zero—based FD | +--------------------------> | + +---+---+------+--------+ | +------------------------+ + | | | + | +--------------+ + | + | +-------------+ + | | Table of | + | | registered | + | | filesystems | + | +-------------+ +-------------+ + +-------> entry +----> esp_vfs_t | + index +-------------+ | structure | + | | | | + | | | | + +-------------+ +-------------+ Standard IO streams (stdin, stdout, stderr) @@ -170,4 +166,3 @@ Such a design has the following consequences: - It is possible to set ``stdin``, ``stdout``, and ``stderr`` for any given task without affecting other tasks, e.g. by doing ``stdin = fopen("/dev/uart/1", "r")``. - Closing default ``stdin``, ``stdout``, or ``stderr`` using ``fclose`` will close the ``FILE`` stream object — this will affect all other tasks. - To change the default ``stdin``, ``stdout``, ``stderr`` streams for new tasks, modify ``_GLOBAL_REENT->_stdin`` (``_stdout``, ``_stderr``) before creating the task. - diff --git a/components/vfs/include/esp_vfs.h b/components/vfs/include/esp_vfs.h index 6a8ed3306..bd1cf322c 100644 --- a/components/vfs/include/esp_vfs.h +++ b/components/vfs/include/esp_vfs.h @@ -64,10 +64,9 @@ extern "C" { * This structure should be filled with pointers to corresponding * FS driver functions. * - * If the FS implementation has an option to use certain offset for - * all file descriptors, this value should be passed into fd_offset - * field. Otherwise VFS component will translate all FDs to start - * at zero offset. + * VFS component will translate all FDs so that the filesystem implementation + * sees them starting at zero. The caller sees a global FD which is prefixed + * with an pre-filesystem-implementation. * * Some FS implementations expect some state (e.g. pointer to some structure) * to be passed in as a first argument. For these implementations, @@ -82,7 +81,6 @@ extern "C" { */ typedef struct { - int fd_offset; /*!< file descriptor offset, determined by the FS driver */ int flags; /*!< ESP_VFS_FLAG_CONTEXT_PTR or ESP_VFS_FLAG_DEFAULT, plus optionally ESP_VFS_FLAG_SHARED_FD_SPACE */ union { ssize_t (*write_p)(void* p, int fd, const void * data, size_t size); diff --git a/components/vfs/vfs.c b/components/vfs/vfs.c index f3624259b..24fde2e30 100644 --- a/components/vfs/vfs.c +++ b/components/vfs/vfs.c @@ -140,9 +140,9 @@ static const vfs_entry_t* get_vfs_for_fd(int fd) static int translate_fd(const vfs_entry_t* vfs, int fd) { if (vfs->vfs.flags & ESP_VFS_FLAG_SHARED_FD_SPACE) { - return fd + vfs->vfs.fd_offset; + return fd; } else { - return (fd & VFS_FD_MASK) + vfs->vfs.fd_offset; + return fd & VFS_FD_MASK; } } @@ -255,8 +255,7 @@ int esp_vfs_open(struct _reent *r, const char * path, int flags, int mode) if (ret < 0) { return ret; } - assert(ret >= vfs->vfs.fd_offset); - return ret - vfs->vfs.fd_offset + (vfs->offset << VFS_INDEX_S); + return ret + (vfs->offset << VFS_INDEX_S); } ssize_t esp_vfs_write(struct _reent *r, int fd, const void * data, size_t size) diff --git a/components/vfs/vfs_uart.c b/components/vfs/vfs_uart.c index e477e0e84..f958278d2 100644 --- a/components/vfs/vfs_uart.c +++ b/components/vfs/vfs_uart.c @@ -263,7 +263,6 @@ static int uart_fcntl(int fd, int cmd, va_list args) void esp_vfs_dev_uart_register() { esp_vfs_t vfs = { - .fd_offset = 0, .flags = ESP_VFS_FLAG_DEFAULT, .write = &uart_write, .open = &uart_open,