gregory-nutt commented on code in PR #10602: URL: https://github.com/apache/nuttx/pull/10602#discussion_r1324554592
########## libs/libc/stdio/lib_fopencookie.c: ########## @@ -0,0 +1,229 @@ +/**************************************************************************** + * libs/libc/stdio/lib_fopencookie.c + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <fcntl.h> +#include <string.h> +#include <assert.h> +#include <errno.h> + +#include "libc.h" + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct cookie_s +{ + void *cookie; /* pointer to the caller's cookie struct */ + cookie_io_functions_t cookie_io; /* programmer-defined hook functions */ + int cookie_fd; /* file descriptor */ +}; + +static ssize_t freadcookie(FAR struct file *filep, FAR char *buf, + size_t nbytes); +static ssize_t fwritecookie(FAR struct file *filep, FAR const char *buf, + size_t nbytes); +static off_t fseekcookie(FAR struct file *filep, off_t offset, int whence); +static int fclosecookie(FAR struct file *filep); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static const struct file_operations cookie_fops = +{ + NULL, /* open */ + fclosecookie, /* close */ + freadcookie, /* read */ + fwritecookie, /* write */ + fseekcookie, /* seek */ + NULL, /* ioctl */ + NULL, /* mmap */ + NULL, /* truncate */ + NULL, /* poll */ +}; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +static ssize_t freadcookie(FAR struct file *filep, FAR char *buf, + size_t nbytes) +{ + struct cookie_s *cookie = (struct cookie_s *)filep->f_priv; + int ret; + + DEBUGASSERT(cookie->cookie != NULL && cookie->cookie_io.read != NULL); + + ret = cookie->cookie_io.read(cookie->cookie, buf, nbytes); + return ret; +} + +static ssize_t fwritecookie(FAR struct file *filep, FAR const char *buf, + size_t nbytes) +{ + struct cookie_s *cookie = (struct cookie_s *)filep->f_priv; + int ret; + + DEBUGASSERT(cookie->cookie != NULL && cookie->cookie_io.write != NULL); + + ret = cookie->cookie_io.write(cookie->cookie, (FAR const char *)buf, + nbytes); + return ret; +} + +static off_t fseekcookie(FAR struct file *filep, off_t offset, int whence) +{ + struct cookie_s *cookie = (struct cookie_s *)filep->f_priv; + int ret; + + DEBUGASSERT(cookie->cookie != NULL && cookie->cookie_io.seek != NULL); + + ret = cookie->cookie_io.seek(cookie->cookie, &offset, whence); + return ret; +} + +static int fclosecookie(FAR struct file *filep) +{ + struct cookie_s *cookie = (struct cookie_s *)filep->f_priv; + int ret; + + DEBUGASSERT(cookie->cookie != NULL && cookie->cookie_io.close != NULL); + + ret = cookie->cookie_io.close(cookie->cookie); + + free(filep->f_priv); + free(filep->f_inode); + close(cookie->cookie_fd); + + return ret; +} + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: fopencookie + ****************************************************************************/ + +FAR FILE *fopencookie(void *restrict cookie, const char *restrict mode, + cookie_io_functions_t io_funcs) +{ + FAR FILE *filestructp = NULL; + FAR struct file filep; + FAR struct inode *inode; + FAR struct cookie_s *cookie_priv; + int oflags; + int ret; + int fd; + + /* Map the open mode string to open flags */ + + oflags = lib_mode2oflags(mode); + if (oflags < 0) + { + return NULL; + } + + /* Allocate necessary structures. */ + + inode = lib_zalloc(sizeof(struct inode)); + if (inode == NULL) + { + set_errno(-ENOMEM); + goto fopencookie_return; + } + + cookie_priv = lib_zalloc(sizeof(struct cookie_s)); + if (cookie_priv == NULL) + { + free(inode); + set_errno(-ENOMEM); + goto fopencookie_return; + } + + memset(&filep, 0, sizeof(filep)); + + /* Fill cookie_priv structure. We need to add cookie provided by user + * and functions providd by user. + */ + + cookie_priv->cookie = cookie; + cookie_priv->cookie_io.read = io_funcs.read; + cookie_priv->cookie_io.write = io_funcs.write; + cookie_priv->cookie_io.seek = io_funcs.seek; + cookie_priv->cookie_io.close = io_funcs.close; + + /* Add file operations */ + + inode->u.i_ops = &cookie_fops; + + filep.f_oflags = oflags; + filep.f_inode = inode; + filep.f_priv = cookie_priv; + + /* And get file descriptor. */ + + fd = file_allocate_from_tcb(nxsched_self(), filep.f_inode, filep.f_oflags, Review Comment: > Ok, you implementation reuse file_operation_s infrastructure, but it mayn't work in protected/kernel mode, since fclosecookie... is part of kernel, but call the callback provided by userspace directly. I would suggest that you have to add these callback to file_struct and call them from libc/stdio directly. > > @patacongo what are you thinking? _[I have not looked at this particular PR, but I can give my usual response when callbacks from the OS are suggested]_ Direct callbacks from the OS into user code have always been forbidden. I don't think we should permit that now. Some justifications: - This is not the POSIX way to do things. There is no way to do such things under any POSIX OS. We have always been concerned about POSIX compliant and portability. Linux code is often ported to NuttX. NuttX code should be runnable on any any POSIX OS as well (with the appropriate drivers). To accomplish these goals, callbacks must not be used. - The callback would probably be running on an kernel stack and not the user stack. In that case, TLS data on the stack would not be accessible. The callback were asynchronous with the tasking. A callback is not a context switch; the interrupt user task is still in place and the OS thinks that is that task that is running. So any OS interactions in this bastardized state could likely be fatal. - The callback would run at whatever the priority of the calling logic is running at. This will break real time behavior which must be strict priority-based scheduling. There are several differ build modes for NuttX: The FLAT build, the PROTECTED build for better security, and the KERNEL build for full support of processes much like Linux. In the PROTECTED and KERNEL builds, there are more significant technical issues. There is some documentation of at least the PROTECTED build here: https://cwiki.apache.org/confluence/display/NUTTX/NuttX+Protected+Build - In the PROTECTED and KERNEL build modes, the kernel runs in supervisor mode in a supervisor address space while all user applications run in non-privileged user mode in a user address space. In the PROTECTED mode, you _could_ call from the kernel supervisor code into user code but this would, however, be a gaping security hole and, hence, must never be done. Potentially hostile user-supplied code must never run in supervisor mode in this build. - The KERNEL build differs in that each _process_ has its own address environment and, in this build, the system will crash if such a thing is attempted. That is because the notification arrives asynchronously with tasking; you never know which process is running and you would call into the wrong address space. This could be fixed be changing the address environment prior to each callback, that would have big performance and complexity issues. It is better to do things in the POSIX way. So even if you use only the FLAT build, direct callbacks from the OS into the user code must be prohibited. What is the POSIX way of doing an aynchronous notification? Signals. In a POSIX OS, you should use standard POSIX signaling and avoid even thinking about callbacks. Drivers under drivers/ that do asynchronous notifications use signals for this purpose. Consider drivers/input/button_upper.c. This is a simple button driver that sends a notification to a user task when a button is pressed or release: - Look at the function btn_ioctl(). The BTNIOC_REGISTER command is used to register a notification. The process ID of the user thread is provided and retained for the notification. - When the button press event occurs, the notification signal is sent in btn_sample(). - A signal handler in the user thread that registered for the notification can then run, in the correct task context, in user mode, and at the correct priority assigned to the task. This is 100% POSIX and works in all build modes. It could even be implemented under any other POSIX OS such as Linux. Signals are only one option. Below I mention usrsock as a better model to follow. In that case, I believe that it uses a character driver in kernel space to interact with the user-space socket implementationi. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
