--6c2NcOVqGQ03X4Wi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Oct 09, 2003 at 10:46:34PM +0100, David Laight wrote:
> [...]
> 
> Comparing allsock to allsock_select and redoing the copy if different
> will fix that one.
> 
> I've also realised that the main programs use of FD_SET() and FD_CLEAR()
> can also lead to bit-rot.

FD_* should probably be guarded by signals disable/enable.
I had a quick look at this:
- there is the FD_CLR() in the main loop, for the wait case.
  This one is done with signals blocked(sigblock(SIGBLOCK))
- setup() and close_sep() probably need to be called with signals disabled.
  setup() is called from config() and retry(). config() is called at startup
  time, and from SIGHUP handler, so should be safe in both cases.
  retry() is called from SIGALRM handler so should be safe too.
  close_sep() is called from the select() loop with signal blockeds 
  (sigblock(SIGBLOCK)), and from config(), so it should be safe too.

Did you see something else ?

BTW, we do a sigblock()/sigsetmask() for each readable file descriptor
after select(). We could probably decrease this to one per select loop,
and move the readable = allsock_select = allsock at the end of the loop.
This way we don't increase the number of system calls needed (and may even
decrease it).

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 24 ans d'experience feront toujours la difference
--

--6c2NcOVqGQ03X4Wi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff

Index: inetd.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/inetd/inetd.c,v
retrieving revision 1.76
diff -u -r1.76 inetd.c
--- inetd.c	2002/01/21 14:42:28	1.76
+++ inetd.c	2003/10/11 15:18:20
@@ -272,7 +272,7 @@
 int	lflag;
 #endif
 int	nsock, maxsock;
-fd_set	allsock;
+fd_set	allsock, allsock_select;
 int	options;
 int	timingout;
 struct	servent *sp;
@@ -443,6 +443,7 @@
 	struct sigvec sv;
 	int ch, dofork;
 	pid_t pid;
+	fd_set readable;
 
 	Argv = argv;
 	if (envp == 0 || *envp == 0)
@@ -521,9 +522,9 @@
 		(void)setenv("inetd_dummy", dummy, 1);
 	}
 
+	readable = allsock_select = allsock;
 	for (;;) {
 	    int n, ctrl;
-	    fd_set readable;
 
 	    if (nsock == 0) {
 		(void) sigblock(SIGBLOCK);
@@ -531,8 +532,7 @@
 		    sigpause(0L);
 		(void) sigsetmask(0L);
 	    }
-	    readable = allsock;
-	    if ((n = select(maxsock + 1, &readable, (fd_set *)0,
+	    if ((n = select(maxsock + 1, &allsock_select, (fd_set *)0,
 		(fd_set *)0, (struct timeval *)0)) <= 0) {
 		    if (n == -1 && errno != EINTR) {
 			syslog(LOG_WARNING, "select: %m");
@@ -540,9 +540,11 @@
 		    }
 		    continue;
 	    }
+	    (void) sigblock(SIGBLOCK);
 	    for (sep = servtab; n && sep; sep = nsep) {
 	    nsep = sep->se_next;
-	    if (sep->se_fd != -1 && FD_ISSET(sep->se_fd, &readable)) {
+	    if (sep->se_fd != -1 && FD_ISSET(sep->se_fd, &readable) &&
+		FD_ISSET(sep->se_fd, &allsock_select)) {
 		n--;
 		if (debug)
 			fprintf(stderr, "someone wants %s\n", sep->se_service);
@@ -561,7 +563,6 @@
 			}
 		} else
 			ctrl = sep->se_fd;
-		(void) sigblock(SIGBLOCK);
 		pid = 0;
 #ifdef LIBWRAP_INTERNAL
 		dofork = 1;
@@ -587,7 +588,6 @@
 					if (!sep->se_wait && sep->se_socktype == SOCK_STREAM)
 					    close(ctrl);
 					close_sep(sep);
-					sigsetmask(0L);
 					if (!timingout) {
 						timingout = 1;
 						alarm(RETRYTIME);
@@ -600,7 +600,6 @@
 				syslog(LOG_ERR, "fork: %m");
 				if (!sep->se_wait && sep->se_socktype == SOCK_STREAM)
 					close(ctrl);
-				sigsetmask(0L);
 				sleep(1);
 				continue;
 			}
@@ -617,7 +616,6 @@
 					setsid();
 			}
 		}
-		sigsetmask(0L);
 		if (pid == 0) {
 			run_service(ctrl, sep);
 			if (dofork)
@@ -627,6 +625,8 @@
 			close(ctrl);
 	    }
 	    }
+	    readable = allsock_select = allsock;
+	    sigsetmask(0L);
 	}
 }
 
@@ -770,6 +770,7 @@
 					    sep->se_server, WTERMSIG(status));
 				sep->se_wait = 1;
 				FD_SET(sep->se_fd, &allsock);
+				FD_SET(sep->se_fd, &allsock_select);
 				nsock++;
 				if (debug)
 					fprintf(stderr, "restored %s, fd %d\n",

--6c2NcOVqGQ03X4Wi--