stdio: a layered / multiheaded approach to I/O

[miri64]

Description

We need to rethink our STDIO model! With new methods for STDIO emerging (stdio_cdc_acm, stdio_nimble, ...) and usage of modules that multiplex STDIOs with other functionality (e.g. ethos or slipdev to multiplex network connections via serial) getting more and more complicated to configure, while only being able to just talk to one specific lower-end implementation of STDIO (stdio_uart in ethos's case) this becomes clearer and clearer. After some brainstorming with @haukepetersen and @kaspar030 I decided to write down our initial ideas to be build upon and maybe to find someone™ to implement it in the end.

Problems with the current model

The current model maps STDIO system calls to the functions stdio_read() and stdio_write(), these can be implemented by a stdio_* module to provide the desired functionality. The problem is that this only allows for exactly one STDIO implementation at a time. This leads to problems if this use case isn't given anymore.

Use cases

There are several use cases that call for a new model.

1. Mapping a more advanced STDIO on top of a more basic STDIO: e.g. multiplexing STDIO with other functionality such as in ethos.
2. Having multiple STDIO interfaces: e.g. stdio_cdc_acm as main interface for user interaction but also providing stdio_uart for debugging (in case stdio_cdc_acm is unable to be initialized).
3. Having no STDIO: This is currently modeled as a module with empty operations called stdio_null. However, maybe we find a way to not necessitate it at all.

Model proposal

My initial gut / keep-it-simple feeling would call for a driver-based approach. That means that a new STDIO implementation would provide a struct with function pointers similar as we did it elsewhere (e.g. in netdev).

typedef struct {
    ssize_t (*read)(void *buffer, size_t max_len);
    ssize_t (*write)(const void *buffer, size_t len);
} stdio_t;

Additionally a module dependent init-function is provided.
A top-level implementation (under the current model that would be the equivalent to stdio_read()/stdio_write()) would then call a stdio_t object that is provided via configuration.

static const stdio_t *_stdio;

void stdio_init(const stdio_t *stdio)
{
    _stdio = stdio;
}

ssize_t stdio_read(void *buffer, size_t max_len)
{
     return _stdio->read(buffer, max_len)
}

ssize_t stdio_write(const void *buffer, size_t len)
{
     return _stdio->write(buffer, max_len)
}

This should IMHO cover at least use case 1 and 2 (please understand the code snippets more as pseudo-code than actual implementation ideas, I don't think it is as easy as I make it seem here):

1. Initialize the more advanced STDIO with the more basic STDIO and use the advanced STDIO in the top-level implementation, e.g.

   const stdio_t *stdio = stdio_uart_init(uart_config, baudrate);
   stdio = ethos_init(stdio, ...);
   stdio_init(stdio);

2. Provide a multiplexer STDIO to call multiple other STDIOs

   const stdio_t *stdios[] = {
       uart,
       cdc_acm,
       NULL,
   }
   const stdio_t *stdio = stdio_multiplex_init(stdios);
   stdio_init(stdio);

Use case 3 still would be a NOP module, but we could use some compile-time config to make at least stdio_read() and stdio_write() also NOPs in case no other STDIO module is present.

For the more graphically minded people here is a somewhat thrown together UML diagram of what I am thinking of (showing both the cases shown in 1. and 2. as a model)

More considerations

• Provide support for different STDIOs to different file descriptors / module-wise STDIO? (stdio: a layered / multiheaded approach to I/O #13469 (comment))
• Provide a separate module list (e.g. STDIO_MODULES) for STDIO modules (Makefile.dep: disable stdio_% modules before they are included #13650 (comment))
• Have there no stdio module if stdio is not needed (Makefile.dep: disable stdio_% modules before they are included #13650 (comment))

Useful links

Related issues, PRs, and discussions:

• sys/stdio_nimble: add new stdio module using nimble #12012
• boards/arduino-mkr: feather-m0: sodaq-*: provide stdio over USB and setup automatic flash with bossa #12304
• stdio_semihosting: Initial include of Semihosting-based STDIO  #13320
• Makefile.dep: DEFAULT_MODULE += stdio_rtt will always load dependencies - even if other stdio is selected #13460
• Makefile.dep: disable stdio_% modules before they are included #13650
• sys/net/application_layer: add telnet server module & example #16723
• UART-ish-over-CoAP protocol proposal in the forum

[kaspar030]

Nice sum-up! The ops look pretty much like the vfs_file_ops. Let's see if we can re-use some of that code.

Maybe refactor vfs a bit, so the mounting / actual FS support becomes optional, and it can be used with only the file descriptor handling. That in turn could then become default, dropping the current non-vfs use case.

[kaspar030]

Maybe refactor vfs a bit, so the mounting / actual FS support becomes optional, and it can be used with only the file descriptor handling. That in turn could then become default, dropping the current non-vfs use case.

An alternative could be to factor file descriptor table / handling out of VFS.

[miri64]

[…] The ops look pretty much like the vfs_file_ops. Let's see if we can re-use some of that code.

Not quite sure where you see the similarity (except that both are a struct of function pointers...). Compared to my proposal the vfs_file_ops struct is quite large.

RIOT/sys/include/vfs.h

Lines 258 to 373 in ab333f3

	struct vfs_file_ops {
	/**
	* @brief Close an open file
	*
	* This function must perform any necessary clean ups and flush any internal
	* buffers in the file system driver.
	*
	* If an error occurs, the file will still be considered closed by the VFS
	* layer. Therefore, the proper clean up must still be performed by the file
	* system driver before returning any error code.
	*
	* @note This implementation does not consider @c -EINTR a special return code,
	* the file is still considered closed.
	*
	* @param[in] filp pointer to open file
	*
	* @return 0 on success
	* @return <0 on error, the file is considered closed anyway
	*/
	int (*close) (vfs_file_t *filp);
	/**
	* @brief Query/set options on an open file
	*
	* @param[in] filp pointer to open file
	* @param[in] cmd fcntl command, see man 3p fcntl
	* @param[in] arg argument to fcntl command, see man 3p fcntl
	*
	* @return 0 on success
	* @return <0 on error
	*/
	int (*fcntl) (vfs_file_t *filp, int cmd, int arg);
	/**
	* @brief Get status of an open file
	*
	* @param[in] filp pointer to open file
	* @param[out] buf pointer to stat struct to fill
	*
	* @return 0 on success
	* @return <0 on error
	*/
	int (*fstat) (vfs_file_t *filp, struct stat *buf);
	/**
	* @brief Seek to position in file
	*
	* @p whence determines the function of the seek and should be set to one of
	* the following values:
	*
	* - @c SEEK_SET: Seek to absolute offset @p off
	* - @c SEEK_CUR: Seek to current location + @p off
	* - @c SEEK_END: Seek to end of file + @p off
	*
	* @param[in] filp pointer to open file
	* @param[in] off seek offset
	* @param[in] whence determines the seek method, see detailed description
	*
	* @return the new seek location in the file on success
	* @return <0 on error
	*/
	off_t (*lseek) (vfs_file_t *filp, off_t off, int whence);
	/**
	* @brief Attempt to open a file in the file system at rel_path
	*
	* A file system driver should perform the necessary checks for file
	* existence etc in this function.
	*
	* The VFS layer will initialize the contents of @p *filp so that
	* @c filp->f_op points to the mounted file system's @c vfs_file_ops_t.
	* @c filp->private_data.ptr will be initialized to NULL, @c filp->pos will
	* be set to 0.
	*
	* @note @p name is an absolute path inside the file system, @p abs_path is
	* the path to the file in the VFS, example:
	* @p abs_path = "/mnt/hd/foo/bar", @p name = "/foo/bar"
	*
	* @note @p name and @p abs_path may point to different locations within the
	* same const char array and the strings may overlap
	*
	* @param[in] filp pointer to open file
	* @param[in] name null-terminated name of the file to open, relative to the file system root, including a leading slash
	* @param[in] flags flags for opening, see man 2 open, man 3p open
	* @param[in] mode mode for creating a new file, see man 2 open, man 3p open
	* @param[in] abs_path null-terminated name of the file to open, relative to the VFS root ("/")
	*
	* @return 0 on success
	* @return <0 on error
	*/
	int (*open) (vfs_file_t *filp, const char *name, int flags, mode_t mode, const char *abs_path);
	/**
	* @brief Read bytes from an open file
	*
	* @param[in] filp pointer to open file
	* @param[in] dest pointer to destination buffer
	* @param[in] nbytes maximum number of bytes to read
	*
	* @return number of bytes read on success
	* @return <0 on error
	*/
	ssize_t (*read) (vfs_file_t *filp, void *dest, size_t nbytes);
	/**
	* @brief Write bytes to an open file
	*
	* @param[in] filp pointer to open file
	* @param[in] src pointer to source buffer
	* @param[in] nbytes maximum number of bytes to write
	*
	* @return number of bytes written on success
	* @return <0 on error
	*/
	ssize_t (*write) (vfs_file_t *filp, const void *src, size_t nbytes);
	};

[kaspar030]

Not quite sure where you see the similarity (except that both are a struct of function pointers...). Compared to my proposal the vfs_file_ops struct is quite large.

Yeah you're right. I just looked at this, which made is seem similar.

RIOT/sys/vfs/vfs_stdio.c

Lines 53 to 56 in e787224

	static vfs_file_ops_t _stdio_ops = {
	.read = _stdio_read,
	.write = _stdio_write,
	};

[miri64]

Thought as much, because for a second while writing OP I thought the same ;-). But quickly remembered "No the rest is just initialized with NULL" ^^