This is an automated email from the ASF dual-hosted git repository. acassis pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 67f10d91a0d59961f3050344c2f9fd7f67d71388 Author: Marco Casaroli <[email protected]> AuthorDate: Mon Oct 19 00:34:16 2026 +0200 !fs/poll: Give struct pollfd the POSIX layout. struct pollfd had 32-bit events and revents and three kernel fields (arg, cb and priv): 40 bytes on a 64-bit target, 24 bytes on a 32-bit target. POSIX gives it fd, short events and short revents: <poll.h> in IEEE Std 1003.1-2017 (Revision of IEEE Std 1003.1-2008) / The Open Group Base Specifications Issue 7, 2018 edition. A program that builds its own array with the standard layout and calls poll() gets its neighbouring entries overwritten. struct pollfd now has only the three POSIX fields. poll() copies fd and events of each entry into a struct pollfd_s array, on the stack for up to 4 entries and on the fs heap for more, and copies revents back, also when the wait is interrupted. The cancellation cleanup frees the heap copy. This also stops the kernel from keeping its poll callback and semaphore in the caller's array in flat and protected builds, where poll_notify() called a function pointer that user code could change. Two threads can now poll the same array. BREAKING CHANGE: struct pollfd has the POSIX layout (int fd, short events, short revents) and no NuttX fields. Quick fix: rebuild the applications; code that uses only fd, events and revents needs no change. Print events and revents with %x, not PRIx32. Out-of-tree drivers use struct pollfd_s from nuttx/poll.h in their poll methods, waiter arrays and poll callbacks. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Marco Casaroli <[email protected]> --- fs/vfs/fs_poll.c | 88 ++++++++++++++++++++++++++++-------------------------- include/sys/poll.h | 28 ++++------------- 2 files changed, 51 insertions(+), 65 deletions(-) diff --git a/fs/vfs/fs_poll.c b/fs/vfs/fs_poll.c index fbd5fe3bb36..11d369a0d91 100644 --- a/fs/vfs/fs_poll.c +++ b/fs/vfs/fs_poll.c @@ -44,6 +44,14 @@ #include "inode/inode.h" #include "fs_heap.h" +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/* poll() keeps the kernel copy of up to this many entries on the stack */ + +#define POLL_NSTACKFDS 4 + /**************************************************************************** * Private Types ****************************************************************************/ @@ -52,6 +60,7 @@ struct pollinfo_s { FAR struct pollfd_s *fds; nfds_t nfds; + bool heap; }; /**************************************************************************** @@ -135,14 +144,7 @@ static inline int poll_setup(FAR struct pollfd_s *fds, nfds_t nfds, for (i = 0; i < nfds; i++) { - /* Setup the poll descriptor - * - * REVISIT: In a multi-threaded environment, one use case might be to - * share a single, array of struct pollfd in poll() calls on different - * threads. That use case is not supportable here because the non- - * standard internal fields get reset here on each call to poll() - * on each thread. - */ + /* Setup the poll descriptor */ fds[i].arg = sem; fds[i].cb = poll_default_cb; @@ -213,6 +215,10 @@ static void poll_cleanup(FAR void *arg) int count; poll_teardown(fdsinfo->fds, fdsinfo->nfds, &count); + if (fdsinfo->heap) + { + fs_heap_free(fdsinfo->fds); + } } /**************************************************************************** @@ -416,10 +422,13 @@ int file_poll(FAR struct file *filep, FAR struct pollfd_s *fds, bool setup) int poll(FAR struct pollfd *fds, nfds_t nfds, int timeout) { + struct pollfd_s sfds[POLL_NSTACKFDS]; + struct pollinfo_s fdsinfo; FAR struct pollfd_s *kfds; sem_t sem; int count = 0; int ret = OK; + nfds_t i; DEBUGASSERT(nfds == 0 || fds != NULL); @@ -427,28 +436,29 @@ int poll(FAR struct pollfd *fds, nfds_t nfds, int timeout) enter_cancellation_point(); -#ifdef CONFIG_BUILD_KERNEL - /* Allocate kernel memory for the fds */ + /* Copy the user fds to a kernel array, which also holds the internal + * fields of each entry. + */ - kfds = fs_heap_malloc(nfds * sizeof(struct pollfd_s)); - if (!kfds) + if (nfds <= POLL_NSTACKFDS) { - /* Out of memory */ - - ret = -ENOMEM; - goto out_with_cancelpt; + kfds = sfds; + } + else + { + kfds = fs_heap_malloc(nfds * sizeof(struct pollfd_s)); + if (kfds == NULL) + { + ret = -ENOMEM; + goto out_with_cancelpt; + } } - /* Copy the user fds to neutral kernel memory */ - - memcpy(kfds, fds, nfds * sizeof(struct pollfd)); -#else - /* Can use the user fds directly: struct pollfd has the layout of struct - * pollfd_s. - */ - - kfds = (FAR struct pollfd_s *)fds; -#endif + for (i = 0; i < nfds; i++) + { + kfds[i].fd = fds[i].fd; + kfds[i].events = fds[i].events; + } /* Set up the poll structure */ @@ -461,14 +471,13 @@ int poll(FAR struct pollfd *fds, nfds_t nfds, int timeout) count = poll_setup(kfds, nfds, &sem); if (count == 0) { - struct pollinfo_s fdsinfo; - /* Push a cancellation point onto the stack. This will be called if * the thread is canceled. */ - fdsinfo.fds = kfds; + fdsinfo.fds = kfds; fdsinfo.nfds = nfds; + fdsinfo.heap = kfds != sfds; tls_cleanup_push(tls_get_info(), poll_cleanup, &fdsinfo); if (timeout > 0) @@ -525,26 +534,19 @@ int poll(FAR struct pollfd *fds, nfds_t nfds, int timeout) nxsem_destroy(&sem); -#ifdef CONFIG_BUILD_KERNEL - /* Copy the events back to user */ + /* Copy the events back to the user */ - if (ret == OK) + for (i = 0; i < nfds; i++) { - int i; - - for (i = 0; i < nfds; i++) - { - fds[i].revents = kfds[i].revents; - } + fds[i].revents = kfds[i].revents; } - /* Free the temporary buffer */ - - fs_heap_free(kfds); + if (kfds != sfds) + { + fs_heap_free(kfds); + } out_with_cancelpt: -#endif - leave_cancellation_point(); if (ret < 0) diff --git a/include/sys/poll.h b/include/sys/poll.h index b2175959be8..0f7fa4d095c 100644 --- a/include/sys/poll.h +++ b/include/sys/poll.h @@ -88,32 +88,16 @@ typedef unsigned int nfds_t; -/* This is the NuttX variant of the standard pollfd structure. The poll() - * interfaces receive a variable length array of such structures. - * - * REVISIT: In a multi-threaded environment, one use case might be to share - * a single, array of struct pollfd in poll calls on different threads. - * That use case is not supportable with this variant due way in which the - * non-standard internal fields are used in the implementation of poll(). +/* The standard pollfd structure. The poll() interfaces receive a variable + * length array of such structures. The kernel copies each entry into its + * own struct pollfd_s (see nuttx/poll.h). */ -struct pollfd_s; - struct pollfd { - /* Standard fields */ - - int fd; /* The descriptor being polled */ - uint32_t events; /* The input event flags */ - uint32_t revents; /* The output event flags */ - - /* Non-standard fields used internally by NuttX. They have the layout of - * struct pollfd_s in nuttx/poll.h. - */ - - FAR void *arg; /* The poll callback function argument */ - CODE void (*cb)(FAR struct pollfd_s *fds); - FAR void *priv; /* For use by drivers */ + int fd; /* The descriptor being polled */ + short events; /* The input event flags */ + short revents; /* The output event flags */ }; /****************************************************************************
