BIND 10 trac1198, updated. b4471621912e7518088b106d829a8431a6c4ea97 [1198] proposed further refactor 7: recovered logs, with defining new ones for each subcase of find().
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Nov 23 04:29:16 UTC 2011
The branch, trac1198 has been updated
via b4471621912e7518088b106d829a8431a6c4ea97 (commit)
via c05dc7099e4ed686ad1af573e6795a751d020025 (commit)
via 1beaacf4d924392323bd08a0c7aed65e9324e092 (commit)
via 84d0d090f5452410a58d8f8503c61d81ec85f2f4 (commit)
via 35015af5525965fdb421a856ffb01fb1ab8a7ad4 (commit)
via 25b7595f3f1b158bd6278cea3c4dd0d6eeca8a2f (commit)
via cf8596b58bd57f4ebfff7d83d24294eaed38f7bf (commit)
via b77f5d1f891daf4c24024b44db6a7502e2728d2a (commit)
from 0337c552ff717ee890ae784451668ce3d789650f (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 b4471621912e7518088b106d829a8431a6c4ea97
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Nov 22 20:13:03 2011 -0800
[1198] proposed further refactor 7: recovered logs, with defining new
ones for each subcase of find().
commit c05dc7099e4ed686ad1af573e6795a751d020025
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Nov 22 20:03:33 2011 -0800
[1198] proposed further refactor 6: we can even remove 'origin' (and thus
eliminating the need for making a copy of it)
commit 1beaacf4d924392323bd08a0c7aed65e9324e092
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Nov 22 19:20:18 2011 -0800
[1198] proposed further refactor 5: replaced variables used only once with
their definitions. define 'origin' immediately before it's used, which
also makes the method a bit more efficient in the delegation case.
commit 84d0d090f5452410a58d8f8503c61d81ec85f2f4
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Nov 22 19:05:52 2011 -0800
[1198] proposed further refactor 4: moved the "no name"
(NXDOMAIN/empty name/wildcard) case to a separate method to make the main
method even more concise.
commit 35015af5525965fdb421a856ffb01fb1ab8a7ad4
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Nov 22 18:43:11 2011 -0800
[1198] proposed further refactor 3: pass DelegationSearchResult to
findWildcardMatch() instead of first_ns and last_known, thereby eliminating
the need for defining and using these variables.
commit 25b7595f3f1b158bd6278cea3c4dd0d6eeca8a2f
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Nov 22 15:18:20 2011 -0800
[1198] proposed further refactor 2: eliminated the need for 'records_found'
in findWildcardMatch(). as a result we can now use the common FindResult
struct as the return type of findWildcardMatch().
commit cf8596b58bd57f4ebfff7d83d24294eaed38f7bf
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Tue Nov 22 14:46:46 2011 -0800
[1198] proposed further refactor 1: simplified find() further by replacing
code logic with intermediate state variable with direct return.
(as a result of some log points were lost, which should be recovered
later)
commit b77f5d1f891daf4c24024b44db6a7502e2728d2a
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Nov 21 22:48:33 2011 -0800
[1198] a trivial cleanup: folded multiple lines into one when they were
sufficiently short.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/database.cc | 258 ++++++++++++++--------------------
src/lib/datasrc/database.h | 45 ++----
src/lib/datasrc/datasrc_messages.mes | 6 +
3 files changed, 130 insertions(+), 179 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index d177265..478a689 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -470,21 +470,19 @@ DatabaseClient::Finder::findDelegationPoint(const isc::dns::Name& name,
last_known));
}
-DatabaseClient::Finder::WildcardSearchResult
+ZoneFinder::FindResult
DatabaseClient::Finder::findWildcardMatch(
const isc::dns::Name& name, const isc::dns::RRType& type,
- const FindOptions options, const isc::dns::ConstRRsetPtr& first_ns,
- size_t last_known)
+ const FindOptions options, const DelegationSearchResult& dresult)
{
// Result of search
isc::dns::ConstRRsetPtr result_rrset;
- ZoneFinder::Result result_status = SUCCESS;
+ ZoneFinder::Result result_status = NXDOMAIN; // in case we don't find any
// Search options
const bool dnssec_data((options & FIND_DNSSEC) != 0);
// Other
- bool records_found = false; // Distinguish between NXDOMAIN and NXRRSET
WantedTypes final_types(FINAL_TYPES());
final_types.insert(type);
@@ -492,7 +490,7 @@ DatabaseClient::Finder::findWildcardMatch(
// We can start at the last known non-empty domain and work up. We remove
// labels one by one and look for the wildcard there, up to the
// first non-empty domain.
- for (size_t i = 1; i <= name.getLabelCount() - last_known; ++i) {
+ for (size_t i = 1; i <= name.getLabelCount() - dresult.last_known; ++i) {
// Construct the name with *
const Name superdomain(name.split(i));
@@ -504,17 +502,16 @@ DatabaseClient::Finder::findWildcardMatch(
FoundRRsets found = getRRsets(wildcard, final_types, true,
&construct_name);
if (found.first) {
- if (first_ns) {
+ if (dresult.first_ns) {
// In case we are under NS, we don't wildcard-match, but return
// delegation
- result_rrset = first_ns;
+ result_rrset = dresult.first_ns;
result_status = DELEGATION;
- records_found = true;
LOG_DEBUG(logger, DBG_TRACE_DETAILED,
DATASRC_DATABASE_WILDCARD_CANCEL_NS).
arg(accessor_->getDBName()).arg(wildcard).
- arg(first_ns->getName());
+ arg(dresult.first_ns->getName());
} else if (!hasSubdomains(name.split(i - 1).toText())) {
@@ -522,7 +519,6 @@ DatabaseClient::Finder::findWildcardMatch(
// go up only to first existing domain, but it could be an empty
// non-terminal. In that case, we need to cancel the match.
- records_found = true;
const FoundIterator cni(found.second.find(RRType::CNAME()));
const FoundIterator nsi(found.second.find(RRType::NS()));
const FoundIterator nci(found.second.find(RRType::NSEC()));
@@ -542,8 +538,7 @@ DatabaseClient::Finder::findWildcardMatch(
} else {
// NXRRSET case in the wildcard
result_status = WILDCARD_NXRRSET;
- if (dnssec_data &&
- nci != found.second.end()) {
+ if (dnssec_data && nci != found.second.end()) {
// User wants a proof the wildcard doesn't contain it
//
// However, we need to get the RRset in the name of the
@@ -555,24 +550,22 @@ DatabaseClient::Finder::findWildcardMatch(
}
}
- LOG_DEBUG(logger, DBG_TRACE_DETAILED,
- DATASRC_DATABASE_WILDCARD).
- arg(accessor_->getDBName()).arg(wildcard).
- arg(name);
+ LOG_DEBUG(logger, DBG_TRACE_DETAILED,DATASRC_DATABASE_WILDCARD).
+ arg(accessor_->getDBName()).arg(wildcard).arg(name);
} else {
LOG_DEBUG(logger, DBG_TRACE_DETAILED,
DATASRC_DATABASE_WILDCARD_CANCEL_SUB).
arg(accessor_->getDBName()).arg(wildcard).
arg(name).arg(superdomain);
+ result_status = NXDOMAIN;
}
break;
} else if (hasSubdomains(wildcard)) {
// Empty non-terminal asterisk
- records_found = true;
LOG_DEBUG(logger, DBG_TRACE_DETAILED,
DATASRC_DATABASE_WILDCARD_EMPTY).
- arg(accessor_->getDBName()).arg(wildcard).
- arg(name);
+ arg(accessor_->getDBName()).arg(wildcard).arg(name);
+ result_status = NXRRSET;
if (dnssec_data) {
result_rrset = findNSECCover(Name(wildcard));
if (result_rrset) {
@@ -582,7 +575,59 @@ DatabaseClient::Finder::findWildcardMatch(
break;
}
}
- return (WildcardSearchResult(result_status, result_rrset, records_found));
+ return (ZoneFinder::FindResult(result_status, result_rrset));
+}
+
+ZoneFinder::FindResult
+DatabaseClient::Finder::findNoNameResult(const Name& name, const RRType& type,
+ FindOptions options,
+ const DelegationSearchResult& dresult)
+{
+ const bool dnssec_data((options & FIND_DNSSEC) != 0);
+
+ // We know the database doesn't have any record for the given name.
+ // But check if something lives below this domain and if so,
+ // pretend something is here as well.
+ if (hasSubdomains(name.toText())) {
+ LOG_DEBUG(logger, DBG_TRACE_DETAILED,
+ DATASRC_DATABASE_FOUND_EMPTY_NONTERMINAL).
+ arg(accessor_->getDBName()).arg(name);
+ return (FindResult(NXRRSET, dnssec_data ? findNSECCover(name) :
+ ConstRRsetPtr()));
+ } else if ((options & NO_WILDCARD) != 0) {
+ // If wildcard check is disabled, the search should terminate
+ // with NXDOMAIN.
+ return (FindResult(NXDOMAIN, dnssec_data ? findNSECCover(name) :
+ ConstRRsetPtr()));
+ } else {
+ // It's not an empty non-terminal so check for wildcards.
+ const ZoneFinder::FindResult wresult =
+ findWildcardMatch(name, type, options, dresult);
+ if (wresult.code == NXDOMAIN && dnssec_data) {
+ // If the result is NXDOMAIN case and the caller wanted
+ // DNSSEC data, try getting the NSEC record.
+ return (FindResult(NXDOMAIN, findNSECCover(name)));
+ }
+ return (FindResult(wresult.code, wresult.rrset));
+ }
+}
+
+ZoneFinder::FindResult
+DatabaseClient::Finder::logAndCreateResult(const Name& name,
+ const RRType& type,
+ ZoneFinder::Result code,
+ ConstRRsetPtr rrset,
+ const isc::log::MessageID& log_id)
+{
+ if (rrset) {
+ LOG_DEBUG(logger, DBG_TRACE_DETAILED, log_id).
+ arg(accessor_->getDBName()).arg(name).arg(getClass()).
+ arg(type).arg(*rrset);
+ } else {
+ LOG_DEBUG(logger, DBG_TRACE_DETAILED, log_id).
+ arg(accessor_->getDBName()).arg(name).arg(type).arg(getClass());
+ }
+ return (ZoneFinder::FindResult(code, rrset));
}
ZoneFinder::FindResult
@@ -594,143 +639,56 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_FIND_RECORDS)
.arg(accessor_->getDBName()).arg(name).arg(type);
- const bool glue_ok((options & FIND_GLUE_OK) != 0);
- const bool dnssec_data((options & FIND_DNSSEC) != 0);
-
- bool records_found = false; // Distinguish between NXDOMAIN and NXRRSET
- bool get_cover = false;
- isc::dns::ConstRRsetPtr result_rrset;
- ZoneFinder::Result result_status = SUCCESS;
- const Name origin(getOrigin());
-
- // First stage: go throught all superdomains from the origin down,
+ // First stage: go through all superdomains from the origin down,
// searching for nodes that indicate a delegation (NS or DNAME).
const DelegationSearchResult dresult = findDelegationPoint(name, options);
- result_status = dresult.code;
- result_rrset = dresult.rrset;
-
- // In case we are in GLUE_OK mode and start matching wildcards,
- // we can't do it under NS, so we store it here to check
- const isc::dns::ConstRRsetPtr first_ns = dresult.first_ns;
- size_t last_known = dresult.last_known;
-
- if (!result_rrset) { // Only if we didn't find a redirect already
-
- // Try getting the final result and extract it
- // It is special if there's a CNAME or NS, DNAME is ignored here
- // And we don't consider the NS in origin
- WantedTypes final_types(FINAL_TYPES());
- final_types.insert(type);
- const FoundRRsets found = getRRsets(name.toText(), final_types,
- name != origin);
- records_found = found.first;
-
- // NS records, CNAME record and Wanted Type records
- const FoundIterator nsi(found.second.find(RRType::NS()));
- const FoundIterator cni(found.second.find(RRType::CNAME()));
- const FoundIterator wti(found.second.find(type));
-
- if (name != origin && !glue_ok && nsi != found.second.end()) {
- // There's a delegation at the exact node.
- LOG_DEBUG(logger, DBG_TRACE_DETAILED,
- DATASRC_DATABASE_FOUND_DELEGATION_EXACT).
- arg(accessor_->getDBName()).arg(name);
- result_status = DELEGATION;
- result_rrset = nsi->second;
-
- } else if (type != isc::dns::RRType::CNAME() &&
- cni != found.second.end()) {
- // A CNAME here
- result_status = CNAME;
- result_rrset = cni->second;
- if (result_rrset->getRdataCount() != 1) {
- isc_throw(DataSourceError, "CNAME with " <<
- result_rrset->getRdataCount() <<
- " rdata at " << name << ", expected 1");
- }
-
- } else if (wti != found.second.end()) {
- // Just get the answer
- result_rrset = wti->second;
-
- } else if (!records_found) {
- // Nothing lives here.
- // But check if something lives below this domain and if so,
- // pretend something is here as well.
- if (hasSubdomains(name.toText())) {
- LOG_DEBUG(logger, DBG_TRACE_DETAILED,
- DATASRC_DATABASE_FOUND_EMPTY_NONTERMINAL).
- arg(accessor_->getDBName()).arg(name);
- records_found = true;
- get_cover = dnssec_data;
-
- } else if ((options & NO_WILDCARD) != 0) {
- // If wildcard check is disabled, the search will ultimately
- // terminate with NXDOMAIN. If DNSSEC is enabled, flag that
- // we need to get the NSEC records to prove this.
- if (dnssec_data) {
- get_cover = true;
- }
+ if (dresult.rrset) {
+ return (FindResult(dresult.code, dresult.rrset));
+ }
- } else {
- // It's not an empty non-terminal so check for wildcards.
- // We remove labels one by one and look for the wildcard there.
- // Go up to first non-empty domain.
- const WildcardSearchResult wresult =
- findWildcardMatch(name, type, options, first_ns,
- last_known);
- result_status = wresult.code;
- result_rrset = wresult.rrset;
- records_found = wresult.records_found;
-
- // This is the NXDOMAIN case (nothing found anywhere). If
- // they want DNSSEC data, try getting the NSEC record
- if (dnssec_data && !records_found) {
- get_cover = true;
- }
- }
- } else if (dnssec_data) {
- // This is the "usual" NXRRSET case
- // So in case they want DNSSEC, provide the NSEC
- // (which should be available already here)
- result_status = NXRRSET;
+ // Try getting the final result and extract it
+ // It is special if there's a CNAME or NS, DNAME is ignored here
+ // And we don't consider the NS in origin
+ const bool is_origin = (name == getOrigin());
+ WantedTypes final_types(FINAL_TYPES());
+ final_types.insert(type);
+ const FoundRRsets found(getRRsets(name.toText(), final_types, !is_origin));
+
+ // NS records, CNAME record and Wanted Type records
+ const FoundIterator nsi(found.second.find(RRType::NS()));
+ const FoundIterator cni(found.second.find(RRType::CNAME()));
+ const FoundIterator wti(found.second.find(type));
+
+ if (!is_origin && (options & FIND_GLUE_OK) == 0 &&
+ nsi != found.second.end()) { // delegation at the exact node
+ return (logAndCreateResult(name, type, DELEGATION, nsi->second,
+ DATASRC_DATABASE_FOUND_DELEGATION_EXACT));
+ } else if (type != RRType::CNAME() && cni != found.second.end()) { // CNAME
+ if (cni->second->getRdataCount() != 1) {
+ isc_throw(DataSourceError, "CNAME with " <<
+ cni->second->getRdataCount() <<
+ " rdata at " << name << ", expected 1");
+ }
+ return (logAndCreateResult(name, type, CNAME, cni->second,
+ DATASRC_DATABASE_FOUND_CNAME));
+ } else if (wti != found.second.end()) { // normal answer
+ return (logAndCreateResult(name, type, SUCCESS, wti->second,
+ DATASRC_DATABASE_FOUND_RRSET));
+ } else if (!found.first) { // NXDOMAIN, empty name, wildcard
+ return (findNoNameResult(name, type, options, dresult));
+ } else {
+ // This is the "usual" NXRRSET case. So in case they want DNSSEC,
+ // provide the NSEC (which should be available already here)
+ if ((options & FIND_DNSSEC) != 0) {
const FoundIterator nci(found.second.find(RRType::NSEC()));
if (nci != found.second.end()) {
- result_rrset = nci->second;
- }
- }
- }
-
- if (!result_rrset) {
- if (result_status == SUCCESS) {
- // Should we look for NSEC covering the name?
- if (get_cover) {
- result_rrset = findNSECCover(name);
- if (result_rrset) {
- result_status = NXDOMAIN;
- }
- }
- // Something is not here and we didn't decide yet what
- if (records_found) {
- logger.debug(DBG_TRACE_DETAILED,
- DATASRC_DATABASE_FOUND_NXRRSET)
- .arg(accessor_->getDBName()).arg(name)
- .arg(getClass()).arg(type);
- result_status = NXRRSET;
- } else {
- logger.debug(DBG_TRACE_DETAILED,
- DATASRC_DATABASE_FOUND_NXDOMAIN)
- .arg(accessor_->getDBName()).arg(name)
- .arg(getClass()).arg(type);
- result_status = NXDOMAIN;
+ return (logAndCreateResult(name, type, NXRRSET, nci->second,
+ DATASRC_DATABASE_FOUND_NXRRSET_NSEC));
}
}
- } else {
- logger.debug(DBG_TRACE_DETAILED,
- DATASRC_DATABASE_FOUND_RRSET)
- .arg(accessor_->getDBName()).arg(*result_rrset);
+ return (logAndCreateResult(name, type, NXRRSET, ConstRRsetPtr(),
+ DATASRC_DATABASE_FOUND_NXRRSET));
}
- return (FindResult(result_status, result_rrset));
}
Name
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index ecb8dd4..3e9a8b8 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -25,6 +25,7 @@
#include <dns/rrtype.h>
#include <datasrc/client.h>
+#include <datasrc/logger.h>
#include <dns/name.h>
#include <exceptions/exceptions.h>
@@ -849,32 +850,6 @@ public:
const size_t last_known; ///< No. labels in last non-empty domain
};
- /// \brief Search result of \c findWildcard().
- ///
- /// This is a tuple combining the result of the search - a status code
- /// and a pointer to the RRset found - together with additional
- /// information needed for subsequent processing: if there is not a
- /// match for the data sought, then whether there is no match for the
- /// wildcard or where there is a match, but no RRs of the type
- /// requested.
- ///
- /// Note that the code and rrset elements are the same as that in
- /// the \c ZoneFinder::FindResult struct: this structure could be
- /// derived from that one, but as it is used just once in the code and
- /// will never be treated as a \c FindResult, the obscurity involved in
- /// deriving it from a parent class was deemed not worthwhile.
- struct WildcardSearchResult {
- WildcardSearchResult(const ZoneFinder::Result param_code,
- const isc::dns::ConstRRsetPtr param_rrset,
- const bool param_found) :
- code(param_code), rrset(param_rrset),
- records_found(param_found)
- {}
- const ZoneFinder::Result code; ///< Result code
- const isc::dns::ConstRRsetPtr rrset; ///< Pointer to RRset found
- const bool records_found; ///< true => NXRRSET
- };
-
private:
boost::shared_ptr<DatabaseAccessor> accessor_;
const int zone_id_;
@@ -997,11 +972,23 @@ public:
* is no match is an indication as to whether there was an
* NXDOMAIN or an NXRRSET.
*/
- WildcardSearchResult
- findWildcardMatch(
+ FindResult findWildcardMatch(
const isc::dns::Name& name,
const isc::dns::RRType& type, const FindOptions options,
- const isc::dns::ConstRRsetPtr& first_ns, size_t last_known);
+ const DelegationSearchResult& dresult);
+
+ /// To be documented.
+ FindResult findNoNameResult(const isc::dns::Name& name,
+ const isc::dns::RRType& type,
+ FindOptions options,
+ const DelegationSearchResult& dresult);
+
+ // To be documented.
+ FindResult logAndCreateResult(const isc::dns::Name& name,
+ const isc::dns::RRType& type,
+ ZoneFinder::Result code,
+ isc::dns::ConstRRsetPtr rrset,
+ const isc::log::MessageID& log_id);
/**
* \brief Checks if something lives below this domain.
diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes
index 04ad610..d59454a 100644
--- a/src/lib/datasrc/datasrc_messages.mes
+++ b/src/lib/datasrc/datasrc_messages.mes
@@ -110,6 +110,12 @@ The data returned by the database backend contained data for the given domain
name, and it either matches the type or has a relevant type. The RRset that is
returned is printed.
+% DATASRC_DATABASE_FOUND_CNAME search in datasource %1 for %2/%3/%4 resulted in CNAME %5
+(TBD)
+
+% DATASRC_DATABASE_FOUND_NXRRSET_NSEC search in datasource %1 for %2/%3/%4 resulted in RRset %5
+(TBD)
+
% DATASRC_DATABASE_ITERATE iterating zone %1
The program is reading the whole zone, eg. not searching for data, but going
through each of the RRsets there.
More information about the bind10-changes
mailing list