Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix cli_tests websocket port binding issue #1178 #1187

Merged
merged 3 commits into from
Jul 28, 2018
Merged

Conversation

abitmore
Copy link
Member

@abitmore abitmore changed the title Set TCP bind port range for Travis-CI #1178 Fix cli_tests websocket port binding issue #1178 Jul 26, 2018
.travis.yml Outdated
@@ -22,6 +22,7 @@ script:
- sed -i '/tests/d' libraries/fc/CMakeLists.txt
- cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_FLAGS=--coverage -DCMAKE_CXX_FLAGS=--coverage -DBoost_USE_STATIC_LIBS=OFF -DCMAKE_CXX_OUTPUT_EXTENSION_REPLACE=ON .
- 'which build-wrapper-linux-x86-64 && build-wrapper-linux-x86-64 --out-dir bw-output make -j 2 cli_wallet witness_node chain_test cli_test || make -j 2 cli_wallet witness_node chain_test cli_test'
- sudo sysctl -w net.ipv4.ip_local_port_range="32768 60999"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove that line, it's not necessary.

@@ -71,7 +71,7 @@ int get_available_port()
socklen_t len = sizeof(sin);
if (getsockname(socket_fd, (struct sockaddr *)&sin, &len) == -1)
return -1;
return sin.sin_port;
return ntohs(sin.sin_port);
Copy link
Contributor

Choose a reason for hiding this comment

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

While you're at it, please close socket_fd before returning.

@abitmore
Copy link
Member Author

Rebased.

@pmconrad
Copy link
Contributor

socket_fd remains open if either bind() or getsockname() fails, but I think in a unit test that's acceptable.

@pmconrad pmconrad merged commit 11d7faa into develop Jul 28, 2018
@abitmore abitmore deleted the abitmore-patch-3 branch July 30, 2018 12:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants