[zeromq-dev] [PATCH] FD_SETSIZE increased for win32

[Pieter Hintjens]

On Thu, Nov 4, 2010 at 4:20 PM, Martin Sustrik <sustrik at 250bpm.com> wrote:

> It looks like FD_SETSIZE should be same for entire application, i.e.
> libzmq *and* client application. Thus there's no way to change the limit
> in libzmq without breaking client apps and no way to change the limit in
> the client app without breaking libzmq :(

This is not accurate afaics.  FD_SETSIZE is not some mystical system
wide constant.  The definition is used only in one (1) specific and
limited way, which is (in two parts):

* to define the fd_set typedef
* to operate on that typedef using the FD_CLR, FD_ISSET, FD_SET, and
FD_ZERO macros

What this means is that any code using those macros MUST use a
consistent value for the typedef.  That value can be as high as wanted
(it's an integer), with a minimum of 64.  The actual OS limit on
sockets is independent and orthogonal to this, which is purely a C
type definition issue.

That typedef, those macros, that's it.

I attach the actual code from winsock.h that uses FD_SETSIZE, you can
see that it's pretty self-explanatory.

Every call to select() already passes the number of FDs and is safe
(you assert that this number is less than FD_SETSIZE).  You can
created fd_sets in one compilation unit and pass them to another to do
select() on, there is no problem.

Again, this definition is not an ABI limit, not a system limit, and
does not need to be constant across an entire application.  Please
stop saying that, it's bogus.  It just needs to be consistent between
the fd_set typedef and the macros that manipulate on it.

The problem in 0MQ - and this is the real difficulty here - is the
source design which has no single place where non-portable definitions
are made.  So you have select.cpp, select.hpp, windows.hpp and zmq.h
all including winsock.h or winsock2.h.

Thus a source file like zmq.cpp uses the FD_ macros but it gets its
winsock definitions from zmq.h.  Whereas select.cpp pulls in
winsock2.h directly, and then includes select.hpp that does the same
thing again (redundantly).

This should be trivial to fix, if winsock[2].h was included in a
single place.  As it is, it's just nasty, there are four places to add
the same code, three if you fix the select.cpp/.hpp redundancy (which
I did in my patch).

My patch won't work since Mato has decreed that zmq.h shall not be
touched, but since it already defacto sets FD_SETSIZE to 64, fixing it
to define a sane FD_SETSIZE cannot be worse.

The quick and dirty solution, which I'll test and prove this weekend,
is to modify the FOUR places that 0MQ calls winsock.h with conditional
setting of FD_SETSIZE.  I do not like making this change because it
makes the code worse.

If anyone (with actual experience of working on win32) would like to
join this discussion, they'd be welcome.

-Pieter


---- snip ----
/*
 * Select uses arrays of SOCKETs.  These macros manipulate such
 * arrays.  FD_SETSIZE may be defined by the user before including
 * this file, but the default here should be >= 64.
 *
 * CAVEAT IMPLEMENTOR and USER: THESE MACROS AND TYPES MUST BE
 * INCLUDED IN WINSOCK.H EXACTLY AS SHOWN HERE.
 */
#ifndef FD_SETSIZE
#define FD_SETSIZE      64
#endif /* FD_SETSIZE */

typedef struct fd_set {
        u_short fd_count;               /* how many are SET? */
        SOCKET  fd_array[FD_SETSIZE];   /* an array of SOCKETs */
} fd_set;

extern int PASCAL FAR __WSAFDIsSet(SOCKET, fd_set FAR *);

#define FD_CLR(fd, set) do { \
    u_int __i; \
    for (__i = 0; __i < ((fd_set FAR *)(set))->fd_count ; __i++) { \
        if (((fd_set FAR *)(set))->fd_array[__i] == fd) { \
            while (__i < ((fd_set FAR *)(set))->fd_count-1) { \
                ((fd_set FAR *)(set))->fd_array[__i] = \
                    ((fd_set FAR *)(set))->fd_array[__i+1]; \
                __i++; \
            } \
            ((fd_set FAR *)(set))->fd_count--; \
            break; \
        } \
    } \
} while(0)

#define FD_SET(fd, set) do { \
    if (((fd_set FAR *)(set))->fd_count < FD_SETSIZE) \
        ((fd_set FAR *)(set))->fd_array[((fd_set FAR *)(set))->fd_count++]=fd;\
} while(0)

#define FD_ZERO(set) (((fd_set FAR *)(set))->fd_count=0)

#define FD_ISSET(fd, set) __WSAFDIsSet((SOCKET)fd, (fd_set FAR *)set)