Re: [PATCH 10/36] fs: cleanup do_pollfd

Darrick J. Wong
2018-03-20T02:14:35Z

On Mon, Mar 05, 2018 at 01:27:17PM -0800, Christoph Hellwig wrote:
> Use straigline code with failure handling gotos instead of a lot
> of nested conditionals.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> fs/select.c | 48 +++++++++++++++++++++++-------------------------
> 1 file changed, 23 insertions(+), 25 deletions(-)
>
> diff --git a/fs/select.c b/fs/select.c
> index 686de7b3a1db..c6c504a814f9 100644
> --- a/fs/select.c
> +++ b/fs/select.c
> @@ -806,34 +806,32 @@ static inline __poll_t do_pollfd(struct pollfd *pollfd, poll_table *pwait,
> bool *can_busy_poll,
> __poll_t busy_flag)
> {
> - __poll_t mask;
> - int fd;
> -
> - mask = 0;
> - fd = pollfd->fd;
> - if (fd >= 0) {
> - struct fd f = fdget(fd);
> - mask = EPOLLNVAL;
> - if (f.file) {
> - /* userland u16 ->events contains POLL... bitmap */
> - __poll_t filter = demangle_poll(pollfd->events) |
> - EPOLLERR | EPOLLHUP;
> - mask = DEFAULT_POLLMASK;
> - if (f.file->f_op->poll) {
> - pwait->_key = filter;
> - pwait->_key |= busy_flag;
> - mask = f.file->f_op->poll(f.file, pwait);
> - if (mask & busy_flag)
> - *can_busy_poll = true;
> - }
> - /* Mask out unneeded events. */
> - mask &= filter;
> - fdput(f);
> - }
> + int fd = pollfd->fd;
> + __poll_t mask = 0, filter;
> + struct fd f;
> +
> + if (fd < 0)
> + goto out;
> + mask = EPOLLNVAL;
> + f = fdget(fd);
> + if (!f.file)
> + goto out;
> +
> + /* userland u16 ->events contains POLL... bitmap */
> + filter = demangle_poll(pollfd->events) | EPOLLERR | EPOLLHUP;
> + mask = DEFAULT_POLLMASK;
> + if (f.file->f_op->poll) {
> + pwait->_key = filter | busy_flag;
> + mask = f.file->f_op->poll(f.file, pwait);
> + if (mask & busy_flag)
> + *can_busy_poll = true;
> }
> + mask &= filter; /* Mask out unneeded events. */
> + fdput(f);
> +
> +out:
> /* ... and so does ->revents */
> pollfd->revents = mangle_poll(mask);
> -
> return mask;
> }
>
> --
> 2.14.2
>