Merge "stagefright/foundation: Increase AMessage max item count to 256" into sc-dev
diff --git a/media/libstagefright/foundation/AMessage.cpp b/media/libstagefright/foundation/AMessage.cpp
index 6bb7b37..c2114b3 100644
--- a/media/libstagefright/foundation/AMessage.cpp
+++ b/media/libstagefright/foundation/AMessage.cpp
@@ -54,13 +54,11 @@
AMessage::AMessage(void)
: mWhat(0),
- mTarget(0),
- mNumItems(0) {
+ mTarget(0) {
}
AMessage::AMessage(uint32_t what, const sp<const AHandler> &handler)
- : mWhat(what),
- mNumItems(0) {
+ : mWhat(what) {
setTarget(handler);
}
@@ -89,13 +87,13 @@
}
void AMessage::clear() {
- for (size_t i = 0; i < mNumItems; ++i) {
- Item *item = &mItems[i];
- delete[] item->mName;
- item->mName = NULL;
- freeItemValue(item);
+ // Item needs to be handled delicately
+ for (Item &item : mItems) {
+ delete[] item.mName;
+ item.mName = NULL;
+ freeItemValue(&item);
}
- mNumItems = 0;
+ mItems.clear();
}
void AMessage::freeItemValue(Item *item) {
@@ -157,7 +155,7 @@
size_t memchecks = 0;
#endif
size_t i = 0;
- for (; i < mNumItems; i++) {
+ for (; i < mItems.size(); i++) {
if (len != mItems[i].mNameLength) {
continue;
}
@@ -172,7 +170,7 @@
{
Mutex::Autolock _l(gLock);
++gFindItemCalls;
- gAverageNumItems += mNumItems;
+ gAverageNumItems += mItems.size();
gAverageNumMemChecks += memchecks;
gAverageNumChecks += i;
reportStats();
@@ -188,20 +186,26 @@
memcpy((void*)mName, name, len + 1);
}
+AMessage::Item::Item(const char *name, size_t len)
+ : mType(kTypeInt32) {
+ // mName and mNameLength are initialized by setName
+ setName(name, len);
+}
+
AMessage::Item *AMessage::allocateItem(const char *name) {
size_t len = strlen(name);
size_t i = findItemIndex(name, len);
Item *item;
- if (i < mNumItems) {
+ if (i < mItems.size()) {
item = &mItems[i];
freeItemValue(item);
} else {
- CHECK(mNumItems < kMaxNumItems);
- i = mNumItems++;
+ CHECK(mItems.size() < kMaxNumItems);
+ i = mItems.size();
+ // place a 'blank' item at the end - this is of type kTypeInt32
+ mItems.emplace_back(name, len);
item = &mItems[i];
- item->mType = kTypeInt32;
- item->setName(name, len);
}
return item;
@@ -210,7 +214,7 @@
const AMessage::Item *AMessage::findItem(
const char *name, Type type) const {
size_t i = findItemIndex(name, strlen(name));
- if (i < mNumItems) {
+ if (i < mItems.size()) {
const Item *item = &mItems[i];
return item->mType == type ? item : NULL;
@@ -220,7 +224,7 @@
bool AMessage::findAsFloat(const char *name, float *value) const {
size_t i = findItemIndex(name, strlen(name));
- if (i < mNumItems) {
+ if (i < mItems.size()) {
const Item *item = &mItems[i];
switch (item->mType) {
case kTypeFloat:
@@ -247,7 +251,7 @@
bool AMessage::findAsInt64(const char *name, int64_t *value) const {
size_t i = findItemIndex(name, strlen(name));
- if (i < mNumItems) {
+ if (i < mItems.size()) {
const Item *item = &mItems[i];
switch (item->mType) {
case kTypeInt64:
@@ -265,15 +269,16 @@
bool AMessage::contains(const char *name) const {
size_t i = findItemIndex(name, strlen(name));
- return i < mNumItems;
+ return i < mItems.size();
}
#define BASIC_TYPE(NAME,FIELDNAME,TYPENAME) \
void AMessage::set##NAME(const char *name, TYPENAME value) { \
Item *item = allocateItem(name); \
- \
- item->mType = kType##NAME; \
- item->u.FIELDNAME = value; \
+ if (item) { \
+ item->mType = kType##NAME; \
+ item->u.FIELDNAME = value; \
+ } \
} \
\
/* NOLINT added to avoid incorrect warning/fix from clang.tidy */ \
@@ -298,8 +303,10 @@
void AMessage::setString(
const char *name, const char *s, ssize_t len) {
Item *item = allocateItem(name);
- item->mType = kTypeString;
- item->u.stringValue = new AString(s, len < 0 ? strlen(s) : len);
+ if (item) {
+ item->mType = kTypeString;
+ item->u.stringValue = new AString(s, len < 0 ? strlen(s) : len);
+ }
}
void AMessage::setString(
@@ -310,10 +317,12 @@
void AMessage::setObjectInternal(
const char *name, const sp<RefBase> &obj, Type type) {
Item *item = allocateItem(name);
- item->mType = type;
+ if (item) {
+ item->mType = type;
- if (obj != NULL) { obj->incStrong(this); }
- item->u.refValue = obj.get();
+ if (obj != NULL) { obj->incStrong(this); }
+ item->u.refValue = obj.get();
+ }
}
void AMessage::setObject(const char *name, const sp<RefBase> &obj) {
@@ -326,22 +335,26 @@
void AMessage::setMessage(const char *name, const sp<AMessage> &obj) {
Item *item = allocateItem(name);
- item->mType = kTypeMessage;
+ if (item) {
+ item->mType = kTypeMessage;
- if (obj != NULL) { obj->incStrong(this); }
- item->u.refValue = obj.get();
+ if (obj != NULL) { obj->incStrong(this); }
+ item->u.refValue = obj.get();
+ }
}
void AMessage::setRect(
const char *name,
int32_t left, int32_t top, int32_t right, int32_t bottom) {
Item *item = allocateItem(name);
- item->mType = kTypeRect;
+ if (item) {
+ item->mType = kTypeRect;
- item->u.rectValue.mLeft = left;
- item->u.rectValue.mTop = top;
- item->u.rectValue.mRight = right;
- item->u.rectValue.mBottom = bottom;
+ item->u.rectValue.mLeft = left;
+ item->u.rectValue.mTop = top;
+ item->u.rectValue.mRight = right;
+ item->u.rectValue.mBottom = bottom;
+ }
}
bool AMessage::findString(const char *name, AString *value) const {
@@ -466,18 +479,18 @@
sp<AMessage> AMessage::dup() const {
sp<AMessage> msg = new AMessage(mWhat, mHandler.promote());
- msg->mNumItems = mNumItems;
+ msg->mItems = mItems;
#ifdef DUMP_STATS
{
Mutex::Autolock _l(gLock);
++gDupCalls;
- gAverageDupItems += mNumItems;
+ gAverageDupItems += mItems.size();
reportStats();
}
#endif
- for (size_t i = 0; i < mNumItems; ++i) {
+ for (size_t i = 0; i < mItems.size(); ++i) {
const Item *from = &mItems[i];
Item *to = &msg->mItems[i];
@@ -560,7 +573,7 @@
}
s.append(") = {\n");
- for (size_t i = 0; i < mNumItems; ++i) {
+ for (size_t i = 0; i < mItems.size(); ++i) {
const Item &item = mItems[i];
switch (item.mType) {
@@ -653,19 +666,20 @@
sp<AMessage> msg = new AMessage();
msg->setWhat(what);
- msg->mNumItems = static_cast<size_t>(parcel.readInt32());
- if (msg->mNumItems > kMaxNumItems) {
+ size_t numItems = static_cast<size_t>(parcel.readInt32());
+ if (numItems > kMaxNumItems) {
ALOGE("Too large number of items clipped.");
- msg->mNumItems = kMaxNumItems;
+ numItems = kMaxNumItems;
}
+ msg->mItems.resize(numItems);
- for (size_t i = 0; i < msg->mNumItems; ++i) {
+ for (size_t i = 0; i < msg->mItems.size(); ++i) {
Item *item = &msg->mItems[i];
const char *name = parcel.readCString();
if (name == NULL) {
ALOGE("Failed reading name for an item. Parsing aborted.");
- msg->mNumItems = i;
+ msg->mItems.resize(i);
break;
}
@@ -709,7 +723,7 @@
if (stringValue == NULL) {
ALOGE("Failed reading string value from a parcel. "
"Parsing aborted.");
- msg->mNumItems = i;
+ msg->mItems.resize(i);
continue;
// The loop will terminate subsequently.
} else {
@@ -754,11 +768,9 @@
void AMessage::writeToParcel(Parcel *parcel) const {
parcel->writeInt32(static_cast<int32_t>(mWhat));
- parcel->writeInt32(static_cast<int32_t>(mNumItems));
+ parcel->writeInt32(static_cast<int32_t>(mItems.size()));
- for (size_t i = 0; i < mNumItems; ++i) {
- const Item &item = mItems[i];
-
+ for (const Item &item : mItems) {
parcel->writeCString(item.mName);
parcel->writeInt32(static_cast<int32_t>(item.mType));
@@ -828,8 +840,7 @@
diff->setTarget(mHandler.promote());
}
- for (size_t i = 0; i < mNumItems; ++i) {
- const Item &item = mItems[i];
+ for (const Item &item : mItems) {
const Item *oitem = other->findItem(item.mName, item.mType);
switch (item.mType) {
case kTypeInt32:
@@ -936,11 +947,11 @@
}
size_t AMessage::countEntries() const {
- return mNumItems;
+ return mItems.size();
}
const char *AMessage::getEntryNameAt(size_t index, Type *type) const {
- if (index >= mNumItems) {
+ if (index >= mItems.size()) {
*type = kTypeInt32;
return NULL;
@@ -953,7 +964,7 @@
AMessage::ItemData AMessage::getEntryAt(size_t index) const {
ItemData it;
- if (index < mNumItems) {
+ if (index < mItems.size()) {
switch (mItems[index].mType) {
case kTypeInt32: it.set(mItems[index].u.int32Value); break;
case kTypeInt64: it.set(mItems[index].u.int64Value); break;
@@ -986,7 +997,7 @@
}
status_t AMessage::setEntryNameAt(size_t index, const char *name) {
- if (index >= mNumItems) {
+ if (index >= mItems.size()) {
return BAD_INDEX;
}
if (name == nullptr) {
@@ -996,7 +1007,7 @@
return OK; // name has not changed
}
size_t len = strlen(name);
- if (findItemIndex(name, len) < mNumItems) {
+ if (findItemIndex(name, len) < mItems.size()) {
return ALREADY_EXISTS;
}
delete[] mItems[index].mName;
@@ -1011,7 +1022,7 @@
sp<AMessage> msgValue;
sp<ABuffer> bufValue;
- if (index >= mNumItems) {
+ if (index >= mItems.size()) {
return BAD_INDEX;
}
if (!item.used()) {
@@ -1060,21 +1071,22 @@
}
status_t AMessage::removeEntryAt(size_t index) {
- if (index >= mNumItems) {
+ if (index >= mItems.size()) {
return BAD_INDEX;
}
// delete entry data and objects
- --mNumItems;
delete[] mItems[index].mName;
mItems[index].mName = nullptr;
freeItemValue(&mItems[index]);
// swap entry with last entry and clear last entry's data
- if (index < mNumItems) {
- mItems[index] = mItems[mNumItems];
- mItems[mNumItems].mName = nullptr;
- mItems[mNumItems].mType = kTypeInt32;
+ size_t lastIndex = mItems.size() - 1;
+ if (index < lastIndex) {
+ mItems[index] = mItems[lastIndex];
+ mItems[lastIndex].mName = nullptr;
+ mItems[lastIndex].mType = kTypeInt32;
}
+ mItems.pop_back();
return OK;
}
@@ -1083,7 +1095,7 @@
return BAD_VALUE;
}
size_t index = findEntryByName(name);
- if (index >= mNumItems) {
+ if (index >= mItems.size()) {
return BAD_INDEX;
}
return removeEntryAt(index);
@@ -1093,7 +1105,7 @@
if (item.used()) {
Item *it = allocateItem(name);
if (it != nullptr) {
- setEntryAt(it - mItems, item);
+ setEntryAt(it - &mItems[0], item);
}
}
}
@@ -1108,11 +1120,11 @@
return;
}
- for (size_t ix = 0; ix < other->mNumItems; ++ix) {
+ for (size_t ix = 0; ix < other->mItems.size(); ++ix) {
Item *it = allocateItem(other->mItems[ix].mName);
if (it != nullptr) {
ItemData data = other->getEntryAt(ix);
- setEntryAt(it - mItems, data);
+ setEntryAt(it - &mItems[0], data);
}
}
}
diff --git a/media/libstagefright/foundation/include/media/stagefright/foundation/AMessage.h b/media/libstagefright/foundation/include/media/stagefright/foundation/AMessage.h
index 98d6147..960212a 100644
--- a/media/libstagefright/foundation/include/media/stagefright/foundation/AMessage.h
+++ b/media/libstagefright/foundation/include/media/stagefright/foundation/AMessage.h
@@ -24,6 +24,8 @@
#include <utils/KeyedVector.h>
#include <utils/RefBase.h>
+#include <vector>
+
namespace android {
struct ABuffer;
@@ -95,6 +97,7 @@
void setTarget(const sp<const AHandler> &handler);
+ // removes all items
void clear();
void setInt32(const char *name, int32_t value);
@@ -302,16 +305,39 @@
size_t mNameLength;
Type mType;
void setName(const char *name, size_t len);
+ Item() : mName(nullptr), mNameLength(0), mType(kTypeInt32) { }
+ Item(const char *name, size_t length);
};
enum {
- kMaxNumItems = 64
+ kMaxNumItems = 256
};
- Item mItems[kMaxNumItems];
- size_t mNumItems;
+ std::vector<Item> mItems;
+ /**
+ * Allocates an item with the given key |name|. If the key already exists, the corresponding
+ * item value is freed. Otherwise a new item is added.
+ *
+ * This method currently asserts if the number of elements would exceed the max number of
+ * elements allowed (kMaxNumItems). This is a security precaution to avoid arbitrarily large
+ * AMessage structures.
+ *
+ * @todo(b/192153245) Either revisit this security precaution, or change the behavior to
+ * silently ignore keys added after the max number of elements are reached.
+ *
+ * @note All previously returned Item* pointers are deemed invalid after this call. (E.g. from
+ * allocateItem or findItem)
+ *
+ * @param name the key for the requested item.
+ *
+ * @return Item* a pointer to the item.
+ */
Item *allocateItem(const char *name);
+
+ /** Frees the value for the item. */
void freeItemValue(Item *item);
+
+ /** Finds an item with given key |name| and |type|. Returns nullptr if item is not found. */
const Item *findItem(const char *name, Type type) const;
void setObjectInternal(