Skip to content

Commit

Permalink
FD_SETSIZE now matches RubyInstaller for windows support
Browse files Browse the repository at this point in the history
  • Loading branch information
@stakach
stakach committed Mar 8, 2012
1 parent ba78ffa commit 9839573
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion ext/project.h
View file
Expand Up @@ -82,7 +82,7 @@ typedef int SOCKET;

#ifdef OS_WIN32
// 21Sep09: windows limits select() to 64 sockets by default, we increase here (before including winsock2.h)
#define FD_SETSIZE 2048
#define FD_SETSIZE 32767

#define WIN32_LEAN_AND_MEAN
// Add support for IPv6
Expand Down

8 comments on commit 9839573

@rdp
Copy link

@rdp rdp commented on 9839573 Mar 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually ruby's extconf system will automagically set this to the higher number for you already, won't it? (i.e. you don't have to set it at all?)

@stakach
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe.
I assume you would have to set it here too as it is compiled independently from ruby then loaded dynamically at run time.

@rdp
Copy link

@rdp rdp commented on 9839573 Mar 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe when ruby calls gcc it passes in the "original parameters ruby itself was built with" which includes the -DFD_SETSIZE config.

@stakach
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet!
Ill do a test compile this afternoon and update the pull request.

@stakach
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, we should leave it here as other projects such as http://rubini.us/ will not be including the flag.
See rubinius/rubinius#1600

@rdp
Copy link

@rdp rdp commented on 9839573 Mar 26, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If other projects don't include it then it should fall back to the default shouldn't it? (otherwise you'll see the descrepancies you noted)?

@stakach
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True that. I'll remove now and update the rubinius issue

@stakach
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See: 8840d22

Please sign in to comment.