BIND 10 trac3007, updated. 1a0742f1fd61234a6ec395729b0c4c4cad64aaac [3007] Merged 3007 into local 3007.
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Jul 1 19:25:34 UTC 2013
The branch, trac3007 has been updated
via 1a0742f1fd61234a6ec395729b0c4c4cad64aaac (commit)
via e7c52fec35a1f6a49351f58877d21fd43b4082e0 (commit)
from 20b0efb43cff8020da949df03ce13d58ee6e4eac (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 1a0742f1fd61234a6ec395729b0c4c4cad64aaac
Merge: e7c52fe 20b0efb
Author: Thomas Markwalder <tmark at isc.org>
Date: Mon Jul 1 14:43:58 2013 -0400
[3007] Merged 3007 into local 3007.
commit e7c52fec35a1f6a49351f58877d21fd43b4082e0
Author: Thomas Markwalder <tmark at isc.org>
Date: Mon Jul 1 14:40:25 2013 -0400
[3007] A few more review changes.
-----------------------------------------------------------------------
Summary of changes:
src/bin/d2/ncr_msg.cc | 54 +++++++++++++++--------------
src/bin/d2/ncr_msg.h | 40 +++++++++++----------
src/bin/d2/tests/ncr_unittests.cc | 69 +++++++++++--------------------------
3 files changed, 70 insertions(+), 93 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/d2/ncr_msg.cc b/src/bin/d2/ncr_msg.cc
index 1698dd4..3a0e5e6 100644
--- a/src/bin/d2/ncr_msg.cc
+++ b/src/bin/d2/ncr_msg.cc
@@ -58,11 +58,12 @@ NameChangeRequest::NameChangeRequest()
dhcid_(), lease_expires_on_(), lease_length_(0), status_(ST_NEW) {
}
-NameChangeRequest::NameChangeRequest(NameChangeType change_type,
- bool forward_change, bool reverse_change,
- const std::string& fqdn, const std::string & ip_address,
- const D2Dhcid& dhcid, const ptime& lease_expires_on,
- uint32_t lease_length)
+NameChangeRequest::NameChangeRequest(const NameChangeType change_type,
+ const bool forward_change, const bool reverse_change,
+ const std::string& fqdn, const std::string& ip_address,
+ const D2Dhcid& dhcid,
+ const boost::posix_time::ptime& lease_expires_on,
+ const uint32_t lease_length)
: change_type_(change_type), forward_change_(forward_change),
reverse_change_(reverse_change), fqdn_(fqdn), ip_address_(ip_address),
dhcid_(dhcid), lease_expires_on_(new ptime(lease_expires_on)),
@@ -74,7 +75,7 @@ NameChangeRequest::NameChangeRequest(NameChangeType change_type,
}
NameChangeRequestPtr
-NameChangeRequest::fromFormat(NameChangeFormat format,
+NameChangeRequest::fromFormat(const NameChangeFormat format,
isc::util::InputBuffer& buffer) {
// Based on the format requested, pull the marshalled request from
// InputBuffer and pass it into the appropriate format-specific factory.
@@ -114,12 +115,11 @@ NameChangeRequest::fromFormat(NameChangeFormat format,
}
void
-NameChangeRequest::toFormat(NameChangeFormat format,
+NameChangeRequest::toFormat(const NameChangeFormat format,
isc::util::OutputBuffer& buffer) const {
// Based on the format requested, invoke the appropriate format handler
// which will marshal this request's contents into the OutputBuffer.
- switch (format)
- {
+ switch (format) {
case FMT_JSON: {
// Invoke toJSON to create a JSON text of this request's contents.
std::string json = toJSON();
@@ -223,7 +223,7 @@ NameChangeRequest::toJSON() const {
void
NameChangeRequest::validateContent() {
- //@TODO This is an initial implementation which provides a minimal amount
+ //@todo This is an initial implementation which provides a minimal amount
// of validation. FQDN, DHCID, and IP Address members are all currently
// strings, these may be replaced with richer classes.
if (fqdn_ == "") {
@@ -271,7 +271,7 @@ NameChangeRequest::getElement(const std::string& name,
}
void
-NameChangeRequest::setChangeType(NameChangeType value) {
+NameChangeRequest::setChangeType(const NameChangeType value) {
change_type_ = value;
}
@@ -299,7 +299,7 @@ NameChangeRequest::setChangeType(isc::data::ConstElementPtr element) {
}
void
-NameChangeRequest::setForwardChange(bool value) {
+NameChangeRequest::setForwardChange(const bool value) {
forward_change_ = value;
}
@@ -320,7 +320,7 @@ NameChangeRequest::setForwardChange(isc::data::ConstElementPtr element) {
}
void
-NameChangeRequest::setReverseChange(bool value) {
+NameChangeRequest::setReverseChange(const bool value) {
reverse_change_ = value;
}
@@ -385,7 +385,8 @@ NameChangeRequest::getLeaseExpiresOnStr() const {
return (to_iso_string(*lease_expires_on_));
}
-void NameChangeRequest::setLeaseExpiresOn(const std::string& value) {
+void
+NameChangeRequest::setLeaseExpiresOn(const std::string& value) {
try {
// Create a new ptime instance from the ISO date-time string in value
// add assign it to lease_expires_on_.
@@ -399,7 +400,8 @@ void NameChangeRequest::setLeaseExpiresOn(const std::string& value) {
}
-void NameChangeRequest::setLeaseExpiresOn(const ptime& value) {
+void
+NameChangeRequest::setLeaseExpiresOn(const boost::posix_time::ptime& value) {
if (lease_expires_on_->is_not_a_date_time()) {
isc_throw(NcrMessageError, "Invalid value for lease_expires_on");
}
@@ -414,7 +416,7 @@ void NameChangeRequest::setLeaseExpiresOn(isc::data::ConstElementPtr element) {
}
void
-NameChangeRequest::setLeaseLength(uint32_t value) {
+NameChangeRequest::setLeaseLength(const uint32_t value) {
lease_length_ = value;
}
@@ -445,7 +447,7 @@ NameChangeRequest::setLeaseLength(isc::data::ConstElementPtr element) {
}
void
-NameChangeRequest::setStatus(NameChangeStatus value) {
+NameChangeRequest::setStatus(const NameChangeStatus value) {
status_ = value;
}
@@ -455,15 +457,15 @@ NameChangeRequest::toText() const {
stream << "Type: " << static_cast<int>(change_type_) << " (";
switch (change_type_) {
- case CHG_ADD:
- stream << "CHG_ADD)\n";
- break;
- case CHG_REMOVE:
- stream << "CHG_REMOVE)\n";
- break;
- default:
- // Shouldn't be possible.
- stream << "Invalid Value\n";
+ case CHG_ADD:
+ stream << "CHG_ADD)\n";
+ break;
+ case CHG_REMOVE:
+ stream << "CHG_REMOVE)\n";
+ break;
+ default:
+ // Shouldn't be possible.
+ stream << "Invalid Value\n";
}
stream << "Forward Change: " << (forward_change_ ? "yes" : "no")
diff --git a/src/bin/d2/ncr_msg.h b/src/bin/d2/ncr_msg.h
index 23c9537..5e7846b 100644
--- a/src/bin/d2/ncr_msg.h
+++ b/src/bin/d2/ncr_msg.h
@@ -101,7 +101,7 @@ public:
}
private:
- /// @Brief Storage for the DHCID value in unsigned bytes.
+ /// @brief Storage for the DHCID value in unsigned bytes.
std::vector<uint8_t> bytes_;
};
@@ -146,14 +146,16 @@ public:
/// updated.
/// @param ip_address the ip address leased to the given FQDN.
/// @param dhcid the lease client's unique DHCID.
- /// @param ptime a timestamp containing the date/time the lease expires.
+ /// @param lease_expires_on a timestamp containing the date/time the lease
+ /// expires.
/// @param lease_length the amount of time in seconds for which the
/// lease is valid (TTL).
- NameChangeRequest(NameChangeType change_type, bool forward_change,
- bool reverse_change, const std::string& fqdn,
- const std::string& ip_address, const D2Dhcid& dhcid,
+ NameChangeRequest(const NameChangeType change_type,
+ const bool forward_change, const bool reverse_change,
+ const std::string& fqdn, const std::string& ip_address,
+ const D2Dhcid& dhcid,
const boost::posix_time::ptime& lease_expires_on,
- uint32_t lease_length);
+ const uint32_t lease_length);
/// @brief Static method for creating a NameChangeRequest from a
/// buffer containing a marshalled request in a given format.
@@ -176,7 +178,7 @@ public:
///
/// @throw throws NcrMessageError if an error occurs creating new
/// request.
- static NameChangeRequestPtr fromFormat(NameChangeFormat format,
+ static NameChangeRequestPtr fromFormat(const NameChangeFormat format,
isc::util::InputBuffer& buffer);
/// @brief Instance method for marshalling the contents of the request
@@ -195,8 +197,8 @@ public:
/// @param format indicates the data format to use
/// @param buffer is the output buffer to which the request should be
/// marshalled.
- void
- toFormat(NameChangeFormat format, isc::util::OutputBuffer& buffer) const;
+ void toFormat(const NameChangeFormat format,
+ isc::util::OutputBuffer& buffer) const;
/// @brief Static method for creating a NameChangeRequest from a
/// string containing a JSON rendition of a request.
@@ -226,7 +228,7 @@ public:
/// - That at least one of the two direction flags, forward change and
/// reverse change is true.
///
- /// @TODO This is an initial implementation which provides a minimal amount
+ /// @todo This is an initial implementation which provides a minimal amount
/// of validation. FQDN, DHCID, and IP Address members are all currently
/// strings, these may be replaced with richer classes.
///
@@ -244,7 +246,7 @@ public:
/// @brief Sets the change type to the given value.
///
/// @param value is the NameChangeType value to assign to the request.
- void setChangeType(NameChangeType value);
+ void setChangeType(const NameChangeType value);
/// @brief Sets the change type to the value of the given Element.
///
@@ -265,7 +267,7 @@ public:
///
/// @param value contains the new value to assign to the forward change
/// flag
- void setForwardChange(bool value);
+ void setForwardChange(const bool value);
/// @brief Sets the forward change flag to the value of the given Element.
///
@@ -287,7 +289,7 @@ public:
///
/// @param value contains the new value to assign to the reverse change
/// flag
- void setReverseChange(bool value);
+ void setReverseChange(const bool value);
/// @brief Sets the reverse change flag to the value of the given Element.
///
@@ -347,7 +349,7 @@ public:
/// @brief Sets the DHCID based on the given string value.
///
- /// @param string is a string of hexadecimal digits. The format is simply
+ /// @param value is a string of hexadecimal digits. The format is simply
/// a contiguous stream of digits, with no delimiters. For example a string
/// containing "14A3" converts to a byte array containing: 0x14, 0xA3.
///
@@ -420,7 +422,7 @@ public:
/// @brief Sets the lease length to the given value.
///
/// @param value contains the new value to assign to the lease length
- void setLeaseLength(uint32_t value);
+ void setLeaseLength(const uint32_t value);
/// @brief Sets the lease length to the value of the given Element.
///
@@ -440,7 +442,7 @@ public:
/// @brief Sets the request status to the given value.
///
/// @param value contains the new value to assign to request status
- void setStatus(NameChangeStatus value);
+ void setStatus(const NameChangeStatus value);
/// @brief Given a name, finds and returns an element from a map of
/// elements.
@@ -471,8 +473,8 @@ private:
bool reverse_change_;
/// @brief The domain name whose DNS entry(ies) are to be updated.
- /// @TODO Currently, this is a std::string but may be replaced with
- /// dns::Name which provides additional validation and domain name
+ /// @todo Currently, this is a std::string but may be replaced with
+ /// dns::Name which provides additional validation and domain name
/// manipulation.
std::string fqdn_;
@@ -480,7 +482,7 @@ private:
std::string ip_address_;
/// @brief The lease client's unique DHCID.
- /// @TODO Currently, this is uses D2Dhcid it but may be replaced with
+ /// @todo Currently, this is uses D2Dhcid it but may be replaced with
/// dns::DHCID which provides additional validation.
D2Dhcid dhcid_;
diff --git a/src/bin/d2/tests/ncr_unittests.cc b/src/bin/d2/tests/ncr_unittests.cc
index eaab93d..70031c0 100644
--- a/src/bin/d2/tests/ncr_unittests.cc
+++ b/src/bin/d2/tests/ncr_unittests.cc
@@ -220,30 +220,26 @@ TEST(NameChangeRequestTest, constructionTests) {
ncr.reset();
// Verify blank FQDN is detected.
- EXPECT_THROW(ncr.reset(new NameChangeRequest(CHG_ADD, true, true, "",
- "192.168.1.101", dhcid, expiry, 1300)), NcrMessageError);
+ EXPECT_THROW(NameChangeRequest(CHG_ADD, true, true, "",
+ "192.168.1.101", dhcid, expiry, 1300), NcrMessageError);
// Verify that an invalid IP address is detected.
- EXPECT_THROW(ncr.reset(
- new NameChangeRequest(CHG_ADD, true, true, "valid.fqdn",
- "xxx.168.1.101", dhcid, expiry, 1300)), NcrMessageError);
+ EXPECT_THROW(NameChangeRequest(CHG_ADD, true, true, "valid.fqdn",
+ "xxx.168.1.101", dhcid, expiry, 1300), NcrMessageError);
// Verify that a blank DHCID is detected.
D2Dhcid blank_dhcid;
- EXPECT_THROW(ncr.reset(
- new NameChangeRequest(CHG_ADD, true, true, "walah.walah.com",
- "192.168.1.101", blank_dhcid, expiry, 1300)), NcrMessageError);
+ EXPECT_THROW(NameChangeRequest(CHG_ADD, true, true, "walah.walah.com",
+ "192.168.1.101", blank_dhcid, expiry, 1300), NcrMessageError);
// Verify that an invalid lease expiration is detected.
ptime blank_expiry;
- EXPECT_THROW(ncr.reset(
- new NameChangeRequest(CHG_ADD, true, true, "valid.fqdn",
- "192.168.1.101", dhcid, blank_expiry, 1300)), NcrMessageError);
+ EXPECT_THROW(NameChangeRequest(CHG_ADD, true, true, "valid.fqdn",
+ "192.168.1.101", dhcid, blank_expiry, 1300), NcrMessageError);
// Verify that one or both of direction flags must be true.
- EXPECT_THROW(ncr.reset(
- new NameChangeRequest(CHG_ADD, false, false, "valid.fqdn",
- "192.168.1.101", dhcid, expiry, 1300)), NcrMessageError);
+ EXPECT_THROW(NameChangeRequest(CHG_ADD, false, false, "valid.fqdn",
+ "192.168.1.101", dhcid, expiry, 1300), NcrMessageError);
}
@@ -275,9 +271,9 @@ TEST(NameChangeRequestTest, dhcidTest) {
// Fetch the byte vector from the dhcid and verify if equals the expected
// content.
const std::vector<uint8_t>& converted_bytes = dhcid.getBytes();
- EXPECT_EQ(expected_bytes.size(), converted_bytes.size());
- EXPECT_TRUE (std::equal(expected_bytes.begin(),
- expected_bytes.begin()+expected_bytes.size(),
+ EXPECT_EQ(expected_bytes.size(), converted_bytes.size());
+ EXPECT_TRUE (std::equal(expected_bytes.begin(),
+ expected_bytes.begin()+expected_bytes.size(),
converted_bytes.begin()));
// Convert the new dhcid back to string and verify it matches the original
@@ -356,22 +352,10 @@ TEST(NameChangeRequestTest, invalidMsgChecks) {
// NameChangeRequest. The attempt should throw a NcrMessageError.
int num_msgs = sizeof(invalid_msgs)/sizeof(char*);
for (int i = 0; i < num_msgs; i++) {
- bool it_threw = false;
- try {
- NameChangeRequest::fromJSON(invalid_msgs[i]);
- } catch (NcrMessageError& ex) {
- it_threw = true;
- std::cout << "Invalid Message: " << ex.what() << std::endl;
- }
-
- // If it the conversion didn't fail, log the index message and fail
- // the test.
- if (!it_threw) {
- std::cerr << "Invalid Message not caught, message idx: " << i
- << std::endl;
- EXPECT_TRUE(it_threw);
-
- }
+ EXPECT_THROW(NameChangeRequest::fromJSON(invalid_msgs[i]),
+ NcrMessageError) << "Invalid message not caught idx: "
+ << i << std::endl << " text:[" << invalid_msgs[i] << "]"
+ << std::endl;
}
}
@@ -389,21 +373,10 @@ TEST(NameChangeRequestTest, validMsgChecks) {
// NameChangeRequest. The attempt should succeed.
int num_msgs = sizeof(valid_msgs)/sizeof(char*);
for (int i = 0; i < num_msgs; i++) {
- bool it_threw = false;
- try {
- NameChangeRequest::fromJSON(valid_msgs[i]);
- } catch (NcrMessageError& ex) {
- it_threw = true;
- std::cout << "Message Error: " << ex.what() << std::endl;
- }
-
- // If it the conversion failed log the index message and fail
- // the test.
- if (it_threw) {
- std::cerr << "Valid Message failed, message idx: " << i
- << std::endl;
- EXPECT_TRUE(!it_threw);
- }
+ EXPECT_NO_THROW(NameChangeRequest::fromJSON(valid_msgs[i]))
+ << "Valid message failed, message idx: " << i
+ << std::endl << " text:[" << valid_msgs[i] << "]"
+ << std::endl;
}
}
More information about the bind10-changes
mailing list