Date: Tue, 1 Sep 2009 20:58:41 +0000 (UTC) From: Jilles Tjoelker <jilles@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r196741 - in stable/8: sys sys/amd64/include/xen sys/cddl/contrib/opensolaris sys/contrib/dev/acpica sys/contrib/pf sys/dev/xen/xenpci sys/fs/fifofs sys/kern tools/regression/poll Message-ID: <200909012058.n81KwfSk072165@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jilles Date: Tue Sep 1 20:58:41 2009 New Revision: 196741 URL: http://svn.freebsd.org/changeset/base/196741 Log: MFC r196460 Fix the conformance of poll(2) for sockets after r195423 by returning POLLHUP instead of POLLIN for several cases. Now, the tools/regression/poll results for FreeBSD are closer to that of the Solaris and Linux. Also, improve the POSIX conformance by explicitely clearing POLLOUT when POLLHUP is reported in pollscan(), making the fix global. Submitted by: bde Reviewed by: rwatson MFC r196556 Fix poll() on half-closed sockets, while retaining POLLHUP for fifos. This reverts part of r196460, so that sockets only return POLLHUP if both directions are closed/error. Fifos get POLLHUP by closing the unused direction immediately after creating the sockets. The tools/regression/poll/*poll.c tests now pass except for two other things: - if POLLHUP is returned, POLLIN is always returned as well instead of only when there is data left in the buffer to be read - fifo old/new reader distinction does not work the way POSIX specs it Reviewed by: kib, bde MFC r196554 Add some tests for poll(2)/shutdown(2) interaction. Approved by: re (kensmith) Added: stable/8/tools/regression/poll/sockpoll.c - copied unchanged from r196554, head/tools/regression/poll/sockpoll.c Modified: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/xen/xenpci/ (props changed) stable/8/sys/fs/fifofs/fifo_vnops.c stable/8/sys/kern/sys_generic.c stable/8/tools/regression/poll/ (props changed) stable/8/tools/regression/poll/Makefile Modified: stable/8/sys/fs/fifofs/fifo_vnops.c ============================================================================== --- stable/8/sys/fs/fifofs/fifo_vnops.c Tue Sep 1 18:30:17 2009 (r196740) +++ stable/8/sys/fs/fifofs/fifo_vnops.c Tue Sep 1 20:58:41 2009 (r196741) @@ -193,6 +193,9 @@ fifo_open(ap) goto fail2; fip->fi_writesock = wso; error = soconnect2(wso, rso); + /* Close the direction we do not use, so we can get POLLHUP. */ + if (error == 0) + error = soshutdown(rso, SHUT_WR); if (error) { (void)soclose(wso); fail2: Modified: stable/8/sys/kern/sys_generic.c ============================================================================== --- stable/8/sys/kern/sys_generic.c Tue Sep 1 18:30:17 2009 (r196740) +++ stable/8/sys/kern/sys_generic.c Tue Sep 1 20:58:41 2009 (r196741) @@ -1228,6 +1228,13 @@ pollscan(td, fds, nfd) selfdalloc(td, fds); fds->revents = fo_poll(fp, fds->events, td->td_ucred, td); + /* + * POSIX requires POLLOUT to be never + * set simultaneously with POLLHUP. + */ + if ((fds->revents & POLLHUP) != 0) + fds->revents &= ~POLLOUT; + if (fds->revents != 0) n++; } Modified: stable/8/tools/regression/poll/Makefile ============================================================================== --- stable/8/tools/regression/poll/Makefile Tue Sep 1 18:30:17 2009 (r196740) +++ stable/8/tools/regression/poll/Makefile Tue Sep 1 20:58:41 2009 (r196741) @@ -3,14 +3,15 @@ # Nothing yet works with gmake for the path to the sources. .PATH: .. -PROG= pipepoll pipeselect +PROG= pipepoll pipeselect sockpoll CFLAGS+= -Werror -Wall all: ${PROG} pipepoll: pipepoll.c pipeselect: pipeselect.c +sockpoll: sockpoll.c -pipepoll pipeselect: +pipepoll pipeselect sockpoll: ${CC} ${CFLAGS} ${LDFLAGS} -o $@ $@.c test: all Copied: stable/8/tools/regression/poll/sockpoll.c (from r196554, head/tools/regression/poll/sockpoll.c) ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ stable/8/tools/regression/poll/sockpoll.c Tue Sep 1 20:58:41 2009 (r196741, copy of r196554, head/tools/regression/poll/sockpoll.c) @@ -0,0 +1,202 @@ +/* $FreeBSD$ */ + +#include <sys/poll.h> +#include <sys/socket.h> +#include <sys/stat.h> + +#include <err.h> +#include <fcntl.h> +#include <signal.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> + +static const char * +decode_events(int events) +{ + char *ncresult; + const char *result; + + switch (events) { + case POLLIN: + result = "POLLIN"; + break; + case POLLOUT: + result = "POLLOUT"; + break; + case POLLIN | POLLOUT: + result = "POLLIN | POLLOUT"; + break; + case POLLHUP: + result = "POLLHUP"; + break; + case POLLIN | POLLHUP: + result = "POLLIN | POLLHUP"; + break; + case POLLOUT | POLLHUP: + result = "POLLOUT | POLLHUP"; + break; + case POLLIN | POLLOUT | POLLHUP: + result = "POLLIN | POLLOUT | POLLHUP"; + break; + default: + asprintf(&ncresult, "%#x", events); + result = ncresult; + break; + } + return (result); +} + +static void +report(int num, const char *state, int expected, int got) +{ + if (expected == got) + printf("ok %-2d ", num); + else + printf("not ok %-2d", num); + printf(" state %s: expected %s; got %s\n", + state, decode_events(expected), decode_events(got)); + fflush(stdout); +} + +static int +set_nonblocking(int sck) +{ + int flags; + + flags = fcntl(sck, F_GETFL, 0); + flags |= O_NONBLOCK; + + if (fcntl(sck, F_SETFL, flags)) + return -1; + + return 0; +} + +static char largeblock[1048576]; /* should be more than AF_UNIX sockbuf size */ +static int fd[2]; +static struct pollfd pfd0; +static struct pollfd pfd1; + +void +setup(void) +{ + if (socketpair(AF_UNIX, SOCK_STREAM, 0, fd) != 0) + err(1, "socketpair"); + if (set_nonblocking(fd[0]) == -1) + err(1, "fcntl"); + if (set_nonblocking(fd[1]) == -1) + err(1, "fcntl"); + pfd0.fd = fd[0]; + pfd0.events = POLLIN | POLLOUT; + pfd1.fd = fd[1]; + pfd1.events = POLLIN | POLLOUT; +} + +int +main(void) +{ + int num; + + num = 1; + printf("1..18\n"); + fflush(stdout); + + /* Large write with close */ + setup(); + if (poll(&pfd0, 1, 0) == -1) + err(1, "poll"); + report(num++, "initial 0", POLLOUT, pfd0.revents); + if (poll(&pfd1, 1, 0) == -1) + err(1, "poll"); + report(num++, "initial 1", POLLOUT, pfd1.revents); + if (write(fd[0], largeblock, sizeof(largeblock)) == -1) + err(1, "write"); + if (poll(&pfd0, 1, 0) == -1) + err(1, "poll"); + report(num++, "after large write", 0, pfd0.revents); + if (poll(&pfd1, 1, 0) == -1) + err(1, "poll"); + report(num++, "other side after large write", POLLIN | POLLOUT, pfd1.revents); + close(fd[0]); + if (poll(&pfd1, 1, 0) == -1) + err(1, "poll"); + report(num++, "other side after close", POLLIN | POLLHUP, pfd1.revents); + if (read(fd[1], largeblock, sizeof(largeblock)) == -1) + err(1, "read"); + if (poll(&pfd1, 1, 0) == -1) + err(1, "poll"); + report(num++, "other side after reading input", POLLHUP, pfd1.revents); + close(fd[1]); + + /* With shutdown(SHUT_WR) */ + setup(); + if (shutdown(fd[0], SHUT_WR) == -1) + err(1, "shutdown"); + if (poll(&pfd0, 1, 0) == -1) + err(1, "poll"); + report(num++, "after shutdown(SHUT_WR)", POLLOUT, pfd0.revents); + if (poll(&pfd1, 1, 0) == -1) + err(1, "poll"); + report(num++, "other side after shutdown(SHUT_WR)", POLLIN | POLLOUT, pfd1.revents); + switch (read(fd[1], largeblock, sizeof(largeblock))) { + case 0: + break; + case -1: + err(1, "read after other side shutdown"); + break; + default: + errx(1, "kernel made up data that was never written"); + } + if (poll(&pfd1, 1, 0) == -1) + err(1, "poll"); + report(num++, "other side after reading EOF", POLLIN | POLLOUT, pfd1.revents); + if (write(fd[1], largeblock, sizeof(largeblock)) == -1) + err(1, "write"); + if (poll(&pfd0, 1, 0) == -1) + err(1, "poll"); + report(num++, "after data from other side", POLLIN | POLLOUT, pfd0.revents); + if (poll(&pfd1, 1, 0) == -1) + err(1, "poll"); + report(num++, "after writing", POLLIN, pfd1.revents); + if (shutdown(fd[1], SHUT_WR) == -1) + err(1, "shutdown second"); + if (poll(&pfd0, 1, 0) == -1) + err(1, "poll"); + report(num++, "after second shutdown", POLLIN | POLLHUP, pfd0.revents); + if (poll(&pfd1, 1, 0) == -1) + err(1, "poll"); + report(num++, "after second shutdown", POLLHUP, pfd1.revents); + close(fd[0]); + if (poll(&pfd1, 1, 0) == -1) + err(1, "poll"); + report(num++, "after close", POLLHUP, pfd1.revents); + close(fd[1]); + + /* + * With shutdown(SHUT_RD) + * Note that shutdown(SHUT_WR) is passed to the peer, but + * shutdown(SHUT_RD) is not. + */ + setup(); + if (shutdown(fd[0], SHUT_RD) == -1) + err(1, "shutdown"); + if (poll(&pfd0, 1, 0) == -1) + err(1, "poll"); + report(num++, "after shutdown(SHUT_RD)", POLLIN | POLLOUT, pfd0.revents); + if (poll(&pfd1, 1, 0) == -1) + err(1, "poll"); + report(num++, "other side after shutdown(SHUT_RD)", POLLOUT, pfd1.revents); + if (shutdown(fd[0], SHUT_WR) == -1) + err(1, "shutdown"); + if (poll(&pfd0, 1, 0) == -1) + err(1, "poll"); + report(num++, "after shutdown(SHUT_WR)", POLLHUP, pfd0.revents); + if (poll(&pfd1, 1, 0) == -1) + err(1, "poll"); + report(num++, "other side after shutdown(SHUT_WR)", POLLIN | POLLOUT, pfd1.revents); + close(fd[0]); + close(fd[1]); + + return (0); +}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200909012058.n81KwfSk072165>