BIND 10 trac1747, updated. 83474d8f558bebb4be0b0f75e843fab993ee3713 [1747] addressed review comments
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Mar 13 13:16:51 UTC 2012
The branch, trac1747 has been updated
via 83474d8f558bebb4be0b0f75e843fab993ee3713 (commit)
from 961f28cdfb0f5cc1f5be9613d5f60a7801c92b0a (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 83474d8f558bebb4be0b0f75e843fab993ee3713
Author: Jelte Jansen <jelte at isc.org>
Date: Tue Mar 13 12:50:12 2012 +0100
[1747] addressed review comments
see
http://bind10.isc.org/ticket/1747?replyto=7#comment:9
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/query.cc | 71 ++++++++++++++++++++++++-------------------------
src/bin/auth/query.h | 34 +++++++++++++++--------
2 files changed, 57 insertions(+), 48 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/query.cc b/src/bin/auth/query.cc
index e79bef7..424d05b 100644
--- a/src/bin/auth/query.cc
+++ b/src/bin/auth/query.cc
@@ -133,14 +133,14 @@ Query::addNXDOMAINProofByNSEC(ZoneFinder& finder, ConstRRsetPtr nsec) {
// and the best possible wildcard is *.b.example.com. If the NXDOMAIN
// NSEC is a.example.com. NSEC c.b.example.com., the longer suffix
// is the next domain of the NSEC, and we get the same wildcard name.
- const int qlabels = qname_.getLabelCount();
- const int olabels = qname_.compare(nsec->getName()).getCommonLabels();
- const int nlabels = qname_.compare(
+ const int qlabels = qname_->getLabelCount();
+ const int olabels = qname_->compare(nsec->getName()).getCommonLabels();
+ const int nlabels = qname_->compare(
dynamic_cast<const generic::NSEC&>(nsec->getRdataIterator()->
getCurrent()).
getNextName()).getCommonLabels();
const int common_labels = std::max(olabels, nlabels);
- const Name wildname(Name("*").concatenate(qname_.split(qlabels -
+ const Name wildname(Name("*").concatenate(qname_->split(qlabels -
common_labels)));
// Confirm the wildcard doesn't exist (this should result in NXDOMAIN;
@@ -206,12 +206,12 @@ Query::addNXDOMAINProofByNSEC3(ZoneFinder& finder) {
// Firstly get the NSEC3 proves for Closest Encloser Proof
// See Section 7.2.1 of RFC 5155.
const uint8_t closest_labels =
- addClosestEncloserProof(finder, qname_, false);
+ addClosestEncloserProof(finder, *qname_, false);
// Next, construct the wildcard name at the closest encloser, i.e.,
// '*' followed by the closest encloser, and add NSEC3 for it.
const Name wildname(Name("*").concatenate(
- qname_.split(qname_.getLabelCount() - closest_labels)));
+ qname_->split(qname_->getLabelCount() - closest_labels)));
addNSEC3ForName(finder, wildname, false);
}
@@ -226,7 +226,7 @@ Query::addWildcardProof(ZoneFinder& finder,
// substitution. Confirm that by specifying NO_WILDCARD. It should
// result in NXDOMAIN and an NSEC RR that proves it should be returned.
ConstZoneFinderContextPtr fcontext =
- finder.find(qname_, RRType::NSEC(),
+ finder.find(*qname_, RRType::NSEC(),
dnssec_opt_ | ZoneFinder::NO_WILDCARD);
if (fcontext->code != ZoneFinder::NXDOMAIN || !fcontext->rrset ||
fcontext->rrset->getRdataCount() == 0) {
@@ -241,7 +241,7 @@ Query::addWildcardProof(ZoneFinder& finder,
// of the matching wildcard, so NSEC3 for its next closer (and only
// that NSEC3) is what we are expected to provided per the RFC (if
// this assumption isn't met the zone is broken anyway).
- addClosestEncloserProof(finder, qname_, false, false);
+ addClosestEncloserProof(finder, *qname_, false, false);
}
}
@@ -254,7 +254,7 @@ Query::addWildcardNXRRSETProof(ZoneFinder& finder, ConstRRsetPtr nsec) {
}
ConstZoneFinderContextPtr fcontext =
- finder.find(qname_, RRType::NSEC(),
+ finder.find(*qname_, RRType::NSEC(),
dnssec_opt_ | ZoneFinder::NO_WILDCARD);
if (fcontext->code != ZoneFinder::NXDOMAIN || !fcontext->rrset ||
fcontext->rrset->getRdataCount() == 0) {
@@ -296,21 +296,21 @@ Query::addNXRRsetProof(ZoneFinder& finder,
addWildcardNXRRSETProof(finder, db_context.rrset);
}
} else if (db_context.isNSEC3Signed() && !db_context.isWildcard()) {
- if (qtype_ == RRType::DS()) {
+ if (*qtype_ == RRType::DS()) {
// RFC 5155, Section 7.2.4. Add either NSEC3 for the qname or
// closest (provable) encloser proof in case of optout.
- addClosestEncloserProof(finder, qname_, true);
+ addClosestEncloserProof(finder, *qname_, true);
} else {
// RFC 5155, Section 7.2.3. Just add NSEC3 for the qname.
- addNSEC3ForName(finder, qname_, true);
+ addNSEC3ForName(finder, *qname_, true);
}
} else if (db_context.isNSEC3Signed() && db_context.isWildcard()) {
// Case for RFC 5155 Section 7.2.5: add closest encloser proof for the
// qname, construct the matched wildcard name and add NSEC3 for it.
const uint8_t closest_labels =
- addClosestEncloserProof(finder, qname_, false);
+ addClosestEncloserProof(finder, *qname_, false);
const Name wname = Name("*").concatenate(
- qname_.split(qname_.getLabelCount() - closest_labels));
+ qname_->split(qname_->getLabelCount() - closest_labels));
addNSEC3ForName(finder, wname, true);
}
}
@@ -331,7 +331,7 @@ Query::addAuthAdditional(ZoneFinder& finder,
finder.getOrigin().toText());
}
authorities_.push_back(ns_context->rrset);
- getAdditional(qname_, qtype_, *ns_context, additionals);
+ getAdditional(*qname_, *qtype_, *ns_context, additionals);
}
namespace {
@@ -352,7 +352,7 @@ findZone(const DataSourceClient& client, const Name& qname, RRType qtype) {
void
Query::process(datasrc::DataSourceClient& datasrc_client,
- const isc::dns::Name qname, const isc::dns::RRType qtype,
+ const isc::dns::Name& qname, const isc::dns::RRType& qtype,
isc::dns::Message& response, bool dnssec)
{
// First a bit of house cleaning
@@ -366,7 +366,7 @@ Query::process(datasrc::DataSourceClient& datasrc_client,
// Found a zone which is the nearest ancestor to QNAME
const DataSourceClient::FindResult result = findZone(*datasrc_client_,
- qname_, qtype_);
+ *qname_, *qtype_);
// If we have no matching authoritative zone for the query name, return
// REFUSED. In short, this is to be compatible with BIND 9, but the
@@ -378,14 +378,12 @@ Query::process(datasrc::DataSourceClient& datasrc_client,
// If we tried to find a "parent zone" for a DS query and failed,
// we may still have authority at the child side. If we do, the query
// has to be handled there.
- if (qtype_ == RRType::DS() && qname_.getLabelCount() > 1 &&
+ if (*qtype_ == RRType::DS() && qname_->getLabelCount() > 1 &&
processDSAtChild()) {
- createResponse();
return;
}
response_->setHeaderFlag(Message::HEADERFLAG_AA, false);
response_->setRcode(Rcode::REFUSED());
- createResponse();
return;
}
ZoneFinder& zfinder = *result.zone_finder;
@@ -395,12 +393,12 @@ Query::process(datasrc::DataSourceClient& datasrc_client,
response_->setHeaderFlag(Message::HEADERFLAG_AA);
response_->setRcode(Rcode::NOERROR());
boost::function0<ZoneFinderContextPtr> find;
- const bool qtype_is_any = (qtype_ == RRType::ANY());
+ const bool qtype_is_any = (*qtype_ == RRType::ANY());
if (qtype_is_any) {
- find = boost::bind(&ZoneFinder::findAll, &zfinder, qname_,
+ find = boost::bind(&ZoneFinder::findAll, &zfinder, *qname_,
boost::ref(answers_), dnssec_opt_);
} else {
- find = boost::bind(&ZoneFinder::find, &zfinder, qname_, qtype_,
+ find = boost::bind(&ZoneFinder::find, &zfinder, *qname_, *qtype_,
dnssec_opt_);
}
ZoneFinderContextPtr db_context(find());
@@ -420,7 +418,7 @@ Query::process(datasrc::DataSourceClient& datasrc_client,
dynamic_cast<const rdata::generic::DNAME&>(
db_context->rrset->getRdataIterator()->getCurrent()));
// The yet unmatched prefix dname
- const Name prefix(qname_.split(0, qname_.getLabelCount() -
+ const Name prefix(qname_->split(0, qname_->getLabelCount() -
db_context->rrset->getName().getLabelCount()));
// If we put it together, will it be too long?
// (The prefix contains trailing ., which will be removed
@@ -436,11 +434,11 @@ Query::process(datasrc::DataSourceClient& datasrc_client,
// The new CNAME we are creating (it will be unsigned even
// with DNSSEC, the DNAME is signed and it can be validated
// by that)
- RRsetPtr cname(new RRset(qname_, db_context->rrset->getClass(),
+ RRsetPtr cname(new RRset(*qname_, db_context->rrset->getClass(),
RRType::CNAME(), db_context->rrset->getTTL()));
// Construct the new target by replacing the end
- cname->addRdata(rdata::generic::CNAME(qname_.split(0,
- qname_.getLabelCount() -
+ cname->addRdata(rdata::generic::CNAME(qname_->split(0,
+ qname_->getLabelCount() -
db_context->rrset->getName().getLabelCount()).
concatenate(dname.getDname())));
answers_.push_back(cname);
@@ -471,7 +469,7 @@ Query::process(datasrc::DataSourceClient& datasrc_client,
}
// Retrieve additional records for the answer
- getAdditional(qname_, qtype_, *db_context, additionals_);
+ getAdditional(*qname_, *qtype_, *db_context, additionals_);
// If apex NS records haven't been provided in the answer
// section, insert apex NS records into the authority section
@@ -481,7 +479,7 @@ Query::process(datasrc::DataSourceClient& datasrc_client,
// qname is the zone origin.
if (result.code != result::SUCCESS ||
db_context->code != ZoneFinder::SUCCESS ||
- (qtype_ != RRType::NS() && !qtype_is_any))
+ (*qtype_ != RRType::NS() && !qtype_is_any))
{
addAuthAdditional(*result.zone_finder, additionals_);
}
@@ -497,8 +495,8 @@ Query::process(datasrc::DataSourceClient& datasrc_client,
// if we are an authority of the child, too. If so, we need to
// complete the process in the child as specified in Section
// 2.2.1.2. of RFC3658.
- if (qtype_ == RRType::DS() && processDSAtChild()) {
- break;
+ if (*qtype_ == RRType::DS() && processDSAtChild()) {
+ return;
}
response_->setHeaderFlag(Message::HEADERFLAG_AA, false);
@@ -542,12 +540,12 @@ Query::process(datasrc::DataSourceClient& datasrc_client,
void
Query::initialize(datasrc::DataSourceClient& datasrc_client,
- const isc::dns::Name qname, const isc::dns::RRType qtype,
+ const isc::dns::Name& qname, const isc::dns::RRType& qtype,
isc::dns::Message& response, bool dnssec)
{
datasrc_client_ = &datasrc_client;
- qname_ = qname;
- qtype_ = qtype;
+ qname_ = &qname;
+ qtype_ = &qtype;
response_ = &response;
dnssec_ = dnssec;
dnssec_opt_ = (dnssec ? isc::datasrc::ZoneFinder::FIND_DNSSEC :
@@ -570,7 +568,7 @@ Query::createResponse() {
bool
Query::processDSAtChild() {
const DataSourceClient::FindResult zresult =
- datasrc_client_->findZone(qname_);
+ datasrc_client_->findZone(*qname_);
if (zresult.code != result::SUCCESS) {
return (false);
@@ -589,13 +587,14 @@ Query::processDSAtChild() {
response_->setRcode(Rcode::NOERROR());
addSOA(*zresult.zone_finder);
ConstZoneFinderContextPtr ds_context =
- zresult.zone_finder->find(qname_, RRType::DS(), dnssec_opt_);
+ zresult.zone_finder->find(*qname_, RRType::DS(), dnssec_opt_);
if (ds_context->code == ZoneFinder::NXRRSET) {
if (dnssec_) {
addNXRRsetProof(*zresult.zone_finder, *ds_context);
}
}
+ createResponse();
return (true);
}
diff --git a/src/bin/auth/query.h b/src/bin/auth/query.h
index 1adfd8b..062925c 100644
--- a/src/bin/auth/query.h
+++ b/src/bin/auth/query.h
@@ -18,6 +18,8 @@
#include <dns/rrset.h>
#include <datasrc/zone.h>
+#include <boost/noncopyable.hpp>
+
#include <vector>
namespace isc {
@@ -34,6 +36,13 @@ class DataSourceClient;
namespace auth {
+/// \brief Initial reserved size for the vectors in Query
+///
+/// The value is larger than we expect the vectors to even become, and
+/// has been chosen arbitrarily. The reason to set them quite high is
+/// to prevent reallocation on addition.
+const size_t RESERVE_RRSETS = 64;
+
/// The \c Query class represents a standard DNS query that encapsulates
/// processing logic to answer the query.
///
@@ -64,7 +73,7 @@ namespace auth {
/// likely to misuse one of the classes instead of the other
/// accidentally, and since it's considered a temporary development state,
/// we keep this name at the moment.
-class Query {
+class Query : boost::noncopyable {
private:
/// \brief Adds a SOA.
@@ -239,7 +248,7 @@ private:
/// \param dnssec If the answer should include signatures and NSEC/NSEC3 if
/// possible.
void initialize(datasrc::DataSourceClient& datasrc_client,
- const isc::dns::Name qname, const isc::dns::RRType qtype,
+ const isc::dns::Name& qname, const isc::dns::RRType& qtype,
isc::dns::Message& response, bool dnssec = false);
/// \brief Fill in the response sections
@@ -250,7 +259,7 @@ private:
///
/// This will take each RRset collected in answers_, authorities_, and
/// additionals_, and add them to their corresponding sections in
- /// the response packet.
+ /// the response message.
///
/// After they are added, the vectors are cleared.
void createResponse();
@@ -264,18 +273,19 @@ private:
}
public:
- /// Empty Constructor.
+ /// Default constructor.
///
/// Query parameters will be set by the call to process()
///
- /// This constructor never throws an exception.
- ///
Query() :
- datasrc_client_(NULL), qname_("."),
- qtype_(isc::dns::RRType::A()),
+ datasrc_client_(NULL), qname_(NULL), qtype_(NULL),
response_(NULL), dnssec_(false),
dnssec_opt_(isc::datasrc::ZoneFinder::FIND_DEFAULT)
- {}
+ {
+ answers_.reserve(RESERVE_RRSETS);
+ authorities_.reserve(RESERVE_RRSETS);
+ additionals_.reserve(RESERVE_RRSETS);
+ }
/// Process the query.
///
@@ -312,7 +322,7 @@ public:
/// \param dnssec If the answer should include signatures and NSEC/NSEC3 if
/// possible.
void process(datasrc::DataSourceClient& datasrc_client,
- const isc::dns::Name qname, const isc::dns::RRType qtype,
+ const isc::dns::Name& qname, const isc::dns::RRType& qtype,
isc::dns::Message& response, bool dnssec = false);
/// \short Bad zone data encountered.
@@ -383,8 +393,8 @@ public:
private:
const isc::datasrc::DataSourceClient* datasrc_client_;
- isc::dns::Name qname_;
- isc::dns::RRType qtype_;
+ const isc::dns::Name* qname_;
+ const isc::dns::RRType* qtype_;
isc::dns::Message* response_;
bool dnssec_;
isc::datasrc::ZoneFinder::FindOptions dnssec_opt_;
More information about the bind10-changes
mailing list