Re: [libmicrohttpd] Sockets on Windows

[Michael Lenaghan]

>> Christian, the following code (from MHD_start_daemon_va) is wrong for 
>> Windows:
>
> That's already fixed in SVN, thanks anyway for pointing it out.

These are still wrong:

In MHD_accept_connection:

  s = ACCEPT (daemon->socket_fd, addr, &addrlen);
  if ((s < 0) || (addrlen <= 0)) {
    ...
  }

In MHD_start_daemon_va:

    socket_fd = SOCKET (PF_INET, SOCK_STREAM, 0);
  if (socket_fd < 0) {
    ...
  }

Ideally you should test against INVALID_SOCKET. You can also test
against "-1"--but then you'd be relying on an implicit signed/unsigned
conversion.

>>  * You should check the total number of sockets being select()ed
>> against FD_SETSIZE
>>
>
> I think that should be checked anytime FD_SET() & friends are called, no?

In theory, yes; otherwise FD_SET() will fail silently if you add too
many sockets to the set. But there's more to it than that. In the
default config the maximum number of connections is constrained by
FD_SETSIZE:

#define MHD_MAX_CONNECTIONS_DEFAULT FD_SETSIZE -4

So you'd only see that condition--ie, too many sockets for select--if
someone increases max connections beyond FD_SETSIZE. Of course, at
that point you have a problem waiting in the wings.

I think you have two choices. You can constrain max connections to
FD_SETSIZE on Windows--or you can allow it to grow beyond FD_SETSIZE.
If you constrain it you never have to worry about checking the count.
If you allow it to grow then you should define your own fd_set struct
on the fly and size it either to the number of connections or to the
number of max connections. (If you use your own FD_* routines Windows
doesn't care how big the array is as long as the count of slots inside
the fd_set struct is set correctly.)

On Windows FD_SETSIZE defaults to 64. Some people might think that's a
bit small. There's a tradeoff in making it too big, though: if you
selected on, say, 64 sockets you have to scan through 64 slots to find
each socket after the call to select(). So it's an O(n^2) operation
right in your innermost loop.

Obviously, select() on Windows is not what you'd use if you were
worried about heavy load--so I think constraining max connections
would be reasonable.