Skip site navigation (1)Skip section navigation (2)
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>