improve Vector<> safety checks
- make errors that will always cause a memory corruption always fatal
(for eg: KeyedVector<>::editValue{For|At}() failure)
- make other errors fatal in debug mode, those that can be caught by
the caller.
- fix typos
Change-Id: I65cc7d81035c37ce2906fc4500c50e5d5b5c49eb
diff --git a/include/utils/KeyedVector.h b/include/utils/KeyedVector.h
index 47c2c56..c4faae0 100644
--- a/include/utils/KeyedVector.h
+++ b/include/utils/KeyedVector.h
@@ -21,6 +21,8 @@
#include <stdint.h>
#include <sys/types.h>
+#include <cutils/log.h>
+
#include <utils/SortedVector.h>
#include <utils/TypeHelpers.h>
#include <utils/Errors.h>
@@ -50,11 +52,11 @@
//! returns number of items in the vector
inline size_t size() const { return mVector.size(); }
- //! returns wether or not the vector is empty
+ //! returns whether or not the vector is empty
inline bool isEmpty() const { return mVector.isEmpty(); }
//! returns how many items can be stored without reallocating the backing store
inline size_t capacity() const { return mVector.capacity(); }
- //! setst the capacity. capacity can never be reduced less than size()
+ //! sets the capacity. capacity can never be reduced less than size()
inline ssize_t setCapacity(size_t size) { return mVector.setCapacity(size); }
// returns true if the arguments is known to be identical to this vector
@@ -139,7 +141,7 @@
template<typename KEY, typename VALUE> inline
const VALUE& KeyedVector<KEY,VALUE>::valueFor(const KEY& key) const {
ssize_t i = this->indexOfKey(key);
- assert(i>=0);
+ LOG_ALWAYS_FATAL_IF(i<0, "%s: key not found", __PRETTY_FUNCTION__);
return mVector.itemAt(i).value;
}
@@ -161,7 +163,7 @@
template<typename KEY, typename VALUE> inline
VALUE& KeyedVector<KEY,VALUE>::editValueFor(const KEY& key) {
ssize_t i = this->indexOfKey(key);
- assert(i>=0);
+ LOG_ALWAYS_FATAL_IF(i<0, "%s: key not found", __PRETTY_FUNCTION__);
return mVector.editItemAt(i).value;
}
diff --git a/include/utils/SortedVector.h b/include/utils/SortedVector.h
index 2445525..fd1cb82 100644
--- a/include/utils/SortedVector.h
+++ b/include/utils/SortedVector.h
@@ -21,6 +21,8 @@
#include <stdint.h>
#include <sys/types.h>
+#include <cutils/log.h>
+
#include <utils/Vector.h>
#include <utils/VectorImpl.h>
#include <utils/TypeHelpers.h>
@@ -61,11 +63,11 @@
//! returns number of items in the vector
inline size_t size() const { return VectorImpl::size(); }
- //! returns wether or not the vector is empty
+ //! returns whether or not the vector is empty
inline bool isEmpty() const { return VectorImpl::isEmpty(); }
//! returns how many items can be stored without reallocating the backing store
inline size_t capacity() const { return VectorImpl::capacity(); }
- //! setst the capacity. capacity can never be reduced less than size()
+ //! sets the capacity. capacity can never be reduced less than size()
inline ssize_t setCapacity(size_t size) { return VectorImpl::setCapacity(size); }
/*!
@@ -76,7 +78,7 @@
inline const TYPE* array() const;
//! read-write C-style access. BE VERY CAREFUL when modifying the array
- //! you ust keep it sorted! You usually don't use this function.
+ //! you must keep it sorted! You usually don't use this function.
TYPE* editArray();
//! finds the index of an item
@@ -100,7 +102,7 @@
const TYPE& mirrorItemAt(ssize_t index) const;
/*!
- * modifing the array
+ * modifying the array
*/
//! add an item in the right place (and replace the one that is there)
@@ -186,7 +188,9 @@
template<class TYPE> inline
const TYPE& SortedVector<TYPE>::operator[](size_t index) const {
- assert( index<size() );
+ LOG_FATAL_IF(index>=size(),
+ "%s: index=%u out of range (%u)", __PRETTY_FUNCTION__,
+ int(index), int(size()));
return *(array() + index);
}
@@ -197,8 +201,11 @@
template<class TYPE> inline
const TYPE& SortedVector<TYPE>::mirrorItemAt(ssize_t index) const {
- assert( (index>0 ? index : -index)<size() );
- return *(array() + ((index<0) ? (size()-index) : index));
+ const size_t i = index>0 ? index : -index;
+ LOG_FATAL_IF(index>=size(),
+ "%s: index=%u out of range (%u)", __PRETTY_FUNCTION__,
+ int(index), int(size()));
+ return *(array() + i);
}
template<class TYPE> inline
diff --git a/include/utils/Vector.h b/include/utils/Vector.h
index a89393f..506acae 100644
--- a/include/utils/Vector.h
+++ b/include/utils/Vector.h
@@ -21,7 +21,8 @@
#include <stdint.h>
#include <sys/types.h>
-#include <utils/Log.h>
+#include <cutils/log.h>
+
#include <utils/VectorImpl.h>
#include <utils/TypeHelpers.h>
@@ -271,8 +272,9 @@
template<class TYPE> inline
const TYPE& Vector<TYPE>::operator[](size_t index) const {
- LOG_FATAL_IF( index>=size(),
- "itemAt: index %d is past size %d", (int)index, (int)size() );
+ LOG_FATAL_IF(index>=size(),
+ "%s: index=%u out of range (%u)", __PRETTY_FUNCTION__,
+ int(index), int(size()));
return *(array() + index);
}
@@ -283,10 +285,11 @@
template<class TYPE> inline
const TYPE& Vector<TYPE>::mirrorItemAt(ssize_t index) const {
- LOG_FATAL_IF( (index>0 ? index : -index)>=size(),
- "mirrorItemAt: index %d is past size %d",
- (int)index, (int)size() );
- return *(array() + ((index<0) ? (size()-index) : index));
+ const size_t i = index>0 ? index : -index;
+ LOG_FATAL_IF(index>=size(),
+ "%s: index=%u out of range (%u)", __PRETTY_FUNCTION__,
+ int(index), int(size()));
+ return *(array() + i);
}
template<class TYPE> inline
diff --git a/include/utils/VectorImpl.h b/include/utils/VectorImpl.h
index c4ec2ff..b1224c6 100644
--- a/include/utils/VectorImpl.h
+++ b/include/utils/VectorImpl.h
@@ -104,16 +104,6 @@
virtual void do_splat(void* dest, const void* item, size_t num) const = 0;
virtual void do_move_forward(void* dest, const void* from, size_t num) const = 0;
virtual void do_move_backward(void* dest, const void* from, size_t num) const = 0;
-
- // take care of FBC...
- virtual void reservedVectorImpl1();
- virtual void reservedVectorImpl2();
- virtual void reservedVectorImpl3();
- virtual void reservedVectorImpl4();
- virtual void reservedVectorImpl5();
- virtual void reservedVectorImpl6();
- virtual void reservedVectorImpl7();
- virtual void reservedVectorImpl8();
private:
void* _grow(size_t where, size_t amount);
@@ -165,16 +155,6 @@
protected:
virtual int do_compare(const void* lhs, const void* rhs) const = 0;
- // take care of FBC...
- virtual void reservedSortedVectorImpl1();
- virtual void reservedSortedVectorImpl2();
- virtual void reservedSortedVectorImpl3();
- virtual void reservedSortedVectorImpl4();
- virtual void reservedSortedVectorImpl5();
- virtual void reservedSortedVectorImpl6();
- virtual void reservedSortedVectorImpl7();
- virtual void reservedSortedVectorImpl8();
-
private:
ssize_t _indexOrderOf(const void* item, size_t* order = 0) const;
diff --git a/libs/utils/VectorImpl.cpp b/libs/utils/VectorImpl.cpp
index e78faa8..020ec15 100644
--- a/libs/utils/VectorImpl.cpp
+++ b/libs/utils/VectorImpl.cpp
@@ -20,7 +20,8 @@
#include <stdlib.h>
#include <stdio.h>
-#include <utils/Log.h>
+#include <cutils/log.h>
+
#include <utils/Errors.h>
#include <utils/SharedBuffer.h>
#include <utils/VectorImpl.h>
@@ -56,9 +57,8 @@
VectorImpl::~VectorImpl()
{
- ALOG_ASSERT(!mCount,
- "[%p] "
- "subclasses of VectorImpl must call finish_vector()"
+ ALOGW_IF(mCount,
+ "[%p] subclasses of VectorImpl must call finish_vector()"
" in their destructor. Leaking %d bytes.",
this, (int)(mCount*mItemSize));
// We can't call _do_destroy() here because the vtable is already gone.
@@ -66,7 +66,7 @@
VectorImpl& VectorImpl::operator = (const VectorImpl& rhs)
{
- ALOG_ASSERT(mItemSize == rhs.mItemSize,
+ LOG_ALWAYS_FATAL_IF(mItemSize != rhs.mItemSize,
"Vector<> have different types (this=%p, rhs=%p)", this, &rhs);
if (this != &rhs) {
release_storage();
@@ -251,6 +251,10 @@
ALOG_ASSERT(index<size(),
"[%p] replace: index=%d, size=%d", this, (int)index, (int)size());
+ if (index >= size()) {
+ return BAD_INDEX;
+ }
+
void* item = editItemLocation(index);
if (item != prototype) {
if (item == 0)
@@ -294,10 +298,13 @@
ALOG_ASSERT(index<capacity(),
"[%p] editItemLocation: index=%d, capacity=%d, count=%d",
this, (int)index, (int)capacity(), (int)mCount);
-
- void* buffer = editArrayImpl();
- if (buffer)
- return reinterpret_cast<char*>(buffer) + index*mItemSize;
+
+ if (index < capacity()) {
+ void* buffer = editArrayImpl();
+ if (buffer) {
+ return reinterpret_cast<char*>(buffer) + index*mItemSize;
+ }
+ }
return 0;
}
@@ -307,9 +314,12 @@
"[%p] itemLocation: index=%d, capacity=%d, count=%d",
this, (int)index, (int)capacity(), (int)mCount);
- const void* buffer = arrayImpl();
- if (buffer)
- return reinterpret_cast<const char*>(buffer) + index*mItemSize;
+ if (index < capacity()) {
+ const void* buffer = arrayImpl();
+ if (buffer) {
+ return reinterpret_cast<const char*>(buffer) + index*mItemSize;
+ }
+ }
return 0;
}
@@ -484,15 +494,6 @@
do_move_backward(dest, from, num);
}
-void VectorImpl::reservedVectorImpl1() { }
-void VectorImpl::reservedVectorImpl2() { }
-void VectorImpl::reservedVectorImpl3() { }
-void VectorImpl::reservedVectorImpl4() { }
-void VectorImpl::reservedVectorImpl5() { }
-void VectorImpl::reservedVectorImpl6() { }
-void VectorImpl::reservedVectorImpl7() { }
-void VectorImpl::reservedVectorImpl8() { }
-
/*****************************************************************************/
SortedVectorImpl::SortedVectorImpl(size_t itemSize, uint32_t flags)
@@ -608,16 +609,6 @@
return i;
}
-void SortedVectorImpl::reservedSortedVectorImpl1() { };
-void SortedVectorImpl::reservedSortedVectorImpl2() { };
-void SortedVectorImpl::reservedSortedVectorImpl3() { };
-void SortedVectorImpl::reservedSortedVectorImpl4() { };
-void SortedVectorImpl::reservedSortedVectorImpl5() { };
-void SortedVectorImpl::reservedSortedVectorImpl6() { };
-void SortedVectorImpl::reservedSortedVectorImpl7() { };
-void SortedVectorImpl::reservedSortedVectorImpl8() { };
-
-
/*****************************************************************************/
}; // namespace android