Re: d_poll() inconsistencies
On 02/04/20(Thu) 20:25, Mark Kettenis wrote: > > Date: Thu, 2 Apr 2020 20:12:08 +0200 > > From: Martin Pieuchot > > Content-Type: text/plain; charset=utf-8 > > > > While reviewing the all current .d_poll() functions I found those two > > which are incoherent with the rest. > > > > - Most of the devices return POLLERR when the device is no longer valid, > > for whatever reason, uhid(4) returns POLLHUP in one of the cases. > > POSIX says: > > POLLHUP > > A device has been disconnected, or a pipe or FIFO has been closed by > the last process that had it open for writing. Once set, the hangup > state of a FIFO shall persist until some process opens the FIFO for > writing or until all read-only file descriptors for the FIFO are > closed. This event and POLLOUT are mutually-exclusive; a stream can > never be writable if a hangup has occurred. However, this event and > POLLIN, POLLRDNORM, POLLRDBAND, or POLLPRI are not > mutually-exclusive. This flag is only valid in the revents bitmask; > it shall be ignored in the events member. > > So POLLHUP makes sense for devices that can be disconnected. that's one interpretation. Now looking at the whole picture none of the other drivers that can be disconnected return POLLHUP. In fact they return POLLERR in that case. Since POLLERR is also valid in this case and it makes the whole drivers coherent, don't you see the benefit of this change? Or are you suggesting we change the other drivers to return POLLHUP in this case?
Re: d_poll() inconsistencies
> Date: Thu, 2 Apr 2020 20:12:08 +0200 > From: Martin Pieuchot > Content-Type: text/plain; charset=utf-8 > > While reviewing the all current .d_poll() functions I found those two > which are incoherent with the rest. > > - Most of the devices return POLLERR when the device is no longer valid, > for whatever reason, uhid(4) returns POLLHUP in one of the cases. POSIX says: POLLHUP A device has been disconnected, or a pipe or FIFO has been closed by the last process that had it open for writing. Once set, the hangup state of a FIFO shall persist until some process opens the FIFO for writing or until all read-only file descriptors for the FIFO are closed. This event and POLLOUT are mutually-exclusive; a stream can never be writable if a hangup has occurred. However, this event and POLLIN, POLLRDNORM, POLLRDBAND, or POLLPRI are not mutually-exclusive. This flag is only valid in the revents bitmask; it shall be ignored in the events member. So POLLHUP makes sense for devices that can be disconnected. > - fusepoll() return EINVAL which isn't a POLL* value, here again POLLERR > is what is wanted. This one is ok. > Index: dev/usb/uhid.c > === > RCS file: /cvs/src/sys/dev/usb/uhid.c,v > retrieving revision 1.77 > diff -u -p -r1.77 uhid.c > --- dev/usb/uhid.c20 Feb 2020 16:56:52 - 1.77 > +++ dev/usb/uhid.c2 Apr 2020 18:08:03 - > @@ -420,7 +420,7 @@ uhidpoll(dev_t dev, int events, struct p > return (POLLERR); > > if (usbd_is_dying(sc->sc_hdev.sc_udev)) > - return (POLLHUP); > + return (POLLERR); > > s = splusb(); > if (events & (POLLOUT | POLLWRNORM)) > Index: miscfs/fuse/fuse_device.c > === > RCS file: /cvs/src/sys/miscfs/fuse/fuse_device.c,v > retrieving revision 1.31 > diff -u -p -r1.31 fuse_device.c > --- miscfs/fuse/fuse_device.c 20 Feb 2020 16:56:52 - 1.31 > +++ miscfs/fuse/fuse_device.c 2 Apr 2020 18:08:03 - > @@ -523,7 +523,7 @@ fusepoll(dev_t dev, int events, struct p > > fd = fuse_lookup(minor(dev)); > if (fd == NULL) > - return (EINVAL); > + return (POLLERR); > > if (events & (POLLIN | POLLRDNORM)) > if (!SIMPLEQ_EMPTY(>fd_fbufs_in)) > >
d_poll() inconsistencies
While reviewing the all current .d_poll() functions I found those two which are incoherent with the rest. - Most of the devices return POLLERR when the device is no longer valid, for whatever reason, uhid(4) returns POLLHUP in one of the cases. - fusepoll() return EINVAL which isn't a POLL* value, here again POLLERR is what is wanted. ok? Index: dev/usb/uhid.c === RCS file: /cvs/src/sys/dev/usb/uhid.c,v retrieving revision 1.77 diff -u -p -r1.77 uhid.c --- dev/usb/uhid.c 20 Feb 2020 16:56:52 - 1.77 +++ dev/usb/uhid.c 2 Apr 2020 18:08:03 - @@ -420,7 +420,7 @@ uhidpoll(dev_t dev, int events, struct p return (POLLERR); if (usbd_is_dying(sc->sc_hdev.sc_udev)) - return (POLLHUP); + return (POLLERR); s = splusb(); if (events & (POLLOUT | POLLWRNORM)) Index: miscfs/fuse/fuse_device.c === RCS file: /cvs/src/sys/miscfs/fuse/fuse_device.c,v retrieving revision 1.31 diff -u -p -r1.31 fuse_device.c --- miscfs/fuse/fuse_device.c 20 Feb 2020 16:56:52 - 1.31 +++ miscfs/fuse/fuse_device.c 2 Apr 2020 18:08:03 - @@ -523,7 +523,7 @@ fusepoll(dev_t dev, int events, struct p fd = fuse_lookup(minor(dev)); if (fd == NULL) - return (EINVAL); + return (POLLERR); if (events & (POLLIN | POLLRDNORM)) if (!SIMPLEQ_EMPTY(>fd_fbufs_in))
