used pimpl + in-class static data approach instead of relying on non local

static objects to minimize the risk of statitic initialization order fiasco.


git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@1143 e5f2f494-b856-4b98-b285-d166d9295462

src/lib/auth/data_source_static.cc

@@ -32,62 +32,72 @@ using namespace isc::dns::rdata;
 namespace isc {
 namespace auth {
 
-// There's no mutable internal state in StaticDataSrcImpl, so define it
-// as a struct.
-
-namespace {
-
-// 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;
-
-class StaticDataInitializer {
+// 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:
-    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"));
-
-        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));
-    }
+    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;
 };
-const StaticDataInitializer initialier_object;
+
+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"));
+
+    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));
 }
 
 StaticDataSrc::StaticDataSrc()
 {
     setClass(RRClass::CH());
+    impl_ = new StaticDataSrcImpl;
+}
+
+StaticDataSrc::~StaticDataSrc()
+{
+    delete impl_;
 }
 
 void
@@ -95,17 +105,17 @@ StaticDataSrc::findClosestEnclosure(NameMatch& match) const {
     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 @@ StaticDataSrc::findRRset(const Query& q, const Name& qname,
         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;

src/lib/auth/data_source_static.h

@@ -41,6 +41,7 @@ namespace auth {
 
 class Query;
 class NameMatch;
+struct StaticDataSrcImpl;
 
 class StaticDataSrc : public DataSrc {
 private:
@@ -54,7 +55,7 @@ private:
     StaticDataSrc& operator=(const StaticDataSrc& source);
 public:
     StaticDataSrc();
-    ~StaticDataSrc() {}
+    ~StaticDataSrc();
     //@}
 
     void findClosestEnclosure(NameMatch& match) const;
@@ -88,6 +89,8 @@ public:
 
     Result init();
     Result close();
+private:
+    StaticDataSrcImpl* impl_;
 };
 
 }