The one-liner appears to work in my testing. Perhaps I was worrying too much about the zero FD's case. (Habit!)
-- Jim -----Original Message----- From: Denys Vlasenko [mailto:[EMAIL PROTECTED] Sent: Wednesday, November 05, 2008 2:35 PM To: Cathey, Jim Cc: [EMAIL PROTECTED]; [email protected] Subject: Re: inetd bug Hi Jim, On Wednesday 05 November 2008 18:47, Cathey, Jim wrote: > On the face of it I think this fix is inadequate, > given that "maxsock + 1" is used in the select(). > With no services, should select not have a 0 as the > first argument, thus requiring maxsock to be -1 and > not 0? All the problems fell out of that value. This is safe. fds 0, 1 and 2 are not included in fd bitmask for select. Look closer: if (!(opt & 2)) bb_daemonize_or_rexec(0, argv - optind); else bb_sanitize_stdio(); This code makes sure that even if user somehow managed to start inetd with these descriptors CLOSED (!), we reopen them to /dev/null. We never use them for network i/o. This the lowest descriptor in select() which can ever be used is 3. Specifying no_of_fds=1 in select() will make it wait on empty set of fds. I want to have minimalistic fix because your patch has bugs: - if (maxsock >= 0 && fd > maxsock) { + if (fd > maxsock) { prev_maxsock = maxsock = fd; Scenario: we need to stop waiting on a fd - we call remove_fd_from_set(). It sets maxsock to -1 ("unknown"). Then we got SIGCHLD and need to start waiting on another fd (another wait-type service). We call add_fd_to_set(). Due to your change fd > maxsock (because maxsock == -1) and it will erroneously set prev_maxsock = maxsock = fd, which is NOT the maximal open fd! On next iteration select() will lose some number of fds (all fds which are > fd). So, does one-line fix I posted work, or not? In my testing, it works. -- vda _______________________________________________ busybox mailing list [email protected] http://busybox.net/cgi-bin/mailman/listinfo/busybox
