BIND 10 trac3088, updated. 59bd021c72b3e2d9538b534f3ba00ae4b410da62 [3088] Addressed review changes.
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Dec 17 13:44:16 UTC 2013
The branch, trac3088 has been updated
via 59bd021c72b3e2d9538b534f3ba00ae4b410da62 (commit)
from 70941f1375063c52a43a10831dc9707c6303f55a (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 59bd021c72b3e2d9538b534f3ba00ae4b410da62
Author: Thomas Markwalder <tmark at isc.org>
Date: Tue Dec 17 08:43:10 2013 -0500
[3088] Addressed review changes.
Changes are largely clean up and commentary.
-----------------------------------------------------------------------
Summary of changes:
src/bin/d2/d2_messages.mes | 12 ++++++------
src/bin/d2/nc_add.cc | 25 ++++++++++++++++++++-----
src/bin/d2/nc_add.h | 8 +++++---
src/bin/d2/nc_remove.cc | 25 ++++++++++++++++++++-----
src/bin/d2/nc_remove.h | 14 ++++++++------
src/bin/d2/nc_trans.cc | 15 ++++++++-------
src/bin/d2/nc_trans.h | 6 +++---
src/bin/d2/tests/nc_remove_unittests.cc | 7 +++----
8 files changed, 73 insertions(+), 39 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/d2/d2_messages.mes b/src/bin/d2/d2_messages.mes
index cd21f11..a5bd5ec 100644
--- a/src/bin/d2/d2_messages.mes
+++ b/src/bin/d2/d2_messages.mes
@@ -347,12 +347,12 @@ aborted. This is most likely a configuration issue.
This is a debug message issued after DHCP_DDNS has submitted DNS mapping
additions which were received and accepted by an appropriate DNS server.
-% DHCP_DDNS_ADD_FAILED DHCP_DDNS failed attempting to make DNS mapping additions for this request: %1
+% DHCP_DDNS_ADD_FAILED DHCP_DDNS failed attempting to make DNS mapping additions for this request: %1, event: %2
This is an error message issued after DHCP_DDNS attempts to submit DNS mapping
entry additions have failed. The precise reason for the failure should be
documented in preceding log entries.
-% DHCP_DDNS_FORWARD_REMOVE_ADDRS_BUILD_FAILURE DNS udpate message to remove a forward DNS Address entry could not be constructed for this request: %1 reason: %2
+% DHCP_DDNS_FORWARD_REMOVE_ADDRS_BUILD_FAILURE DNS udpate message to remove a forward DNS Address entry could not be constructed for this request: %1, reason: %2
This is an error message issued when an error occurs attempting to construct
the server bound packet requesting a forward address (A or AAAA) removal. This
is due to invalid data contained in the NameChangeRequest. The request will be
@@ -378,7 +378,7 @@ This is an error message issued when DNSClient returns an unrecognized status
while DHCP_DDNS was removing a forward address mapping. The request will be
aborted. This is most likely a programmatic issue and should be reported.
-% DHCP_DDNS_FORWARD_REMOVE_RRS_BUILD_FAILURE DNS udpate message to remove forward DNS RR entries could not be constructed for this request: %1 reason: %2
+% DHCP_DDNS_FORWARD_REMOVE_RRS_BUILD_FAILURE DNS udpate message to remove forward DNS RR entries could not be constructed for this request: %1, reason: %2
This is an error message issued when an error occurs attempting to construct
the server bound packet requesting forward RR (DHCID RR) removal. This is due
to invalid data contained in the NameChangeRequest. The request will be aborted.This is most likely a configuration issue.
@@ -403,7 +403,7 @@ This is an error message issued when DNSClient returns an unrecognized status
while DHCP_DDNS was removing forward RRs. The request will be aborted. This is
most likely a programmatic issue and should be reported.
-% DHCP_DDNS_REVERSE_REMOVE_BUILD_FAILURE DNS update message to remove a reverse DNS entry could not be constructed from this request: %1 reason: %2
+% DHCP_DDNS_REVERSE_REMOVE_BUILD_FAILURE DNS update message to remove a reverse DNS entry could not be constructed from this request: %1, reason: %2
This is an error message issued when an error occurs attempting to construct
the server bound packet requesting a reverse PTR removal. This is
due to invalid data contained in the NameChangeRequest. The request will be
@@ -433,8 +433,8 @@ aborted. This is most likely a programmatic issue and should be reported.
This is a debug message issued after DHCP_DDNS has submitted DNS mapping
removals which were received and accepted by an appropriate DNS server.
-% DHCP_DDNS_REMOVE_FAILED DHCP_DDNS failed attempting to make DNS mapping removals for this request: %1
+% DHCP_DDNS_REMOVE_FAILED DHCP_DDNS failed attempting to make DNS mapping removals for this request: %1, event: %2
This is an error message issued after DHCP_DDNS attempts to submit DNS mapping
-entry removals have failed. There precise reason for the failure should be
+entry removals have failed. The precise reason for the failure should be
documented in preceding log entries.
diff --git a/src/bin/d2/nc_add.cc b/src/bin/d2/nc_add.cc
index 3773d6e..da20763 100644
--- a/src/bin/d2/nc_add.cc
+++ b/src/bin/d2/nc_add.cc
@@ -61,10 +61,18 @@ NameAddTransaction::defineEvents() {
void
NameAddTransaction::verifyEvents() {
- // Call superclass impl first.
+ // Call superclass implementation first to verify its events. These are
+ // events common to all transactions, and they must be defined.
+ // SELECT_SERVER_EVT
+ // SERVER_SELECTED_EVT
+ // SERVER_IO_ERROR_EVT
+ // NO_MORE_SERVERS_EVT
+ // IO_COMPLETED_EVT
+ // UPDATE_OK_EVT
+ // UPDATE_FAILED_EVT
NameChangeTransaction::verifyEvents();
- // Verify NameAddTransaction events.
+ // Verify NameAddTransaction events by attempting to fetch them.
getEvent(FQDN_IN_USE_EVT);
getEvent(FQDN_NOT_IN_USE_EVT);
}
@@ -102,10 +110,16 @@ NameAddTransaction::defineStates() {
}
void
NameAddTransaction::verifyStates() {
- // Call superclass impl first.
+ // Call superclass implementation first to verify its states. These are
+ // states common to all transactions, and they must be defined.
+ // READY_ST
+ // SELECTING_FWD_SERVER_ST
+ // SELECTING_REV_SERVER_ST
+ // PROCESS_TRANS_OK_ST
+ // PROCESS_TRANS_FAILED_ST
NameChangeTransaction::verifyStates();
- // Verify NameAddTransaction states.
+ // Verify NameAddTransaction states by attempting to fetch them.
getState(ADDING_FWD_ADDRS_ST);
getState(REPLACING_FWD_ADDRS_ST);
getState(REPLACING_REV_PTRS_ST);
@@ -542,7 +556,8 @@ NameAddTransaction::processAddFailedHandler() {
switch(getNextEvent()) {
case UPDATE_FAILED_EVT:
case NO_MORE_SERVERS_EVT:
- LOG_ERROR(dctl_logger, DHCP_DDNS_ADD_FAILED).arg(getNcr()->toText());
+ LOG_ERROR(dctl_logger, DHCP_DDNS_ADD_FAILED).arg(getNcr()->toText())
+ .arg(getContextStr());
setNcrStatus(dhcp_ddns::ST_FAILED);
endModel();
break;
diff --git a/src/bin/d2/nc_add.h b/src/bin/d2/nc_add.h
index a9e748f..1fe167b 100644
--- a/src/bin/d2/nc_add.h
+++ b/src/bin/d2/nc_add.h
@@ -110,7 +110,8 @@ protected:
/// @brief Validates the contents of the set of events.
///
/// Invokes NameChangeTransaction's implementation and then verifies the
- /// Add transaction's events.
+ /// Add transaction's. This tests that the needed events are in the event
+ /// dictionary.
///
/// @throw StateModelError if an event value is undefined.
virtual void verifyEvents();
@@ -126,7 +127,8 @@ protected:
/// @brief Validates the contents of the set of states.
///
/// Invokes NameChangeTransaction's implementation and then verifies the
- /// Add transaction's states.
+ /// Add transaction's states. This tests that the needed states are in the
+ /// state dictionary.
///
/// @throw StateModelError if an event value is undefined.
virtual void verifyStates();
@@ -441,7 +443,7 @@ protected:
void buildReplaceRevPtrsRequest();
};
-/// @brief Defines a pointer to a NameChangeTransaction.
+/// @brief Defines a pointer to a NameAddTransaction.
typedef boost::shared_ptr<NameAddTransaction> NameAddTransactionPtr;
} // namespace isc::d2
diff --git a/src/bin/d2/nc_remove.cc b/src/bin/d2/nc_remove.cc
index 401333e..d7135bd 100644
--- a/src/bin/d2/nc_remove.cc
+++ b/src/bin/d2/nc_remove.cc
@@ -58,10 +58,18 @@ NameRemoveTransaction::defineEvents() {
void
NameRemoveTransaction::verifyEvents() {
- // Call superclass impl first.
+ // Call superclass implementation first to verify its events. These are
+ // events common to all transactions, and they must be defined.
+ // SELECT_SERVER_EVT
+ // SERVER_SELECTED_EVT
+ // SERVER_IO_ERROR_EVT
+ // NO_MORE_SERVERS_EVT
+ // IO_COMPLETED_EVT
+ // UPDATE_OK_EVT
+ // UPDATE_FAILED_EVT
NameChangeTransaction::verifyEvents();
- // Verify NameRemoveTransaction events.
+ // Verify NameRemoveTransaction events by attempting to fetch them.
// Currently NameRemoveTransaction does not define any events.
// getEvent(TBD_EVENT);
}
@@ -106,10 +114,16 @@ NameRemoveTransaction::defineStates() {
void
NameRemoveTransaction::verifyStates() {
- // Call superclass impl first.
+ // Call superclass implementation first to verify its states. These are
+ // states common to all transactions, and they must be defined.
+ // READY_ST
+ // SELECTING_FWD_SERVER_ST
+ // SELECTING_REV_SERVER_ST
+ // PROCESS_TRANS_OK_ST
+ // PROCESS_TRANS_FAILED_ST
NameChangeTransaction::verifyStates();
- // Verify NameRemoveTransaction states.
+ // Verify NameRemoveTransaction states by attempting to fetch them.
getState(REMOVING_FWD_ADDRS_ST);
getState(REMOVING_FWD_RRS_ST);
getState(REMOVING_REV_PTRS_ST);
@@ -551,7 +565,8 @@ NameRemoveTransaction::processRemoveFailedHandler() {
case UPDATE_FAILED_EVT:
case NO_MORE_SERVERS_EVT:
case SERVER_IO_ERROR_EVT:
- LOG_ERROR(dctl_logger, DHCP_DDNS_REMOVE_FAILED).arg(getNcr()->toText());
+ LOG_ERROR(dctl_logger, DHCP_DDNS_REMOVE_FAILED).arg(getNcr()->toText())
+ .arg(getEventLabel(getNextEvent()));
setNcrStatus(dhcp_ddns::ST_FAILED);
endModel();
break;
diff --git a/src/bin/d2/nc_remove.h b/src/bin/d2/nc_remove.h
index 3790a7c..f7b0dcc 100644
--- a/src/bin/d2/nc_remove.h
+++ b/src/bin/d2/nc_remove.h
@@ -95,7 +95,7 @@ public:
virtual ~NameRemoveTransaction();
protected:
- /// @brief Removes events defined by NameRemoveTransaction to the event set.
+ /// @brief Adds events defined by NameRemoveTransaction to the event set.
///
/// Invokes NameChangeTransaction's implementation and then defines the
/// events unique to NCR Remove transaction processing.
@@ -106,12 +106,13 @@ protected:
/// @brief Validates the contents of the set of events.
///
/// Invokes NameChangeTransaction's implementation and then verifies the
- /// Remove transaction's events.
+ /// Remove transaction's events. This tests that the needed events are in
+ /// the event dictionary.
///
/// @throw StateModelError if an event value is undefined.
virtual void verifyEvents();
- /// @brief Removes states defined by NameRemoveTransaction to the state set.
+ /// @brief Adds states defined by NameRemoveTransaction to the state set.
///
/// Invokes NameChangeTransaction's implementation and then defines the
/// states unique to NCR Remove transaction processing.
@@ -122,7 +123,8 @@ protected:
/// @brief Validates the contents of the set of states.
///
/// Invokes NameChangeTransaction's implementation and then verifies the
- /// Remove transaction's states.
+ /// Remove transaction's states. This tests that the needed states are in
+ /// the state dictionary.
///
/// @throw StateModelError if an event value is undefined.
virtual void verifyStates();
@@ -134,7 +136,7 @@ protected:
///
/// The READY_ST is the state the model transitions into when the inherited
/// method, startTransaction() is invoked. This handler, therefore, is the
- /// entry point into the state model execution.h Its primary task is to
+ /// entry point into the state model execution. Its primary task is to
/// determine whether to start with a forward DNS change or a reverse DNS
/// change.
///
@@ -424,7 +426,7 @@ protected:
void buildRemoveRevPtrsRequest();
};
-/// @brief Defines a pointer to a NameChangeTransaction.
+/// @brief Defines a pointer to a NameRemoveTransaction.
typedef boost::shared_ptr<NameRemoveTransaction> NameRemoveTransactionPtr;
diff --git a/src/bin/d2/nc_trans.cc b/src/bin/d2/nc_trans.cc
index 0d9fc77..e264df5 100644
--- a/src/bin/d2/nc_trans.cc
+++ b/src/bin/d2/nc_trans.cc
@@ -263,12 +263,13 @@ NameChangeTransaction::addLeaseAddressRdata(dns::RRsetPtr& rrset) {
try {
// Manufacture an RData from the lease address then add it to the RR.
- dns::rdata::ConstRdataPtr rdata;
if (ncr_->isV4()) {
- rdata.reset(new dns::rdata::in::A(ncr_->getIpAddress()));
+ dns::rdata::ConstRdataPtr rdata(new dns::rdata::in::
+ A(ncr_->getIpAddress()));
rrset->addRdata(rdata);
} else {
- rdata.reset(new dns::rdata::in::AAAA(ncr_->getIpAddress()));
+ dns::rdata::ConstRdataPtr rdata(new dns::rdata::in::
+ AAAA(ncr_->getIpAddress()));
rrset->addRdata(rdata);
}
} catch (const std::exception& ex) {
@@ -286,10 +287,10 @@ NameChangeTransaction::addDhcidRdata(dns::RRsetPtr& rrset) {
}
try {
- dns::rdata::ConstRdataPtr rdata;
const std::vector<uint8_t>& ncr_dhcid = ncr_->getDhcid().getBytes();
util::InputBuffer buffer(ncr_dhcid.data(), ncr_dhcid.size());
- rdata.reset(new dns::rdata::in::DHCID(buffer, ncr_dhcid.size()));
+ dns::rdata::ConstRdataPtr rdata (new dns::rdata::in::
+ DHCID(buffer, ncr_dhcid.size()));
rrset->addRdata(rdata);
} catch (const std::exception& ex) {
isc_throw(NameChangeTransactionError, "Cannot add DCHID rdata: "
@@ -306,8 +307,8 @@ NameChangeTransaction::addPtrRdata(dns::RRsetPtr& rrset) {
}
try {
- dns::rdata::ConstRdataPtr rdata;
- rdata.reset(new dns::rdata::generic::PTR(getNcr()->getFqdn()));
+ dns::rdata::ConstRdataPtr rdata(new dns::rdata::generic::
+ PTR(getNcr()->getFqdn()));
rrset->addRdata(rdata);
} catch (const std::exception& ex) {
isc_throw(NameChangeTransactionError, "Cannot add PTR rdata: "
diff --git a/src/bin/d2/nc_trans.h b/src/bin/d2/nc_trans.h
index 7415d88..86a89d7 100644
--- a/src/bin/d2/nc_trans.h
+++ b/src/bin/d2/nc_trans.h
@@ -277,7 +277,7 @@ protected:
/// If the maximum number of attempts has been reached, it will transition
/// to the given state with a next event of SERVER_IO_ERROR_EVT.
///
- /// @param server_sel_state State to transition to if maximum attempts
+ /// @param fail_to_state State to transition to if maximum attempts
/// have been tried.
///
void retryTransition(const int fail_to_state);
@@ -435,13 +435,13 @@ public:
/// @brief Fetches the forward DdnsDomain.
///
- /// @return A pointer reference to the forward DdnsDomain. If
+ /// @return A pointer reference to the forward DdnsDomain. If
/// the request does not include a forward change, the pointer will empty.
DdnsDomainPtr& getForwardDomain();
/// @brief Fetches the reverse DdnsDomain.
///
- /// @return A pointer reference to the reverse DdnsDomain. If
+ /// @return A pointer reference to the reverse DdnsDomain. If
/// the request does not include a reverse change, the pointer will empty.
DdnsDomainPtr& getReverseDomain();
diff --git a/src/bin/d2/tests/nc_remove_unittests.cc b/src/bin/d2/tests/nc_remove_unittests.cc
index 2ae9326..2f12feb 100644
--- a/src/bin/d2/tests/nc_remove_unittests.cc
+++ b/src/bin/d2/tests/nc_remove_unittests.cc
@@ -33,9 +33,9 @@ namespace {
class NameRemoveStub : public NameRemoveTransaction {
public:
NameRemoveStub(IOServicePtr& io_service,
- dhcp_ddns::NameChangeRequestPtr& ncr,
- DdnsDomainPtr& forward_domain,
- DdnsDomainPtr& reverse_domain)
+ dhcp_ddns::NameChangeRequestPtr& ncr,
+ DdnsDomainPtr& forward_domain,
+ DdnsDomainPtr& reverse_domain)
: NameRemoveTransaction(io_service, ncr, forward_domain,
reverse_domain),
simulate_send_exception_(false),
@@ -328,7 +328,6 @@ TEST_F(NameRemoveTransactionTest, buildRemoveFwdAddressRequest) {
// and then verify the request contents.
NameRemoveStubPtr name_remove;
ASSERT_NO_THROW(name_remove = makeTransaction4(FORWARD_CHG));
- (name_remove->buildRemoveFwdAddressRequest());
ASSERT_NO_THROW(name_remove->buildRemoveFwdAddressRequest());
checkRemoveFwdAddressRequest(*name_remove);
More information about the bind10-changes
mailing list