Hello all,

This is the vote for the breaking change in
https://github.com/apache/nuttx/pull/20796, as CONTRIBUTING.md 1.13
step 5 requires. The [DISCUSS] thread "struct pollfd with the POSIX
layout (breaking change)" got no objection.

The change:
struct pollfd gets the POSIX layout: int fd, short events, short
revents (8 bytes). Today it is 40 bytes on a 64-bit target and 24 bytes
on a 32-bit target. The kernel fields (arg, cb, priv) and the 32-bit
event masks move to a new kernel type, struct pollfd_s, in
include/nuttx/poll.h. poll() copies the entries into kernel memory and
copies revents back. select(), ppoll(), pselect(), epoll and the system
call interface do not change.

Reference: <poll.h> in IEEE Std 1003.1-2017 / The Open Group Base
Specifications Issue 7, 2018 edition:
https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/basedefs/poll.h.html

Why it is necessary:
- A program or library that uses the standard layout (for example the
  Rust libc crate) gets its neighbouring entries overwritten by poll().
- In flat and protected builds, the kernel writes a callback pointer
  into the user array and calls it from poll_notify(), often in an
  interrupt handler. In a protected build user code can change that
  pointer.
- Two threads cannot poll the same array. On the ESP32-S3 this crashes
  the board today (store to address 0 in pipecommon_poll()).

Non-breaking alternative: there is none. POSIX defines struct pollfd
with these three fields only, so a program that uses the standard type
cannot get a correct result while the NuttX type keeps its extra fields.
Keeping the old layout under a new name keeps the problem.

Quick fix:
- Applications: rebuild. Code that uses only fd, events and revents
  needs no change. Print events and revents with %x, not PRIx32
  (nuttx-apps #3878, merged).
- Out-of-tree drivers: rename struct pollfd to struct pollfd_s in the
  poll methods, the waiter arrays and the poll callbacks, and include
  <nuttx/poll.h> if the file does not include <nuttx/fs/fs.h>. The
  compiler reports every place that is not renamed.

Tests: build and runtime logs on real hardware (RP2350, Cortex-M33, and
ESP32-S3, Xtensa LX7) are in the PR, with ostest and a test of the
layout, of poll() on standard arrays, and of two threads on one array.

Cost: +168 bytes of code (flat build) and +48 bytes (kernel build) on
qemu-armv8a; the stack of poll() grows by 160 bytes (64-bit) or 96
bytes (32-bit).

The vote is open for at least 72 hours.

[ ] +1 Accept the change
[ ] 0  No opinion
[ ] -1 Reject the change, because...

NuttX PMC members cast binding votes. The change needs at least 4
binding +1 votes and no binding -1 vote.

My vote: +1

Regards,
Marco Casaroli

Reply via email to