[svn] commit: r1143 - in /trunk/src/lib/auth: data_source_static.cc data_source_static.h

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Mar 5 23:53:06 UTC 2010


Author: jinmei
Date: Fri Mar  5 23:53:06 2010
New Revision: 1143

Log:
used pimpl + in-class static data approach instead of relying on non local
static objects to minimize the risk of statitic initialization order fiasco.

Modified:
    trunk/src/lib/auth/data_source_static.cc
    trunk/src/lib/auth/data_source_static.h

Modified: trunk/src/lib/auth/data_source_static.cc
==============================================================================
--- trunk/src/lib/auth/data_source_static.cc (original)
+++ trunk/src/lib/auth/data_source_static.cc Fri Mar  5 23:53:06 2010
@@ -32,62 +32,72 @@
 namespace isc {
 namespace auth {
 
-// There's no mutable internal state in StaticDataSrcImpl, so define it
-// as a struct.
+// This class stores the "static" data for the built-in static data source.
+// Since it's static, it could be literally static, i.e, defined as static
+// objects.  But to avoid the static initialization order fiasco, which would
+// be unlikely to happen for this class in practice but is still possible,
+// we took a safer approach.  A downside of this approach is that redundant
+// copies of exactly the same content of these objects can be created.
+// In practice, however, there's normally at most one StaticDataSrc object,
+// so this should be acceptable (if this turns out to be a real concern we
+// might consider making this class a singleton).
+// We use the "pimpl" idiom for this class.  Operations for this class is
+// not expected to be performance sensitive, so the overhead due to the pimpl
+// should be okay, and it's more beneficial to hide details and minimize
+// inter module dependencies in header files.
+struct StaticDataSrcImpl {
+public:
+    StaticDataSrcImpl();
+    const Name authors_name;
+    const Name version_name;
+    // XXX: unfortunately these can't be ConstRRsetPtr because they'll be
+    // passed to RRsetList::addRRset(), which expect non const RRsetPtr.
+    // We should revisit this design later.
+    RRsetPtr authors;
+    RRsetPtr authors_ns;
+    RRsetPtr version;
+    RRsetPtr version_ns;
+};
 
-namespace {
+StaticDataSrcImpl::StaticDataSrcImpl() :
+    authors_name("authors.bind"), version_name("version.bind")
+{
+    authors = RRsetPtr(new RRset(authors_name, RRClass::CH(),
+                                 RRType::TXT(), RRTTL(0)));
+    authors->addRdata(generic::TXT("Evan Hunt"));
+    authors->addRdata(generic::TXT("Han Feng"));
+    authors->addRdata(generic::TXT("Jelte Jansen"));
+    authors->addRdata(generic::TXT("Jeremy C. Reed")); 
+    authors->addRdata(generic::TXT("Jin Jian"));
+    authors->addRdata(generic::TXT("JINMEI Tatuya"));
+    authors->addRdata(generic::TXT("Kazunori Fujiwara"));
+    authors->addRdata(generic::TXT("Michael Graff"));
+    authors->addRdata(generic::TXT("Naoki Kambe"));
+    authors->addRdata(generic::TXT("Shane Kerr"));
+    authors->addRdata(generic::TXT("Zhang Likun"));
 
-// All data in this class is literally static and can be generated at
-// initialization time (and won't be changed).  Names can be statically
-// initialized.  For RRsets, a helper class (StaticDataInitializer) and its
-// only instance will be used for initialization.
-const Name authors_name("authors.bind");
-const Name version_name("version.bind");
-// XXX: unfortunately these can't be RRsetPtr because they'll be passed to
-// RRsetList::addRRset(), which expect non const RRsetPtr.  We should revisit
-// this design later.
-RRsetPtr authors;
-RRsetPtr authors_ns;
-RRsetPtr version;
-RRsetPtr version_ns;
+    authors_ns = RRsetPtr(new RRset(authors_name, RRClass::CH(),
+                                    RRType::NS(), RRTTL(0)));
+    authors_ns->addRdata(generic::NS(authors_name));
 
-class StaticDataInitializer {
-public:
-    StaticDataInitializer()
-    {
-        authors = RRsetPtr(new RRset(authors_name, RRClass::CH(),
-                                     RRType::TXT(), RRTTL(0)));
-        authors->addRdata(generic::TXT("Evan Hunt"));
-        authors->addRdata(generic::TXT("Han Feng"));
-        authors->addRdata(generic::TXT("Jelte Jansen"));
-        authors->addRdata(generic::TXT("Jeremy C. Reed")); 
-        authors->addRdata(generic::TXT("Jin Jian"));
-        authors->addRdata(generic::TXT("JINMEI Tatuya"));
-        authors->addRdata(generic::TXT("Kazunori Fujiwara"));
-        authors->addRdata(generic::TXT("Michael Graff"));
-        authors->addRdata(generic::TXT("Naoki Kambe"));
-        authors->addRdata(generic::TXT("Shane Kerr"));
-        authors->addRdata(generic::TXT("Zhang Likun"));
+    version = RRsetPtr(new RRset(version_name, RRClass::CH(),
+                                 RRType::TXT(), RRTTL(0)));
+    version->addRdata(generic::TXT("BIND10 0.0.0 (pre-alpha)"));
 
-        authors_ns = RRsetPtr(new RRset(authors_name, RRClass::CH(),
-                                        RRType::NS(), RRTTL(0)));
-        authors_ns->addRdata(generic::NS(authors_name));
-
-        version = RRsetPtr(new RRset(version_name, RRClass::CH(),
-                                     RRType::TXT(), RRTTL(0)));
-        version->addRdata(generic::TXT("BIND10 0.0.0 (pre-alpha)"));
-
-        version_ns = RRsetPtr(new RRset(version_name, RRClass::CH(),
-                                        RRType::NS(), RRTTL(0)));
-        version_ns->addRdata(generic::NS(version_name));
-    }
-};
-const StaticDataInitializer initialier_object;
+    version_ns = RRsetPtr(new RRset(version_name, RRClass::CH(),
+                                    RRType::NS(), RRTTL(0)));
+    version_ns->addRdata(generic::NS(version_name));
 }
 
 StaticDataSrc::StaticDataSrc()
 {
     setClass(RRClass::CH());
+    impl_ = new StaticDataSrcImpl;
+}
+
+StaticDataSrc::~StaticDataSrc()
+{
+    delete impl_;
 }
 
 void
@@ -95,17 +105,17 @@
     const Name& qname = match.qname();
     NameComparisonResult::NameRelation cmp;
 
-    cmp = qname.compare(version_name).getRelation();
+    cmp = qname.compare(impl_->version_name).getRelation();
     if (cmp == NameComparisonResult::EQUAL ||
         cmp == NameComparisonResult::SUBDOMAIN) {
-        match.update(*this, version_name);
+        match.update(*this, impl_->version_name);
         return;
     }
 
-    cmp = qname.compare(authors_name).getRelation();
+    cmp = qname.compare(impl_->authors_name).getRelation();
     if (cmp == NameComparisonResult::EQUAL ||
         cmp == NameComparisonResult::SUBDOMAIN) {
-        match.update(*this, authors_name);
+        match.update(*this, impl_->authors_name);
         return;
     }
 }
@@ -121,22 +131,22 @@
         return (ERROR);
     }
 
-    bool any = (qtype == RRType::ANY());
+    const bool any = (qtype == RRType::ANY());
 
-    if (qname == version_name) {
+    if (qname == impl_->version_name) {
         if (qtype == RRType::TXT() || any) {
-            target.addRRset(version);
+            target.addRRset(impl_->version);
         } else if (qtype == RRType::NS()) {
-            target.addRRset(version_ns);
+            target.addRRset(impl_->version_ns);
         } else {
             flags = TYPE_NOT_FOUND;
         }
-    } else if (qname == authors_name) {
+    } else if (qname == impl_->authors_name) {
         if (qtype == RRType::TXT() || any) {
-            target.addRRset(authors);
+            target.addRRset(impl_->authors);
             return (SUCCESS);
         } else if (qtype == RRType::NS()) {
-            target.addRRset(authors_ns);
+            target.addRRset(impl_->authors_ns);
             return (SUCCESS);
         } else {
             flags = TYPE_NOT_FOUND;

Modified: trunk/src/lib/auth/data_source_static.h
==============================================================================
--- trunk/src/lib/auth/data_source_static.h (original)
+++ trunk/src/lib/auth/data_source_static.h Fri Mar  5 23:53:06 2010
@@ -41,6 +41,7 @@
 
 class Query;
 class NameMatch;
+struct StaticDataSrcImpl;
 
 class StaticDataSrc : public DataSrc {
 private:
@@ -54,7 +55,7 @@
     StaticDataSrc& operator=(const StaticDataSrc& source);
 public:
     StaticDataSrc();
-    ~StaticDataSrc() {}
+    ~StaticDataSrc();
     //@}
 
     void findClosestEnclosure(NameMatch& match) const;
@@ -88,6 +89,8 @@
 
     Result init();
     Result close();
+private:
+    StaticDataSrcImpl* impl_;
 };
 
 }




More information about the bind10-changes mailing list