Re: select() on non-sockets in eval.c

Hi,

At Fri, 27 May 2005 22:49:50 +0900,
BG - Ben Armstrong wrote in [ruby-core:05113]:
> Nobu, if I understand this code correctly, I think I see a problem:

Correct.

> Later, once we have ruled out the socket-only & non-socket-only cases
> which are trivial, we go on to just select against the socket fds: 
> 
> > +    ret = select(maxsock, rp, wp, ep, &z);

I forgot to initialize rp, wp and ep.

> If an error was returned, we just ignore it and return nsocks, the # of
> non-socket fds, and return the non-socket fd bits in rfd, wfd & efd.  So
> far, so good.

Counting non-sockets was wrong.

> But if select returned >= 0, then for each set bit in rp, wp & ep, we
> set a bit in rfd, wfd & efd to return:
> 
> > +    for (i = 0; i < maxsock; ++i) {
> > +	if (rp && FD_ISSET(i, rp)) {
> > +	    FD_SET(i, rfd);
> > +	    ret++;
> > +	}
> ... etc. ...
> 
> This is where I get confused about what you intended.  Ret started as
> the return value from select on the sockets, and rp, wp and ep should
> already contain the ready socket fds.  Why are we adding into ret the
> count of *socket* fds ready?  Shouldn't this be setting rfd, wfd and efd
> to the returned rp, wp and ep, and then add in the *non-socket* bits
> (and increment ret per ready non-socket fd)?  But we don't have any
> variables storing just the non-socket bits, so we can't do that.

New select() wrapper function, does this work?

int
rb_vms_select(int maxfd, fd_set *rfd, fd_set *wfd, fd_set *efd, struct timeval *tv)
{
    int i, maxsock, socks, nsocks;
    fd_set rs, ws, es, *rp = 0, *wp = 0, *ep = 0;
    struct timeval z;

    FD_ZERO(&rs);
    FD_ZERO(&ws);
    FD_ZERO(&es);

    /* count socket and non-socket fds, and move socket bits to local
     * fd_sets */
    maxsock = socks = nsocks = 0;
    if (rfd || wfd || efd) {
	for (i = 0; i < maxfd; ++i) {
	    int r = (rfd != 0) && FD_ISSET(i, rfd);
	    int w = (wfd != 0) && FD_ISSET(i, wfd);
	    int e = (efd != 0) && FD_ISSET(i, efd);
	    if (!r && !w && !e) continue;
	    if (is_socket(i)) {
		if (r) {
		    FD_SET(i, (rp = &rs));
		    FD_CLR(i, rfd);
		}
		if (w) {
		    FD_SET(i, (wp = &ws));
		    FD_CLR(i, wfd);
		}
		if (e) {
		    FD_SET(i, (ep = &es));
		    FD_CLR(i, efd);
		}
		maxsock = i + 1;
		socks++;
	    }
	    else {
		nsocks += r + w + e;
	    }
	}
    }

    if (!nsocks) {
	/* sockets only */
	return select(maxsock, rfd, wfd, efd, tv);
    }

    if (!socks) {
	/* assuming non-socket fds are always ready */
	return nsocks;
    }

    /* check if sockets are ready */
    z.tv_sec = 0;
    z.tv_usec = 0;
    ret = select(maxsock, rp, wp, ep, &z);
    if (ret <= 0) {
	return nsocks;
    }

    /* copy set bits back */
    for (i = 0; i < maxsock; ++i) {
	if (rp && FD_ISSET(i, rp)) FD_SET(i, rfd);
	if (wp && FD_ISSET(i, wp)) FD_SET(i, wfd);
	if (ep && FD_ISSET(i, ep)) FD_SET(i, efd);
    }
    ret += nsocks;

    return ret;
}

-- 
Nobu Nakada