Why is this?

[punt]

At least on linux, this defaults to 1024 (not perhaps what the author was thinking of the old unix 64). Clearly this is attempting to define the max number of players (so the socket monitor will work).

Code: Select all

#if UOX_PLATFORM != PLATFORM_WIN32
	#define FD_SETSIZE 256 
#endif

Perhaps this would be more appropriate:

Code: Select all

#if UOX_PLATFORM != PLATFORM_WIN32
       #ifdef FD_SETSIZE
             #if FD_SETSIZE < 256
                 #undef FD_SETSIZE
           	#define FD_SETSIZE 256
              #endif
        #else
              #define FD_SETSIZE 256 
        #endif
#endif

Actually, if you do this, not sure why you need the platform check anymore.


Even if you don't accept/like this, the curernt implementation will give a warning about redefining FD_SETSIZE (so it should be at least checked if defined, and undefined)

[giwo]

Ahh, good catch.

I'm not entirely sure what brought the need for the setsize, but I think it was something having to do with cygwin. Whatever the case, it certainly should be checked before we re-set it....

[jr]

Some googling has led me to believe that fd_set/select is handled differently enough on windows to not require fiddling with FD_SETSIZE at all, but I don't have access to VC to verify this.

On *nix FD_SETSIZE has to be specified before the relevant header (on cygwin this is sys/types.h; I think it is the same on Linux and *BSD) is included, because several other defines are computed from it in the same header. Therefore it is not possible to pick up the default and change it afterwards.

The reason it is necessary at all on cygwin is that cygwin's default is 64, but after initialization 80 FDs are already allocated (this is probably a bug somewhere else, my guess is the *nix code in cDirectoryListing is not closing everything it opens, but I haven't debugged it).

FD_SETSIZE also limits the maximum number of connections, so a reasonable value should be picked (and this dependency should probably be documented somewhere).

[punt]

Some googling has led me to believe that fd_set/select is handled differently enough on windows to not require fiddling with FD_SETSIZE at all, but I don't have access to VC to verify this.

On *nix FD_SETSIZE has to be specified before the relevant header (on cygwin this is sys/types.h; I think it is the same on Linux and *BSD) is included, because several other defines are computed from it in the same header. Therefore it is not possible to pick up the default and change it afterwards.

The reason it is necessary at all on cygwin is that cygwin's default is 64, but after initialization 80 FDs are already allocated (this is probably a bug somewhere else, my guess is the *nix code in cDirectoryListing is not closing everything it opens, but I haven't debugged it).

FD_SETSIZE also limits the maximum number of connections, so a reasonable value should be picked (and this dependency should probably be documented somewhere).

I understood what it did, and what it was applibcable for (my point was it doesn't hurt if set on windows, so cound just avoid the platform check if one wanted to).

I suppose I had two main issues that I wanted:

1. If one was going to set FD_SETSIZE, it should be checked first, befure being redefined (and clearly be in the correct place).
2. One could eliminate the platform check.

[giwo]

Ahh, didn't realize it needed to be declared before specific headers were included... that makes sense, though. I'll move it up to before the #includes in network.cpp.

As for Windows, changing it may not alter anything, but it is not a necesarry change either, and thus I'd rather leave it #ifdefd for a *nix-only environment.