Skip to content

Conversation

robbak
Copy link
Contributor

@robbak robbak commented May 25, 2013

When compiling on FreeBSD, the calculation here returns an unsigned int. This causes a compile-time error with std::min, which cannot compare signed with unsigned integers.

This pull inserts an explicit cast, treating the calculated value as signed, keeping the compiler happy.

@jgarzik
Copy link
Contributor

jgarzik commented May 25, 2013

ACK

@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/03f498080a813850e1a3addf2c555450aa5e65c1 for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@sipa
Copy link
Member

sipa commented May 30, 2013

ACK

sipa added a commit that referenced this pull request May 30, 2013
Explictly cast calculation to int, to allow std::min to work.
@sipa sipa merged commit d315eb0 into bitcoin:master May 30, 2013
@@ -530,7 +530,7 @@ bool AppInit2(boost::thread_group& threadGroup)
// Make sure enough file descriptors are available
int nBind = std::max((int)mapArgs.count("-bind"), 1);
nMaxConnections = GetArg("-maxconnections", 125);
nMaxConnections = std::max(std::min(nMaxConnections, FD_SETSIZE - nBind - MIN_CORE_FILEDESCRIPTORS), 0);
nMaxConnections = std::max(std::min(nMaxConnections, (int)(FD_SETSIZE - nBind - MIN_CORE_FILEDESCRIPTORS)), 0);
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it will create problems if nBind+MIN_CORE_FILEDESCRIPTORS are > FD_SETSIZE...
Not that I see any reason someone should bind more than 1024 ports...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants