Skip to content

Commit f7329af

Browse files
author
Razvan Becheriu
committed
[#3536] addressed review comments
1 parent ce11cf9 commit f7329af

9 files changed

+42
-37
lines changed

‎src/bin/dhcp4/json_config_parser.cc

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -900,15 +900,14 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set,
900900
}
901901
}
902902

903-
// Print the list of known backends.
904-
LeaseMgrFactory::printRegistered();
903+
// Log the list of known backends.
904+
LeaseMgrFactory::logRegistered();
905905

906-
// Print the list of known backends.
907-
HostDataSourceFactory::printRegistered();
906+
// Log the list of known backends.
907+
HostDataSourceFactory::logRegistered();
908908

909909
// Moved from the commit block to add the config backend indication.
910910
if (status_code == CONTROL_RESULT_SUCCESS && (!check_only || extra_checks)) {
911-
912911
try {
913912
// If there are config backends, fetch and merge into staging config
914913
server.getCBControl()->databaseConfigFetch(srv_config,

‎src/bin/dhcp6/json_config_parser.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,11 +1036,11 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set,
10361036
}
10371037
}
10381038

1039-
// Print the list of known backends.
1040-
LeaseMgrFactory::printRegistered();
1039+
// Log the list of known backends.
1040+
LeaseMgrFactory::logRegistered();
10411041

1042-
// Print the list of known backends.
1043-
HostDataSourceFactory::printRegistered();
1042+
// Log the list of known backends.
1043+
HostDataSourceFactory::logRegistered();
10441044

10451045
// Moved from the commit block to add the config backend indication.
10461046
if (status_code == CONTROL_RESULT_SUCCESS && (!check_only || extra_checks)) {

‎src/lib/dhcpsrv/host_data_source_factory.cc

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ HostDataSourceFactory::add(HostDataSourceList& sources,
4343
DatabaseConnection::ParameterMap parameters =
4444
DatabaseConnection::parse(dbaccess);
4545

46-
// Get the database type and open the corresponding database
46+
// Get the database type and open the corresponding database.
4747
DatabaseConnection::ParameterMap::iterator it = parameters.find("type");
4848
if (it == parameters.end()) {
4949
isc_throw(InvalidParameter, "Host database configuration does not "
@@ -55,8 +55,7 @@ HostDataSourceFactory::add(HostDataSourceList& sources,
5555

5656
// No match?
5757
if (index == map_.end()) {
58-
if ((db_type == "mysql") ||
59-
(db_type == "postgresql")) {
58+
if ((db_type == "mysql") || (db_type == "postgresql")) {
6059
string with = (db_type == "postgresql" ? "pgsql" : db_type);
6160
isc_throw(InvalidType, "The type of host backend: '" << db_type
6261
<< "' is not compiled in. Did you forget to use --with-"
@@ -69,11 +68,11 @@ HostDataSourceFactory::add(HostDataSourceList& sources,
6968
// Call the factory and push the pointer on sources.
7069
sources.push_back(index->second(parameters));
7170

72-
// Check the factory did not return NULL.
71+
// Check the factory did not return null.
7372
if (!sources.back()) {
7473
sources.pop_back();
7574
isc_throw(Unexpected, "Hosts database " << db_type <<
76-
" factory returned NULL");
75+
" factory returned null");
7776
}
7877
}
7978

@@ -163,14 +162,18 @@ HostDataSourceFactory::registeredFactory(const std::string& db_type) {
163162
}
164163

165164
void
166-
HostDataSourceFactory::printRegistered() {
165+
HostDataSourceFactory::logRegistered() {
167166
std::stringstream txt;
168167

169168
for (auto const& x : map_) {
170-
txt << x.first << " ";
169+
if (!txt.str().empty()) {
170+
txt << " ";
171+
}
172+
txt << x.first;
171173
}
172174

173-
LOG_INFO(hosts_logger, HOSTS_BACKENDS_REGISTERED).arg(txt.str());
175+
LOG_INFO(hosts_logger, HOSTS_BACKENDS_REGISTERED)
176+
.arg(txt.str());
174177
}
175178

176179
} // namespace dhcp

‎src/lib/dhcpsrv/host_data_source_factory.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class HostDataSourceFactory {
9696
/// @brief Type of host data source factory
9797
///
9898
/// A factory takes a parameter map and returns a pointer to a host
99-
/// data source. In case of failure it must throw and not return NULL.
99+
/// data source. In case of failure it must throw and not return null.
100100
typedef std::function<HostDataSourcePtr (const db::DatabaseConnection::ParameterMap&)> Factory;
101101

102102
/// @brief Register a host data source factory
@@ -131,12 +131,12 @@ class HostDataSourceFactory {
131131
/// @return true if a factory was registered for db_type, false if not.
132132
static bool registeredFactory(const std::string& db_type);
133133

134-
/// @brief Prints out all registered backends.
134+
/// @brief Logs out all registered backends.
135135
///
136136
/// We need a dedicated method for this, because we sometimes can't log
137137
/// the backend type when doing early initialization for backends
138138
/// initialized statically.
139-
static void printRegistered();
139+
static void logRegistered();
140140

141141
private:
142142
/// @brief Factory map

‎src/lib/dhcpsrv/lease_mgr_factory.cc

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ LeaseMgrFactory::create(const std::string& dbaccess) {
4747
DatabaseConnection::ParameterMap parameters = DatabaseConnection::parse(dbaccess);
4848
std::string redacted = DatabaseConnection::redactedAccessString(parameters);
4949

50-
// Get the database type and open the corresponding database
50+
// Get the database type and open the corresponding database.
5151
DatabaseConnection::ParameterMap::iterator it = parameters.find(type);
5252
if (it == parameters.end()) {
5353
LOG_ERROR(dhcpsrv_logger, DHCPSRV_NOTYPE_DB).arg(dbaccess);
@@ -88,8 +88,7 @@ LeaseMgrFactory::create(const std::string& dbaccess) {
8888

8989
// No match?
9090
if (index == map_.end()) {
91-
if ((db_type == "mysql") ||
92-
(db_type == "postgresql")) {
91+
if ((db_type == "mysql") || (db_type == "postgresql")) {
9392
LOG_ERROR(dhcpsrv_logger, DHCPSRV_UNKNOWN_DB).arg(db_type);
9493
string with = (db_type == "postgresql" ? "pgsql" : db_type);
9594
isc_throw(InvalidType, "The Kea server has not been compiled with "
@@ -106,10 +105,10 @@ LeaseMgrFactory::create(const std::string& dbaccess) {
106105
// Call the factory.
107106
getLeaseMgrPtr() = index->second(parameters);
108107

109-
// Check the factory did not return NULL.
108+
// Check the factory did not return null.
110109
if (!getLeaseMgrPtr()) {
111110
isc_throw(Unexpected, "Lease database " << db_type <<
112-
" factory returned NULL");
111+
" factory returned null");
113112
}
114113
}
115114

@@ -159,7 +158,7 @@ LeaseMgrFactory::haveInstance() {
159158
TrackingLeaseMgr&
160159
LeaseMgrFactory::instance() {
161160
TrackingLeaseMgr* lmptr = getLeaseMgrPtr().get();
162-
if (lmptr == NULL) {
161+
if (!lmptr) {
163162
isc_throw(NoLeaseManager, "no current lease manager is available");
164163
}
165164
return (*lmptr);
@@ -208,14 +207,18 @@ LeaseMgrFactory::registeredFactory(const std::string& db_type) {
208207
}
209208

210209
void
211-
LeaseMgrFactory::printRegistered() {
210+
LeaseMgrFactory::logRegistered() {
212211
std::stringstream txt;
213212

214213
for (auto const& x : map_) {
215-
txt << x.first << " ";
214+
if (!txt.str().empty()) {
215+
txt << " ";
216+
}
217+
txt << x.first;
216218
}
217219

218-
LOG_INFO(dhcpsrv_logger, DHCPSRV_LEASE_MGR_BACKENDS_REGISTERED).arg(txt.str());
220+
LOG_INFO(dhcpsrv_logger, DHCPSRV_LEASE_MGR_BACKENDS_REGISTERED)
221+
.arg(txt.str());
219222
}
220223

221224
} // namespace dhcp

‎src/lib/dhcpsrv/lease_mgr_factory.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
namespace isc {
1919
namespace dhcp {
2020

21-
2221
/// @brief No lease manager exception
2322
///
2423
/// Thrown if an attempt is made to get a reference to the current lease
@@ -106,7 +105,7 @@ class LeaseMgrFactory {
106105
/// @brief Type of lease mgr factory
107106
///
108107
/// A factory takes a parameter map and returns a pointer to a lease mgr.
109-
/// In case of failure it must throw and not return NULL.
108+
/// In case of failure it must throw and not return null.
110109
typedef std::function<TrackingLeaseMgrPtr (const db::DatabaseConnection::ParameterMap&)> Factory;
111110

112111
/// @brief Register a lease mgr factory
@@ -121,7 +120,8 @@ class LeaseMgrFactory {
121120
/// @return true if the factory was successfully added to the map, false
122121
/// if it already exists.
123122
static bool registerFactory(const std::string& db_type,
124-
const Factory& factory, bool no_log = false);
123+
const Factory& factory,
124+
bool no_log = false);
125125

126126
/// @brief Deregister a lease mgr factory
127127
///
@@ -141,12 +141,12 @@ class LeaseMgrFactory {
141141
/// @return true if a factory was registered for db_type, false if not.
142142
static bool registeredFactory(const std::string& db_type);
143143

144-
/// @brief Prints out all registered backends.
144+
/// @brief Logs out all registered backends.
145145
///
146146
/// We need a dedicated method for this, because we sometimes can't log
147147
/// the backend type when doing early initialization for backends
148148
/// initialized statically.
149-
static void printRegistered();
149+
static void logRegistered();
150150

151151
private:
152152
/// @brief Hold pointer to lease manager

‎src/lib/dhcpsrv/tests/host_data_source_factory_unittest.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ TEST_F(HostDataSourceFactoryTest, notype) {
149149
InvalidType);
150150
}
151151

152-
// Verify that factory must not return NULL
152+
// Verify that factory must not return null
153153
TEST_F(HostDataSourceFactoryTest, null) {
154154
EXPECT_TRUE(HostDataSourceFactory::registerFactory("mem", factory0));
155155
EXPECT_THROW(HostDataSourceFactory::add(sources_, "type=mem"),

‎src/lib/dhcpsrv/tests/lease_mgr_factory_unittest.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ TEST_F(LeaseMgrFactoryTest, notype) {
173173
EXPECT_FALSE(LeaseMgrFactory::haveInstance());
174174
}
175175

176-
// Verify that factory must not return NULL
176+
// Verify that factory must not return null
177177
TEST_F(LeaseMgrFactoryTest, null) {
178178
EXPECT_TRUE(LeaseMgrFactory::registerFactory("mem", factory0));
179179
EXPECT_THROW(LeaseMgrFactory::create("type=mem"),

‎src/lib/dhcpsrv/tracking_lease_mgr.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ class TrackingLeaseMgr : public LeaseMgr {
303303
};
304304

305305
/// @brief TrackingLeaseMgr pointer
306-
typedef boost::shared_ptr<TrackingLeaseMgr> TrackingLeaseMgrPtr;
306+
typedef std::unique_ptr<TrackingLeaseMgr> TrackingLeaseMgrPtr;
307307

308308
} // end of namespace isc::dhcp
309309
} // end of namespace isc

0 commit comments

Comments
 (0)