[1595] Comments

diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index 357ba31dfc8711e78fdf5daa325db1c45990ec9d..d9c3de663de6ea6de33482ef8737478807718d8a 100644 (file)
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -71,6 +71,9 @@ protected:
         dnss_(ios_, NULL, NULL, NULL),
         server(true, xfrout),
         rrclass(RRClass::IN()),
+        // The empty string is expected value of the parameter of
+        // requestSocket, not the app_name (there's no fallback, it checks
+        // the empty string is passed).
         sock_requestor_(dnss_, address_store_, 53210, "")
     {
         server.setDNSService(dnss_);

diff --git a/src/bin/auth/tests/config_unittest.cc b/src/bin/auth/tests/config_unittest.cc
index 3ed20613e06fcbe3b1737019084d7eac222f24d1..973ab31b78c3e573c94d61fd95c43f46542b3ef9 100644 (file)
--- a/src/bin/auth/tests/config_unittest.cc
+++ b/src/bin/auth/tests/config_unittest.cc
@@ -46,6 +46,9 @@ protected:
         dnss_(ios_, NULL, NULL, NULL),
         rrclass(RRClass::IN()),
         server(true, xfrout),
+        // The empty string is expected value of the parameter of
+        // requestSocket, not the app_name (there's no fallback, it checks
+        // the empty string is passed).
         sock_requestor_(dnss_, address_store_, 53210, "")
     {
         server.setDNSService(dnss_);

diff --git a/src/bin/resolver/tests/resolver_config_unittest.cc b/src/bin/resolver/tests/resolver_config_unittest.cc
index 1ccc596b7146a26dddb15dbeca1bfff23a3e8b67..ea42ba000e4d0aaf1fe8a2e0ff5a407990c8c8df 100644 (file)
--- a/src/bin/resolver/tests/resolver_config_unittest.cc
+++ b/src/bin/resolver/tests/resolver_config_unittest.cc
@@ -85,6 +85,9 @@ protected:
     scoped_ptr<const RequestContext> request;
     ResolverConfig() :
         dnss(ios, NULL, NULL, NULL),
+        // The empty string is expected value of the parameter of
+        // requestSocket, not the app_name (there's no fallback, it checks
+        // the empty string is passed).
         sock_requestor_(dnss, address_store_, 53210, "")
     {
         server.setDNSService(dnss);

diff --git a/src/lib/server_common/socket_request.h b/src/lib/server_common/socket_request.h
index 9182ae5ad706b3cbe59438b19439ed819dcbee1d..ceef2d38ca0164e1bb2acd91bd162bad6aef185e 100644 (file)
--- a/src/lib/server_common/socket_request.h
+++ b/src/lib/server_common/socket_request.h
@@ -161,10 +161,17 @@ public:
     /// \param port the port to which the socket should be bound (native endian,
     ///     not network byte order).
     /// \param share_mode how the socket can be shared with other requests.
-    /// This must be one of the defined values of ShareMode.
+    ///     This must be one of the defined values of ShareMode..
     /// \param share_name the name of sharing group, relevant for SHARE_SAME
     ///     (specified by us or someone else). If left empty (the default),
-    ///     the app_name parameter of initSocketRequestor is used.
+    ///     the app_name parameter of initSocketRequestor is used. If that one
+    ///     is empty as well, it is accepted, but not recommended, as such
+    ///     a non-descriptive name has a high chance of collisions between
+    ///     applications. nOTE THat you should provide a name (by share_name
+    ///     or app_name) even when you set it to DONT_SHARE (for logs and
+    ///     debugging) and you need to provide one with SHARE_SAME (to know
+    ///     what is same) and SHARE_ANY (someone else might want SHARE_SAME,
+    ///     so it would check against this)
     /// \return the socket, as a file descriptor and token representing it on
     ///     the socket creator side.
     ///
@@ -217,7 +224,12 @@ SocketRequestor& socketRequestor();
 /// \param session the CC session that'll be used to talk to the
 ///                socket creator.
 /// \param app_name default share name if one is not provided with
-///                 requestSocket
+///                 requestSocket. You can leave this as empty string,
+///                 but then you should provide a reasonably descriptive
+///                 name to requestSocket. Empty names work like any others,
+///                 but have a high chance of collisions, so it is recommended
+///                 to avoid them and provide the name of the application
+///                 here.
 /// \throw InvalidOperation when it is called more than once
 void initSocketRequestor(cc::AbstractSession& session,
                          const std::string& app_name);

diff --git a/src/lib/server_common/tests/portconfig_unittest.cc b/src/lib/server_common/tests/portconfig_unittest.cc
index 2b6e993293f25730056ba9209e1f14dc9893fbf6..2ea3808c40b4fe4f44a9ee8b0c29020d9ad71703 100644 (file)
--- a/src/lib/server_common/tests/portconfig_unittest.cc
+++ b/src/lib/server_common/tests/portconfig_unittest.cc
@@ -133,6 +133,9 @@ TEST_F(ParseAddresses, invalid) {
 struct InstallListenAddresses : public ::testing::Test {
     InstallListenAddresses() :
         dnss_(ios_, NULL, NULL, NULL),
+        // The empty string is expected parameter of requestSocket,
+        // not app_name - the request does not fall back to this, it
+        // is checked to be the same.
         sock_requestor_(dnss_, store_, 5288, "")
     {
         valid_.push_back(AddressPair("127.0.0.1", 5288));

diff --git a/src/lib/testutils/socket_request.h b/src/lib/testutils/socket_request.h
index 8795db31c42143e999a0b812f7883cfe73b48938..2c1412037975e6a720a037106d279633386bd912 100644 (file)
--- a/src/lib/testutils/socket_request.h
+++ b/src/lib/testutils/socket_request.h
@@ -60,7 +60,10 @@ public:
     ///     the application requests a different port, it is considered
     ///     a failure.
     /// \param expeted_app The share name for which all the requests should
-    ///     be made.
+    ///     be made. This is not the usual app_name - the requestSocket does
+    ///     not fall back to this value if its share_name is left empty, if
+    ///     you want to check the code relies on the requestor to use the
+    ///     app name, you set this to empty string.
     TestSocketRequestor(asiodns::DNSService& dnss,
                         server_common::portconfig::AddressList& store,
                         uint16_t expect_port,
@@ -147,7 +150,11 @@ public:
     /// \param port to bind to
     /// \param mode checked to be SHARE_SAME for now
     /// \param name checked to be the same as expected_app parameter of the
-    ///      constructor
+    ///      constructor. Note that this class does not provide the fallback
+    ///      to an app_name if this is empty string. To check the code relies
+    ///      on the fallback (wants to use the app_name instead of providing
+    ///      its own share name), you need to create this class with empty
+    ///      expected_app.
     /// \return The token and FD
     /// \throw SocketAllocateError as described above, to test error handling
     /// \throw ShareError as described above, to test error handling