Simplify system::get_available_port() (nanocurrency#4388)

The function get_available_port is too confusing. To simplify it, I am
refactoring one test job it has into a separate test function.

The test function speculatively chooses a free tcp binding port, for one
unit test case, to check that the node can be configured with a manual
port.

nano/core_test/network.cpp

@@ -57,7 +57,8 @@ TEST (network, tcp_connection)
 TEST (network, construction_with_specified_port)
 {
 	nano::test::system system{};
-	auto const port = system.get_available_port (/* do not allow 0 port */ false);
+	auto const port = nano::test::speculatively_choose_a_free_tcp_bind_port ();
+	ASSERT_NE (port, 0);
 	auto const node = system.add_node (nano::node_config{ port });
 	EXPECT_EQ (port, node->network.port);
 	EXPECT_EQ (port, node->network.endpoint ().port ());

nano/test_common/network.cpp

@@ -31,3 +31,28 @@ std::shared_ptr<nano::node> nano::test::add_outer_node (nano::test::system & sys
 	system_a.nodes.push_back (outer_node);
 	return outer_node;
 }
+
+// Note: this is not guaranteed to work, it is speculative
+uint16_t nano::test::speculatively_choose_a_free_tcp_bind_port ()
+{
+	/*
+	 * This works because the kernel doesn't seem to reuse port numbers until it absolutely has to.
+	 * Subsequent binds to port 0 will allocate a different port number.
+	 */
+	boost::asio::io_context io_ctx;
+	boost::asio::ip::tcp::acceptor acceptor{ io_ctx };
+	boost::asio::ip::tcp::tcp::endpoint endpoint{ boost::asio::ip::tcp::v4 (), 0 };
+	acceptor.open (endpoint.protocol ());
+
+	boost::asio::socket_base::reuse_address option{ true };
+	acceptor.set_option (option); // set SO_REUSEADDR option
+
+	acceptor.bind (endpoint);
+
+	auto actual_endpoint = acceptor.local_endpoint ();
+	auto port = actual_endpoint.port ();
+
+	acceptor.close ();
+
+	return port;
+}

nano/test_common/network.hpp

@@ -18,7 +18,11 @@ namespace test
 	class system;
 	/** Waits until a TCP connection is established and returns the TCP channel on success*/
 	std::shared_ptr<nano::transport::channel_tcp> establish_tcp (nano::test::system &, nano::node &, nano::endpoint const &);
+
 	/** Adds a node to the system without establishing connections */
 	std::shared_ptr<nano::node> add_outer_node (nano::test::system & system, nano::node_flags = nano::node_flags ());
+
+	/** speculatively (it is not guaranteed that the port will remain free) find a free tcp binding port and return it */
+	uint16_t speculatively_choose_a_free_tcp_bind_port ();
 }
 }

nano/test_common/system.cpp

@@ -566,56 +566,27 @@ nano::node_config nano::test::system::default_config ()
 	return config;
 }
 
-uint16_t nano::test::system::get_available_port (bool can_be_zero)
+uint16_t nano::test::system::get_available_port ()
 {
 	auto base_port_str = std::getenv ("NANO_TEST_BASE_PORT");
-	if (base_port_str)
-	{
-		// Maximum possible sockets which may feasibly be used in 1 test
-		constexpr auto max = 200;
-		static uint16_t current = 0;
-		// Read the TEST_BASE_PORT environment and override the default base port if it exists
-		uint16_t base_port = boost::lexical_cast<uint16_t> (base_port_str);
-
-		uint16_t const available_port = base_port + current;
-		++current;
-		// Reset port number once we have reached the maximum
-		if (current == max)
-		{
-			current = 0;
-		}
+	if (!base_port_str)
+		return 0; // let the O/S decide
 
-		return available_port;
-	}
-	else
-	{
-		if (!can_be_zero)
-		{
-			/*
-			 * This works because the kernel doesn't seem to reuse port numbers until it absolutely has to.
-			 * Subsequent binds to port 0 will allocate a different port number.
-			 */
-			boost::asio::ip::tcp::acceptor acceptor{ io_ctx };
-			boost::asio::ip::tcp::tcp::endpoint endpoint{ boost::asio::ip::tcp::v4 (), 0 };
-			acceptor.open (endpoint.protocol ());
+	// Maximum possible sockets which may feasibly be used in 1 test
+	constexpr auto max = 200;
+	static uint16_t current = 0;
 
-			boost::asio::socket_base::reuse_address option{ true };
-			acceptor.set_option (option); // set SO_REUSEADDR option
+	// Read the TEST_BASE_PORT environment and override the default base port if it exists
+	uint16_t base_port = boost::lexical_cast<uint16_t> (base_port_str);
 
-			acceptor.bind (endpoint);
+	uint16_t const available_port = base_port + current;
+	++current;
 
-			auto actual_endpoint = acceptor.local_endpoint ();
-			auto port = actual_endpoint.port ();
+	// Reset port number once we have reached the maximum
+	if (current >= max)
+		current = 0;
 
-			acceptor.close ();
-
-			return port;
-		}
-		else
-		{
-			return 0;
-		}
-	}
+	return available_port;
 }
 
 // Makes sure everything is cleaned up

nano/test_common/system.hpp

@@ -61,7 +61,12 @@ namespace test
 		 * Returns default config for node running in test environment
 		 */
 		nano::node_config default_config ();
-		uint16_t get_available_port (bool can_be_zero = true);
+
+		/*
+		 * Returns port 0 by default, to let the O/S choose a port number.
+		 * If NANO_TEST_BASE_PORT is set then it allocates numbers by itself from that range.
+		 */
+		uint16_t get_available_port ();
 
 	public:
 		boost::asio::io_context io_ctx;