[svn] commit: r473 - in /branches/jinmei-dnsrrparams: ./ src/lib/dns/cpp/rrparamregistry.cc src/lib/dns/cpp/rrparamregistry.h src/lib/dns/cpp/rrparamregistry_unittest.cc src/lib/dns/cpp/rrtype.h
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Jan 18 23:54:09 UTC 2010
Author: jinmei
Date: Mon Jan 18 23:54:09 2010
New Revision: 473
Log:
- applied review feedback
- added comments about thread-safe considerations (based on review comments)
- some more code cleanups
Modified:
branches/jinmei-dnsrrparams/ (props changed)
branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry.cc
branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry.h
branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry_unittest.cc
branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrtype.h
Modified: branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry.cc
==============================================================================
--- branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry.cc (original)
+++ branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry.cc Mon Jan 18 23:54:09 2010
@@ -169,29 +169,20 @@
// Rollback logic on failure is complicated. If adding the new type or
// class fails, we should revert to the original state, cleaning up
// intermediate state. But we need to make sure that we don't remove
- // existing data. addType()/AddClass() will simply ignore an attempt to
+ // existing data. addType()/addClass() will simply ignore an attempt to
// add the same data, so the cleanup should be performed only when we add
// something new but we fail in other part of the process.
- bool will_add_type = false;
bool type_added = false;
- bool will_add_class = false;
bool class_added = false;
-
- if (impl_->code2typemap.find(typecode) == impl_->code2typemap.end()) {
- will_add_type = true;
- }
- if (impl_->code2classmap.find(classcode) == impl_->code2classmap.end()) {
- will_add_class = true;
- }
try {
type_added = addType(typecode_string, typecode);
class_added = addClass(classcode_string, classcode);
} catch (...) {
- if (will_add_type && type_added) {
+ if (type_added) {
removeType(typecode);
}
- if (will_add_class && class_added) {
+ if (class_added) {
removeClass(classcode);
}
throw;
@@ -222,7 +213,7 @@
/// Code logic for RRTypes and RRClasses is mostly common except (C++) type and
/// member names. So we define type-independent templates to describe the
-/// common logic and let concrete classes to avoid code duplicates.
+/// common logic and let concrete classes use it to avoid code duplicates.
/// The following summarize template parameters used in the set of template
/// functions:
/// PT: parameter type, either RRTypeParam or RRClassParam
@@ -326,7 +317,7 @@
bool
RRParamRegistry::addType(const string& type_string, uint16_t code)
{
- return (addParam<RRTypeParam, CodeRRTypeMap, StrRRTypeMap, RRTypeExist>
+ return (addParam<RRTypeParam, CodeRRTypeMap, StrRRTypeMap, RRTypeExists>
(type_string, code, impl_->code2typemap, impl_->str2typemap));
}
@@ -353,7 +344,7 @@
bool
RRParamRegistry::addClass(const string& class_string, uint16_t code)
{
- return (addParam<RRClassParam, CodeRRClassMap, StrRRClassMap, RRClassExist>
+ return (addParam<RRClassParam, CodeRRClassMap, StrRRClassMap, RRClassExists>
(class_string, code, impl_->code2classmap, impl_->str2classmap));
}
@@ -375,7 +366,8 @@
string
RRParamRegistry::codeToClassText(uint16_t code) const
{
- return (codeToText<RRClassParam, CodeRRClassMap>(code, impl_->code2classmap));
-}
-}
-}
+ return (codeToText<RRClassParam, CodeRRClassMap>(code,
+ impl_->code2classmap));
+}
+}
+}
Modified: branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry.h
==============================================================================
--- branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry.h (original)
+++ branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry.h Mon Jan 18 23:54:09 2010
@@ -30,22 +30,22 @@
struct RRParamRegistryImpl;
///
+/// \brief A standard DNS module exception that is thrown if a new RR type is
+/// being registered with a different type string.
+///
+class RRTypeExists : public Exception {
+public:
+ RRTypeExists(const char* file, size_t line, const char* what) :
+ isc::dns::Exception(file, line, what) {}
+};
+
+///
/// \brief A standard DNS module exception that is thrown if a new RR class is
/// being registered with a different type string.
///
-class RRClassExist : public Exception {
+class RRClassExists : public Exception {
public:
- RRClassExist(const char* file, size_t line, const char* what) :
- isc::dns::Exception(file, line, what) {}
-};
-
-///
-/// \brief A standard DNS module exception that is thrown if a new RR type is
-/// being registered with a different type string.
-///
-class RRTypeExist : public Exception {
-public:
- RRTypeExist(const char* file, size_t line, const char* what) :
+ RRClassExists(const char* file, size_t line, const char* what) :
isc::dns::Exception(file, line, what) {}
};
@@ -73,6 +73,17 @@
/// constructor is intentionally private, and applications must get access to
/// the single instance via the \c getRegistry() static member function.
///
+/// In the current implementation, access to the singleton \c RRParamRegistry
+/// object is not thread safe.
+/// The application should ensure that multiple threads don't race in the
+/// first invocation of \c getRegistry(), and, if the registry needs to
+/// be changed dynamically, read and write operations are performed
+/// exclusively.
+/// Since this class should be static in common usage this restriction would
+/// be acceptable in practice.
+/// In the future, we may extend the implementation so that multiple threads can
+/// get access to the registry fully concurrently without any restriction.
+///
/// Note: the implementation of this class is incomplete: we should at least
/// add RDATA related parameters. This will be done in a near future version,
/// at which point some of method signatures will be changed.
@@ -88,6 +99,24 @@
~RRParamRegistry();
//@}
public:
+ ///
+ /// \brief Return the singleton instance of \c RRParamRegistry.
+ ///
+ /// This method is a unified access point to the singleton instance of
+ /// the RR parameter registry (\c RRParamRegistry).
+ /// On first invocation it internally constructs an instance of the
+ /// \c RRParamRegistry class and returns a reference to it.
+ /// This is a static object inside this method and will remain valid
+ /// throughout the rest of the application lifetime.
+ /// On subsequent calls this method simply returns a reference to the
+ /// singleton object.
+ ///
+ /// If resource allocation fails in the first invocation,
+ /// a corresponding standard exception will be thrown.
+ /// This method never fails otherwise. In particular, this method
+ /// doesn't throw an exception once the singleton instance is constructed.
+ ///
+ /// \return A reference to the singleton instance of \c RRParamRegistry.
static RRParamRegistry& getRegistry();
///
Modified: branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry_unittest.cc
==============================================================================
--- branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry_unittest.cc (original)
+++ branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrparamregistry_unittest.cc Mon Jan 18 23:54:09 2010
@@ -82,14 +82,14 @@
EXPECT_THROW(RRParamRegistry::getRegistry().add(test_type_str,
test_type_code,
test_class_str, 1),
- RRClassExist);
+ RRClassExists);
EXPECT_EQ("IN", RRClass(1).toText());
// Same for RRType
EXPECT_THROW(RRParamRegistry::getRegistry().add(test_type_str, 1,
test_class_str,
test_class_code),
- RRTypeExist);
+ RRTypeExists);
EXPECT_EQ("A", RRType(1).toText());
}
}
Modified: branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrtype.h
==============================================================================
--- branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrtype.h (original)
+++ branches/jinmei-dnsrrparams/src/lib/dns/cpp/rrtype.h Mon Jan 18 23:54:09 2010
@@ -80,6 +80,11 @@
/// before the initialization for \c default_type, we need help from
/// the proxy function.
///
+/// In the current implementation, the initialization of the well-known
+/// static objects is not thread safe. The same consideration as the
+/// \c RRParamRegistry class applies. We may extend the implementation so
+/// that the initialization is ensured to be thread safe in a future version.
+///
/// Note to developers: since it's expected that some of these constant
/// \c RRType objects are frequently used in a performance sensitive path,
/// we define these proxy functions as inline. This makes sense only when
More information about the bind10-changes
mailing list