BIND 10 trac2850_2, updated. 68343c845a46a9be3f38c38388ec02a8bb4ef884 [2850] Make getNamedAddress() return a std::pair
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu May 2 07:22:54 UTC 2013
The branch, trac2850_2 has been updated
via 68343c845a46a9be3f38c38388ec02a8bb4ef884 (commit)
from 10f2ae4625a238a4899372432df13d29eca9f9b3 (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 68343c845a46a9be3f38c38388ec02a8bb4ef884
Author: Mukund Sivaraman <muks at isc.org>
Date: Thu May 2 12:15:32 2013 +0530
[2850] Make getNamedAddress() return a std::pair
-----------------------------------------------------------------------
Summary of changes:
.../datasrc/memory/zone_table_segment_mapped.cc | 42 ++++++++++-----
src/lib/util/memory_segment.h | 16 ++++--
src/lib/util/memory_segment_local.cc | 6 +--
src/lib/util/memory_segment_local.h | 2 +-
src/lib/util/memory_segment_mapped.cc | 6 +--
src/lib/util/memory_segment_mapped.h | 2 +-
.../util/tests/memory_segment_common_unittest.cc | 16 +++---
.../util/tests/memory_segment_mapped_unittest.cc | 54 ++++++++++++--------
8 files changed, 88 insertions(+), 56 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/zone_table_segment_mapped.cc b/src/lib/datasrc/memory/zone_table_segment_mapped.cc
index 6bab720..6c50700 100644
--- a/src/lib/datasrc/memory/zone_table_segment_mapped.cc
+++ b/src/lib/datasrc/memory/zone_table_segment_mapped.cc
@@ -43,8 +43,11 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
// If there is a previously opened segment, and it was
// opened in read-write mode, update its checksum.
mem_sgmt_->shrinkToFit();
- uint32_t* checksum = static_cast<uint32_t*>
- (mem_sgmt_->getNamedAddress("zone_table_checksum"));
+ const MemorySegment::NamedAddressResult result =
+ mem_sgmt_->getNamedAddress("zone_table_checksum");
+ assert(result.first);
+ assert(result.second);
+ uint32_t* checksum = static_cast<uint32_t*>(result.second);
// First, clear the checksum so that getCheckSum() returns
// a consistent value.
*checksum = 0;
@@ -86,7 +89,9 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
(filename,
MemorySegmentMapped::CREATE_ONLY));
// There must be no previously saved checksum.
- if (segment->getNamedAddress("zone_table_checksum")) {
+ MemorySegment::NamedAddressResult result =
+ segment->getNamedAddress("zone_table_checksum");
+ if (result.first) {
isc_throw(isc::Unexpected,
"There is already a saved checksum in a mapped segment "
"opened in create mode.");
@@ -97,7 +102,8 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
segment->setNamedAddress("zone_table_checksum", checksum);
// There must be no previously saved ZoneTableHeader.
- if (segment->getNamedAddress("zone_table_header")) {
+ result = segment->getNamedAddress("zone_table_header");
+ if (result.first) {
isc_throw(isc::Unexpected,
"There is already a saved ZoneTableHeader in a "
"mapped segment opened in create mode.");
@@ -116,11 +122,13 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
// If there is a previously saved checksum, verify that it is
// consistent. Otherwise, allocate space for a checksum (which
// is saved during close).
- if (segment->getNamedAddress("zone_table_checksum")) {
+ MemorySegment::NamedAddressResult result =
+ segment->getNamedAddress("zone_table_checksum");
+ if (result.first) {
// The segment was already shrunk when it was last
// closed. Check that its checksum is consistent.
- uint32_t* checksum = static_cast<uint32_t*>
- (segment->getNamedAddress("zone_table_checksum"));
+ assert(result.second);
+ uint32_t* checksum = static_cast<uint32_t*>(result.second);
const uint32_t saved_checksum = *checksum;
// First, clear the checksum so that getCheckSum() returns
// a consistent value.
@@ -138,9 +146,11 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
// If there is a previously saved ZoneTableHeader, use
// it. Otherwise, allocate a new header.
- header_ = static_cast<ZoneTableHeader*>
- (segment->getNamedAddress("zone_table_header"));
- if (!header_) {
+ result = segment->getNamedAddress("zone_table_header");
+ if (result.first) {
+ assert(result.second);
+ header_ = static_cast<ZoneTableHeader*>(result.second);
+ } else {
void* ptr = segment->allocate(sizeof(ZoneTableHeader));
ZoneTableHeader* new_header = new(ptr)
ZoneTableHeader(ZoneTable::create(*segment, rrclass_));
@@ -153,7 +163,9 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
case READ_ONLY: {
segment.reset(new MemorySegmentMapped(filename));
// There must be a previously saved checksum.
- if (!segment->getNamedAddress("zone_table_checksum")) {
+ MemorySegment::NamedAddressResult result =
+ segment->getNamedAddress("zone_table_checksum");
+ if (!result.first) {
isc_throw(isc::Unexpected,
"There is no previously saved checksum in a "
"mapped segment opened in read-only mode.");
@@ -164,9 +176,11 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
// to 0 for checksum calculation in a read-only segment.
// There must be a previously saved ZoneTableHeader.
- header_ = static_cast<ZoneTableHeader*>
- (segment->getNamedAddress("zone_table_header"));
- if (!header_) {
+ result = segment->getNamedAddress("zone_table_header");
+ if (result.first) {
+ assert(result.second);
+ header_ = static_cast<ZoneTableHeader*>(result.second);
+ } else {
isc_throw(isc::Unexpected,
"There is no previously saved ZoneTableHeader in a "
"mapped segment opened in read-only mode.");
diff --git a/src/lib/util/memory_segment.h b/src/lib/util/memory_segment.h
index e62c9df..44b9560 100644
--- a/src/lib/util/memory_segment.h
+++ b/src/lib/util/memory_segment.h
@@ -17,6 +17,8 @@
#include <exceptions/exceptions.h>
+#include <utility>
+
#include <stdlib.h>
namespace isc {
@@ -176,7 +178,8 @@ public:
/// as \c addr even if it wouldn't be considered to "belong to" the
/// segment in its normal sense; it can be used to indicate that memory
/// has not been allocated for the specified name. A subsequent call
- /// to \c getNamedAddress() will return NULL for that name.
+ /// to \c getNamedAddress() will return std::pair (true, NULL) for
+ /// that name.
///
/// \note Naming an address is intentionally separated from allocation
/// so that, for example, one module of a program can name a memory
@@ -228,6 +231,9 @@ public:
return (setNamedAddressImpl(name, addr));
}
+ /// \brief Type definition for result returned by getNamedAddress()
+ typedef std::pair<bool, void*> NamedAddressResult;
+
/// \brief Return the address in the segment that has the given name.
///
/// This method returns the memory address in the segment corresponding
@@ -245,8 +251,10 @@ public:
///
/// \param name A C string of which the segment memory address is to be
/// returned. Must not be NULL.
- /// \return The address associated with the name, or NULL if not found.
- void* getNamedAddress(const char* name) {
+ /// \return An std::pair containing a bool (set to true if the name
+ /// was found, or false otherwise) and the address associated with
+ /// the name (which is undefined if the name was not found).
+ NamedAddressResult getNamedAddress(const char* name) {
// This public method implements common validation. The actual
// work specific to the derived segment is delegated to the
// corresponding protected method.
@@ -288,7 +296,7 @@ protected:
virtual bool setNamedAddressImpl(const char* name, void* addr) = 0;
/// \brief Implementation of getNamedAddress beyond common validation.
- virtual void* getNamedAddressImpl(const char* name) = 0;
+ virtual NamedAddressResult getNamedAddressImpl(const char* name) = 0;
/// \brief Implementation of clearNamedAddress beyond common validation.
virtual bool clearNamedAddressImpl(const char* name) = 0;
diff --git a/src/lib/util/memory_segment_local.cc b/src/lib/util/memory_segment_local.cc
index 81548fd..e68db27 100644
--- a/src/lib/util/memory_segment_local.cc
+++ b/src/lib/util/memory_segment_local.cc
@@ -51,13 +51,13 @@ MemorySegmentLocal::allMemoryDeallocated() const {
return (allocated_size_ == 0 && named_addrs_.empty());
}
-void*
+MemorySegment::NamedAddressResult
MemorySegmentLocal::getNamedAddressImpl(const char* name) {
std::map<std::string, void*>::iterator found = named_addrs_.find(name);
if (found != named_addrs_.end()) {
- return (found->second);
+ return (NamedAddressResult(true, found->second));
}
- return (0);
+ return (NamedAddressResult(false, NULL));
}
bool
diff --git a/src/lib/util/memory_segment_local.h b/src/lib/util/memory_segment_local.h
index 1db55a0..90d4907 100644
--- a/src/lib/util/memory_segment_local.h
+++ b/src/lib/util/memory_segment_local.h
@@ -70,7 +70,7 @@ public:
///
/// There's a small chance this method could throw std::bad_alloc.
/// It should be considered a fatal error.
- virtual void* getNamedAddressImpl(const char* name);
+ virtual NamedAddressResult getNamedAddressImpl(const char* name);
/// \brief Local segment version of setNamedAddress.
///
diff --git a/src/lib/util/memory_segment_mapped.cc b/src/lib/util/memory_segment_mapped.cc
index e2ac944..ca9c9c6 100644
--- a/src/lib/util/memory_segment_mapped.cc
+++ b/src/lib/util/memory_segment_mapped.cc
@@ -279,14 +279,14 @@ MemorySegmentMapped::allMemoryDeallocated() const {
return (impl_->base_sgmt_->all_memory_deallocated());
}
-void*
+MemorySegment::NamedAddressResult
MemorySegmentMapped::getNamedAddressImpl(const char* name) {
offset_ptr<void>* storage =
impl_->base_sgmt_->find<offset_ptr<void> >(name).first;
if (storage) {
- return (storage->get());
+ return (NamedAddressResult(true, storage->get()));
}
- return (NULL);
+ return (NamedAddressResult(false, NULL));
}
bool
diff --git a/src/lib/util/memory_segment_mapped.h b/src/lib/util/memory_segment_mapped.h
index 7685e30..6798210 100644
--- a/src/lib/util/memory_segment_mapped.h
+++ b/src/lib/util/memory_segment_mapped.h
@@ -195,7 +195,7 @@ public:
/// \brief Mapped segment version of getNamedAddress.
///
/// This version never throws.
- virtual void* getNamedAddressImpl(const char* name);
+ virtual NamedAddressResult getNamedAddressImpl(const char* name);
/// \brief Mapped segment version of clearNamedAddress.
///
diff --git a/src/lib/util/tests/memory_segment_common_unittest.cc b/src/lib/util/tests/memory_segment_common_unittest.cc
index 3810e0a..4ad9c50 100644
--- a/src/lib/util/tests/memory_segment_common_unittest.cc
+++ b/src/lib/util/tests/memory_segment_common_unittest.cc
@@ -30,9 +30,8 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) {
// NULL name is not allowed.
EXPECT_THROW(segment.getNamedAddress(NULL), InvalidParameter);
- // If the name does not exist, NULL should be returned.
- EXPECT_EQ(static_cast<void*>(NULL),
- segment.getNamedAddress("test address"));
+ // If the name does not exist, false should be returned.
+ EXPECT_FALSE(segment.getNamedAddress("test address").first);
// Now set it
void* ptr32 = segment.allocate(sizeof(uint32_t));
@@ -44,7 +43,8 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) {
EXPECT_THROW(segment.setNamedAddress(NULL, ptr32), InvalidParameter);
// we can now get it; the stored value should be intact.
- EXPECT_EQ(ptr32, segment.getNamedAddress("test address"));
+ EXPECT_EQ(MemorySegment::NamedAddressResult(true, ptr32),
+ segment.getNamedAddress("test address"));
EXPECT_EQ(test_val, *static_cast<const uint32_t*>(ptr32));
// Override it.
@@ -52,20 +52,20 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) {
const uint16_t test_val16 = 4200;
*static_cast<uint16_t*>(ptr16) = test_val16;
EXPECT_FALSE(segment.setNamedAddress("test address", ptr16));
- EXPECT_EQ(ptr16, segment.getNamedAddress("test address"));
+ EXPECT_EQ(MemorySegment::NamedAddressResult(true, ptr16),
+ segment.getNamedAddress("test address"));
EXPECT_EQ(test_val16, *static_cast<const uint16_t*>(ptr16));
// Clear it. Then we won't be able to find it any more.
EXPECT_TRUE(segment.clearNamedAddress("test address"));
- EXPECT_EQ(static_cast<void*>(NULL),
- segment.getNamedAddress("test address"));
+ EXPECT_FALSE(segment.getNamedAddress("test address").first);
// duplicate attempt of clear will result in false as it doesn't exist.
EXPECT_FALSE(segment.clearNamedAddress("test address"));
// Setting NULL is okay.
EXPECT_FALSE(segment.setNamedAddress("null address", NULL));
- EXPECT_EQ(static_cast<void*>(NULL),
+ EXPECT_EQ(MemorySegment::NamedAddressResult(true, NULL),
segment.getNamedAddress("null address"));
// If the underlying implementation performs explicit check against
diff --git a/src/lib/util/tests/memory_segment_mapped_unittest.cc b/src/lib/util/tests/memory_segment_mapped_unittest.cc
index 1d9979d..98e2a3e 100644
--- a/src/lib/util/tests/memory_segment_mapped_unittest.cc
+++ b/src/lib/util/tests/memory_segment_mapped_unittest.cc
@@ -287,12 +287,14 @@ void
checkNamedData(const std::string& name, const std::vector<uint8_t>& data,
MemorySegment& sgmt, bool delete_after_check = false)
{
- void* dp = sgmt.getNamedAddress(name.c_str());
- ASSERT_TRUE(dp);
- EXPECT_EQ(0, std::memcmp(dp, &data[0], data.size()));
+ const MemorySegment::NamedAddressResult result =
+ sgmt.getNamedAddress(name.c_str());
+ ASSERT_TRUE(result.first);
+ ASSERT_TRUE(result.second);
+ EXPECT_EQ(0, std::memcmp(result.second, &data[0], data.size()));
if (delete_after_check) {
- sgmt.deallocate(dp, data.size());
+ sgmt.deallocate(result.second, data.size());
sgmt.clearNamedAddress(name.c_str());
}
}
@@ -309,10 +311,10 @@ TEST_F(MemorySegmentMappedTest, namedAddress) {
segment_.reset(); // close it before opening another one
segment_.reset(new MemorySegmentMapped(mapped_file));
- EXPECT_NE(static_cast<void*>(NULL),
- segment_->getNamedAddress("test address"));
- EXPECT_EQ(test_val16, *static_cast<const uint16_t*>(
- segment_->getNamedAddress("test address")));
+ const MemorySegment::NamedAddressResult result =
+ segment_->getNamedAddress("test address");
+ ASSERT_TRUE(result.first);
+ EXPECT_EQ(test_val16, *static_cast<const uint16_t*>(result.second));
// try to set an unusually long name. We re-create the file so
// creating the name would cause allocation failure and trigger internal
@@ -323,7 +325,7 @@ TEST_F(MemorySegmentMappedTest, namedAddress) {
const std::string long_name(1025, 'x'); // definitely larger than segment
// setNamedAddress should return true, indicating segment has grown.
EXPECT_TRUE(segment_->setNamedAddress(long_name.c_str(), NULL));
- EXPECT_EQ(static_cast<void*>(NULL),
+ EXPECT_EQ(MemorySegment::NamedAddressResult(true, NULL),
segment_->getNamedAddress(long_name.c_str()));
// Check contents pointed by named addresses survive growing and
@@ -410,10 +412,12 @@ TEST_F(MemorySegmentMappedTest, multiProcess) {
EXPECT_EQ(0, from_parent);
MemorySegmentMapped sgmt(mapped_file);
- void* ptr_child = sgmt.getNamedAddress("test address");
- EXPECT_TRUE(ptr_child);
- if (ptr_child) {
- const uint32_t val = *static_cast<const uint32_t*>(ptr_child);
+ const MemorySegment::NamedAddressResult result =
+ sgmt.getNamedAddress("test address");
+ ASSERT_TRUE(result.first);
+ EXPECT_TRUE(result.second);
+ if (result.second) {
+ const uint32_t val = *static_cast<const uint32_t*>(result.second);
EXPECT_EQ(424242, val);
// tell the parent whether it succeeded. 0 means it did,
// 0xff means it failed.
@@ -425,9 +429,11 @@ TEST_F(MemorySegmentMappedTest, multiProcess) {
// parent: open another read-only segment, then tell the child to open
// its own segment.
segment_.reset(new MemorySegmentMapped(mapped_file));
- ptr = segment_->getNamedAddress("test address");
- ASSERT_TRUE(ptr);
- EXPECT_EQ(424242, *static_cast<const uint32_t*>(ptr));
+ const MemorySegment::NamedAddressResult result =
+ segment_->getNamedAddress("test address");
+ ASSERT_TRUE(result.first);
+ ASSERT_TRUE(result.second);
+ EXPECT_EQ(424242, *static_cast<const uint32_t*>(result.second));
const char some_data = 0;
EXPECT_EQ(1, write(pipe_to_child.getWriteFD(), &some_data,
sizeof(some_data)));
@@ -477,9 +483,11 @@ TEST_F(MemorySegmentMappedTest, violateReadOnly) {
if (!isc::util::unittests::runningOnValgrind()) {
EXPECT_DEATH_IF_SUPPORTED({
MemorySegmentMapped segment_ro(mapped_file);
- EXPECT_TRUE(segment_ro.getNamedAddress("test address"));
- *static_cast<uint32_t*>(
- segment_ro.getNamedAddress("test address")) = 0;
+ const MemorySegment::NamedAddressResult result =
+ segment_ro.getNamedAddress("test address");
+ ASSERT_TRUE(result.first);
+ ASSERT_TRUE(result.second);
+ *static_cast<uint32_t*>(result.second) = 0;
}, "");
}
@@ -487,10 +495,12 @@ TEST_F(MemorySegmentMappedTest, violateReadOnly) {
// attempts are prohibited. When detectable it must result in an
// exception.
MemorySegmentMapped segment_ro(mapped_file);
- ptr = segment_ro.getNamedAddress("test address");
- EXPECT_NE(static_cast<void*>(NULL), ptr);
+ const MemorySegment::NamedAddressResult result =
+ segment_ro.getNamedAddress("test address");
+ ASSERT_TRUE(result.first);
+ EXPECT_NE(static_cast<void*>(NULL), result.second);
- EXPECT_THROW(segment_ro.deallocate(ptr, 4), MemorySegmentError);
+ EXPECT_THROW(segment_ro.deallocate(result.second, 4), MemorySegmentError);
EXPECT_THROW(segment_ro.allocate(16), MemorySegmentError);
// allocation that would otherwise require growing the segment; permission
More information about the bind10-changes
mailing list