Minor code cleanups in vector.

Fixed a potential bug where calling replaceAt with a reference to
an existing element in the vector at the same index would cause
the element to be destroyed while being copied to itself.

Refactored the conditions in _grow and _shrink for clarity.
The computations are exactly the same but I think it reads better
this way.  In particular, the ssize_t variable 's' is gone: it didn't
need to be signed anyways because its value could never be negative.

Change-Id: If087841c15e6a87160eee874720c4a77eb0e99a6
This commit is contained in:
Jeff Brown 2011-07-14 00:29:49 -07:00
parent 686f62fcaf
commit c105333997

View File

@ -252,6 +252,7 @@ ssize_t VectorImpl::replaceAt(const void* prototype, size_t index)
"[%p] replace: index=%d, size=%d", this, (int)index, (int)size()); "[%p] replace: index=%d, size=%d", this, (int)index, (int)size());
void* item = editItemLocation(index); void* item = editItemLocation(index);
if (item != prototype) {
if (item == 0) if (item == 0)
return NO_MEMORY; return NO_MEMORY;
_do_destroy(item, 1); _do_destroy(item, 1);
@ -260,6 +261,7 @@ ssize_t VectorImpl::replaceAt(const void* prototype, size_t index)
} else { } else {
_do_copy(item, prototype, 1); _do_copy(item, prototype, 1);
} }
}
return ssize_t(index); return ssize_t(index);
} }
@ -367,10 +369,10 @@ void* VectorImpl::_grow(size_t where, size_t amount)
SharedBuffer* sb = SharedBuffer::alloc(new_capacity * mItemSize); SharedBuffer* sb = SharedBuffer::alloc(new_capacity * mItemSize);
if (sb) { if (sb) {
void* array = sb->data(); void* array = sb->data();
if (where>0) { if (where != 0) {
_do_copy(array, mStorage, where); _do_copy(array, mStorage, where);
} }
if (mCount>where) { if (where != mCount) {
const void* from = reinterpret_cast<const uint8_t *>(mStorage) + where*mItemSize; const void* from = reinterpret_cast<const uint8_t *>(mStorage) + where*mItemSize;
void* dest = reinterpret_cast<uint8_t *>(array) + (where+amount)*mItemSize; void* dest = reinterpret_cast<uint8_t *>(array) + (where+amount)*mItemSize;
_do_copy(dest, from, mCount-where); _do_copy(dest, from, mCount-where);
@ -380,15 +382,14 @@ void* VectorImpl::_grow(size_t where, size_t amount)
} }
} }
} else { } else {
ssize_t s = mCount-where; if (where != mCount) {
if (s>0) {
void* array = editArrayImpl(); void* array = editArrayImpl();
void* to = reinterpret_cast<uint8_t *>(array) + (where+amount)*mItemSize;
const void* from = reinterpret_cast<const uint8_t *>(array) + where*mItemSize; const void* from = reinterpret_cast<const uint8_t *>(array) + where*mItemSize;
_do_move_forward(to, from, s); void* to = reinterpret_cast<uint8_t *>(array) + (where+amount)*mItemSize;
_do_move_forward(to, from, mCount - where);
} }
} }
mCount += amount; mCount = new_size;
void* free_space = const_cast<void*>(itemLocation(where)); void* free_space = const_cast<void*>(itemLocation(where));
return free_space; return free_space;
} }
@ -409,7 +410,7 @@ void VectorImpl::_shrink(size_t where, size_t amount)
if (new_size*3 < capacity()) { if (new_size*3 < capacity()) {
const size_t new_capacity = max(kMinVectorCapacity, new_size*2); const size_t new_capacity = max(kMinVectorCapacity, new_size*2);
// LOGV("shrink vector %p, new_capacity=%d", this, (int)new_capacity); // LOGV("shrink vector %p, new_capacity=%d", this, (int)new_capacity);
if ((where == mCount-amount) && if ((where == new_size) &&
(mFlags & HAS_TRIVIAL_COPY) && (mFlags & HAS_TRIVIAL_COPY) &&
(mFlags & HAS_TRIVIAL_DTOR)) (mFlags & HAS_TRIVIAL_DTOR))
{ {
@ -420,13 +421,13 @@ void VectorImpl::_shrink(size_t where, size_t amount)
SharedBuffer* sb = SharedBuffer::alloc(new_capacity * mItemSize); SharedBuffer* sb = SharedBuffer::alloc(new_capacity * mItemSize);
if (sb) { if (sb) {
void* array = sb->data(); void* array = sb->data();
if (where>0) { if (where != 0) {
_do_copy(array, mStorage, where); _do_copy(array, mStorage, where);
} }
if (mCount > where+amount) { if (where != new_size) {
const void* from = reinterpret_cast<const uint8_t *>(mStorage) + (where+amount)*mItemSize; const void* from = reinterpret_cast<const uint8_t *>(mStorage) + (where+amount)*mItemSize;
void* dest = reinterpret_cast<uint8_t *>(array) + where*mItemSize; void* dest = reinterpret_cast<uint8_t *>(array) + where*mItemSize;
_do_copy(dest, from, mCount-(where+amount)); _do_copy(dest, from, new_size - where);
} }
release_storage(); release_storage();
mStorage = const_cast<void*>(array); mStorage = const_cast<void*>(array);
@ -436,15 +437,12 @@ void VectorImpl::_shrink(size_t where, size_t amount)
void* array = editArrayImpl(); void* array = editArrayImpl();
void* to = reinterpret_cast<uint8_t *>(array) + where*mItemSize; void* to = reinterpret_cast<uint8_t *>(array) + where*mItemSize;
_do_destroy(to, amount); _do_destroy(to, amount);
ssize_t s = mCount-(where+amount); if (where != new_size) {
if (s>0) {
const void* from = reinterpret_cast<uint8_t *>(array) + (where+amount)*mItemSize; const void* from = reinterpret_cast<uint8_t *>(array) + (where+amount)*mItemSize;
_do_move_backward(to, from, s); _do_move_backward(to, from, new_size - where);
} }
} }
mCount = new_size;
// adjust the number of items...
mCount -= amount;
} }
size_t VectorImpl::itemSize() const { size_t VectorImpl::itemSize() const {