From: Michal 'vorner' Vaner Date: Fri, 3 Feb 2012 10:04:36 +0000 (+0100) Subject: [1595] Comments X-Git-Tag: trac2351_base~264^2~5 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=33ff861b4cd62e8d2e6063452cc14e980d35bbaf;p=thirdparty%2Fkea.git [1595] Comments --- diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc index 357ba31dfc..d9c3de663d 100644 --- 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 3ed20613e0..973ab31b78 100644 --- 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 1ccc596b71..ea42ba000e 100644 --- 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 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 9182ae5ad7..ceef2d38ca 100644 --- 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 2b6e993293..2ea3808c40 100644 --- 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 8795db31c4..2c14120379 100644 --- 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