Bug in select_modify?

[Marc Lehmann]

On Sun, Jun 08, 2008 at 01:24:31AM -0700, Matt Tolton <matt at tolton.com> wrote:
> In getting libev running on windows, I think I may have found a bug in
> the select_modify() function in ev_select.c.

What you actually found is yet another bug on that horribly broken
platform.

Well spotted (none of my programs aggressively re-use fds :)

> an array for its fd set, and it doesn't check to see if the fd is
> already in the set when you do an FD_SET.

Right, because FD_SET is guarenteed to not to add an fd twice even as per
msdn docs - of course, the implementation is broken as always.

Since microsoft will unliekly bother to fix their pile of bugs, libev has to
yield again.

> kept returning EBADF, but libev couldn't find the bad fd.  I imagine
> that most fd sets use bitvectors or something so this isn't a problem
> (not that I know much about this stuff).

Well, the point of having those macros is to not have to know about how they
are implemented - the berkeley sockets interface guarantees that FD_SET only
adds the fd once to the set (after all, thats a set), and microsoft claims
to follow this.

So the problem is not how its implemented, but the fatc that it is simply
broken - the whole point of having FD_SET was to not have that bug in the
first place. *sigh*.

> I made the following
> modification to select_modify (sorry it's not in diff format...I'm
> sure you can see where it goes):

Indeed, for such small patches, it's ok (you could have formatted it
better, though...)

I will try to add somrething like that that only punishes windows to libev.

Thanks a lot for catching this and analysing the situation!

> It seems that something like this should be added to the real libev
> source.  You could also, of course, use the FD_ISSET macro.  This
> would probably be more foolproof, but I'm guessing it would be slower
> (not that we'll get much performance out of windows using select()
> anyway).

oev <-> nev changes should catch 100% of the transitions for each fd,
except after a fork, which doesn't matter as this is not an operation
windows supports in a sensible way.

-- 
                The choice of a       Deliantra, the free code+content MORPG
      -----==-     _GNU_              http://www.deliantra.net
      ----==-- _       generation
      ---==---(_)__  __ ____  __      Marc Lehmann
      --==---/ / _ \/ // /\ \/ /      pcg at goof.com
      -=====/_/_//_/\_,_/ /_/\_\