BIND 10 trac2850_4, updated. 54c8596393d90528dad115ccf111a5dbc51c5b73 [2850] Use non-const implementation of allMemoryDeallocated() again
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue May 21 08:10:10 UTC 2013
The branch, trac2850_4 has been updated
via 54c8596393d90528dad115ccf111a5dbc51c5b73 (commit)
via 14398c533d5f036d1c005565d9236f6721390c5d (commit)
via 62b6680e17683955b24149d63fd9a3f329e0636c (commit)
from 5fe71727421c85412f6df8c77f644f69ee36c6f4 (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 54c8596393d90528dad115ccf111a5dbc51c5b73
Author: Mukund Sivaraman <muks at isc.org>
Date: Tue May 21 13:38:23 2013 +0530
[2850] Use non-const implementation of allMemoryDeallocated() again
commit 14398c533d5f036d1c005565d9236f6721390c5d
Author: Mukund Sivaraman <muks at isc.org>
Date: Tue May 21 13:16:28 2013 +0530
[2850] Validate names in all name-related methods
commit 62b6680e17683955b24149d63fd9a3f329e0636c
Author: Mukund Sivaraman <muks at isc.org>
Date: Tue May 21 12:44:18 2013 +0530
[2850] Move allocate_size_ to private class
-----------------------------------------------------------------------
Summary of changes:
src/lib/util/memory_segment.h | 82 +++++++++++++-------
src/lib/util/memory_segment_mapped.cc | 25 +++---
src/lib/util/memory_segment_mapped.h | 1 -
.../util/tests/memory_segment_common_unittest.cc | 6 ++
.../util/tests/memory_segment_mapped_unittest.cc | 6 +-
5 files changed, 77 insertions(+), 43 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/util/memory_segment.h b/src/lib/util/memory_segment.h
index 2563112..da2c5ec 100644
--- a/src/lib/util/memory_segment.h
+++ b/src/lib/util/memory_segment.h
@@ -171,13 +171,13 @@ public:
/// corresponding address by that name (in such cases the real address
/// may be different between these two processes).
///
- /// Note that names beginning with an underscore (such as
- /// \c "_example") are reserved for internal use by this class. If such
- /// a name is passed to this method, an \c isc::InvalidParameter
- /// exception will be thrown.
+ /// Names beginning with an underscore (such as \c "_example") are
+ /// reserved for internal use by this class. If such a name is
+ /// passed to this method, an \c isc::InvalidParameter exception
+ /// will be thrown.
///
- /// Note that empty names (\c "") are not allowed too. If an empty name
- /// is passed to this method, an \c isc::InvalidParameter exception
+ /// Empty names (\c "") are not allowed too. If an empty name is
+ /// passed to this method, an \c isc::InvalidParameter exception
/// will be thrown.
///
/// \c addr must be 0 (NULL) or an address that belongs to this segment.
@@ -223,7 +223,8 @@ public:
///
/// \throw std::bad_alloc Allocation of a segment space for the given name
/// failed.
- /// \throw InvalidParameter name is NULL.
+ /// \throw InvalidParameter name is NULL, empty ("") or begins with
+ /// an underscore ('_').
/// \throw MemorySegmentError Failure of implementation specific
/// validation.
///
@@ -235,18 +236,7 @@ public:
// This public method implements common validation. The actual
// work specific to the derived segment is delegated to the
// corresponding protected method.
- if (!name) {
- isc_throw(InvalidParameter,
- "NULL name is given to setNamedAddress");
- }
- if (*name == '\0') {
- isc_throw(InvalidParameter,
- "Empty name was passed to setNamedAddress");
- } else if (*name == '_') {
- isc_throw(InvalidParameter,
- "Names beginning with _ are reserved for "
- "internal use only.");
- }
+ validateName(name);
return (setNamedAddressImpl(name, addr));
}
@@ -260,13 +250,23 @@ public:
/// associated by a prior call to \c setNameAddress(). If no address
/// associated with the given name is found, it returns NULL.
///
+ /// Names beginning with an underscore (such as \c "_example") are
+ /// reserved for internal use by this class. If such a name is
+ /// passed to this method, an \c isc::InvalidParameter exception
+ /// will be thrown.
+ ///
+ /// Empty names (\c "") are not allowed too. If an empty name is
+ /// passed to this method, an \c isc::InvalidParameter exception
+ /// will be thrown.
+ ///
/// This method should generally be considered exception free, but there
/// can be a small chance it throws, depending on the internal
/// implementation (e.g., if it converts the name to std::string), so the
/// API doesn't guarantee that property. In general, if this method
/// throws it should be considered a fatal condition.
///
- /// \throw InvalidParameter name is NULL.
+ /// \throw InvalidParameter name is NULL, empty ("") or begins with
+ /// an underscore ('_').
///
/// \param name A C string of which the segment memory address is to be
/// returned. Must not be NULL.
@@ -277,10 +277,7 @@ public:
// This public method implements common validation. The actual
// work specific to the derived segment is delegated to the
// corresponding protected method.
- if (!name) {
- isc_throw(InvalidParameter,
- "NULL name is given to getNamedAddress");
- }
+ validateName(name);
return (getNamedAddressImpl(name));
}
@@ -291,9 +288,19 @@ public:
/// \c setNamedAddress(). If there is no association for the given name
/// this method returns false; otherwise it returns true.
///
+ /// Names beginning with an underscore (such as \c "_example") are
+ /// reserved for internal use by this class. If such a name is
+ /// passed to this method, an \c isc::InvalidParameter exception
+ /// will be thrown.
+ ///
+ /// Empty names (\c "") are not allowed too. If an empty name is
+ /// passed to this method, an \c isc::InvalidParameter exception
+ /// will be thrown.
+ ///
/// See \c getNamedAddress() about exception consideration.
///
- /// \throw InvalidParameter name is NULL.
+ /// \throw InvalidParameter name is NULL, empty ("") or begins with
+ /// an underscore ('_').
/// \throw MemorySegmentError Failure of implementation specific
/// validation.
///
@@ -303,11 +310,32 @@ public:
// This public method implements common validation. The actual
// work specific to the derived segment is delegated to the
// corresponding protected method.
+ validateName(name);
+ return (clearNamedAddressImpl(name));
+ }
+
+private:
+ /// \brief Validate the passed name.
+ ///
+ /// This method validates the passed name (for name/address pairs)
+ /// and throws \c InvalidParameter if the name fails
+ /// validation. Otherwise, it does nothing.
+ ///
+ /// \throw InvalidParameter name is NULL, empty ("") or begins with
+ /// an underscore ('_').
+ static void validateName(const char* name) {
if (!name) {
isc_throw(InvalidParameter,
- "NULL name is given to clearNamedAddress");
+ "NULL is invalid for a name.");
+ }
+ if (*name == '\0') {
+ isc_throw(InvalidParameter,
+ "Empty names are invalid.");
+ } else if (*name == '_') {
+ isc_throw(InvalidParameter,
+ "Names beginning with '_' are reserved for "
+ "internal use only.");
}
- return (clearNamedAddressImpl(name));
}
protected:
diff --git a/src/lib/util/memory_segment_mapped.cc b/src/lib/util/memory_segment_mapped.cc
index 0cc011b..55ba395 100644
--- a/src/lib/util/memory_segment_mapped.cc
+++ b/src/lib/util/memory_segment_mapped.cc
@@ -236,8 +236,7 @@ private:
};
MemorySegmentMapped::MemorySegmentMapped(const std::string& filename) :
- impl_(NULL),
- allocated_size_(0)
+ impl_(NULL)
{
try {
impl_ = new Impl(filename, true);
@@ -250,8 +249,7 @@ MemorySegmentMapped::MemorySegmentMapped(const std::string& filename) :
MemorySegmentMapped::MemorySegmentMapped(const std::string& filename,
OpenMode mode, size_t initial_size) :
- impl_(NULL),
- allocated_size_(0)
+ impl_(NULL)
{
try {
switch (mode) {
@@ -295,7 +293,6 @@ MemorySegmentMapped::allocate(size_t size) {
if (impl_->base_sgmt_->get_free_memory() >= size) {
void* ptr = impl_->base_sgmt_->allocate(size, std::nothrow);
if (ptr) {
- allocated_size_ += size;
return (ptr);
}
}
@@ -311,7 +308,7 @@ MemorySegmentMapped::allocate(size_t size) {
}
void
-MemorySegmentMapped::deallocate(void* ptr, size_t size) {
+MemorySegmentMapped::deallocate(void* ptr, size_t) {
if (impl_->read_only_) {
isc_throw(MemorySegmentError,
"deallocate attempt on read-only segment");
@@ -324,15 +321,21 @@ MemorySegmentMapped::deallocate(void* ptr, size_t size) {
}
impl_->base_sgmt_->deallocate(ptr);
- allocated_size_ -= size;
}
bool
MemorySegmentMapped::allMemoryDeallocated() const {
- const size_t expected_num_named_objs = impl_->read_only_ ? 0 : 1;
- const size_t num_named_objs = impl_->base_sgmt_->get_num_named_objects();
- return ((allocated_size_ == 0) &&
- (num_named_objs == expected_num_named_objs));
+ // This method is not technically const, but it reserves the
+ // const-ness property. In case of exceptions, we abort here. (See
+ // ticket #2850 for additional commentary.)
+ try {
+ impl_->freeReservedMemory();
+ const bool result = impl_->base_sgmt_->all_memory_deallocated();
+ impl_->reserveMemory();
+ return (result);
+ } catch (...) {
+ abort();
+ }
}
MemorySegment::NamedAddressResult
diff --git a/src/lib/util/memory_segment_mapped.h b/src/lib/util/memory_segment_mapped.h
index d52ab98..301b174 100644
--- a/src/lib/util/memory_segment_mapped.h
+++ b/src/lib/util/memory_segment_mapped.h
@@ -256,7 +256,6 @@ public:
private:
struct Impl;
Impl* impl_;
- size_t allocated_size_;
};
} // namespace util
diff --git a/src/lib/util/tests/memory_segment_common_unittest.cc b/src/lib/util/tests/memory_segment_common_unittest.cc
index 2da7751..17719ae 100644
--- a/src/lib/util/tests/memory_segment_common_unittest.cc
+++ b/src/lib/util/tests/memory_segment_common_unittest.cc
@@ -41,12 +41,18 @@ checkSegmentNamedAddress(MemorySegment& segment, bool out_of_segment_ok) {
// NULL name isn't allowed.
EXPECT_THROW(segment.setNamedAddress(NULL, ptr32), InvalidParameter);
+ EXPECT_THROW(segment.getNamedAddress(NULL), InvalidParameter);
+ EXPECT_THROW(segment.clearNamedAddress(NULL), InvalidParameter);
// Empty names are not allowed.
EXPECT_THROW(segment.setNamedAddress("", ptr32), InvalidParameter);
+ EXPECT_THROW(segment.getNamedAddress(""), InvalidParameter);
+ EXPECT_THROW(segment.clearNamedAddress(""), InvalidParameter);
// Names beginning with _ are not allowed.
EXPECT_THROW(segment.setNamedAddress("_foo", ptr32), InvalidParameter);
+ EXPECT_THROW(segment.getNamedAddress("_foo"), InvalidParameter);
+ EXPECT_THROW(segment.clearNamedAddress("_foo"), InvalidParameter);
// we can now get it; the stored value should be intact.
MemorySegment::NamedAddressResult result =
diff --git a/src/lib/util/tests/memory_segment_mapped_unittest.cc b/src/lib/util/tests/memory_segment_mapped_unittest.cc
index 1a22721..287ee38 100644
--- a/src/lib/util/tests/memory_segment_mapped_unittest.cc
+++ b/src/lib/util/tests/memory_segment_mapped_unittest.cc
@@ -276,13 +276,11 @@ TEST_F(MemorySegmentMappedTest, badDeallocate) {
resetSegment();
}
- // Invalid size; this implementation doesn't detect such errors and
- // will free the memory, but \c allMemoryDeallocated() will detect
- // it.
+ // Invalid size; this implementation doesn't detect such errors.
ptr = segment_->allocate(4);
EXPECT_NE(static_cast<void*>(NULL), ptr);
segment_->deallocate(ptr, 8);
- EXPECT_FALSE(segment_->allMemoryDeallocated());
+ EXPECT_TRUE(segment_->allMemoryDeallocated());
}
// A helper of namedAddress.
More information about the bind10-changes
mailing list