Revert "Reduce number of allocation when writing string vec"
This reverts commit 9f16bc66a0b41486a632e60a24c84a67f291f9b3.
Reason for revert: trusty_test_xts builds are broken during a test due to implicit conversion:
[ 67.594669][ T81] trusty-log trusty-ffa:trusty-core:log: UBSan: (implicit conversion) frameworks/native/libs/binder/include/binder/Parcel.h:1103:56
[ 67.595565][ T81] trusty-log trusty-ffa:trusty-core:log: Details: implicit conversion (sign change) from -4 to 18446744073709551612
Bug: 438750707
Change-Id: Ibbe72c431969c63552a180aa7d7b300ab839b0ae
diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h
index 7244689..7260d88 100644
--- a/libs/binder/include/binder/Parcel.h
+++ b/libs/binder/include/binder/Parcel.h
@@ -1073,8 +1073,8 @@
using T = first_template_type_t<CT>; // The T in CT == C<T, ...>
if (c.size() > static_cast<size_t>(std::numeric_limits<int32_t>::max())) return BAD_VALUE;
const auto size = static_cast<int32_t>(c.size());
+ writeData(size);
if constexpr (is_pointer_equivalent_array_v<T>) {
- writeData(size);
constexpr size_t limit = std::numeric_limits<size_t>::max() / sizeof(T);
if (c.size() > limit) return BAD_VALUE;
// is_pointer_equivalent types do not have gaps which could leak info,
@@ -1085,34 +1085,13 @@
return write(c.data(), c.size() * sizeof(T));
} else if constexpr (std::is_same_v<T, bool>
|| std::is_same_v<T, char16_t>) {
- writeData(size);
// reserve data space to write to
auto data = reinterpret_cast<int32_t*>(writeInplace(c.size() * sizeof(int32_t)));
if (data == nullptr) return BAD_VALUE;
for (const auto t: c) {
*data++ = static_cast<int32_t>(t);
}
- } else if constexpr (std::is_same_v<T, std::string>) {
- size_t totalSize = 4; // 4 bytes for the size of the vector
- for (const auto& t : c) {
- // String.length() * 2 because each char is at least one
- // UTF-16 code point (2 bytes)
- // + 2 for null char
- // The string + null is padded for 4 byte alignment
- // + 4 byte int32 for len
- totalSize += ((t.size() * 2 + 2 + 3) & ~3) + 4;
- }
- totalSize += mDataPos;
- if (dataCapacity() < totalSize) {
- setDataCapacity(totalSize / 2 * 3);
- }
- writeData(size);
- for (const auto& t : c) {
- const status_t status = writeData(t);
- if (status != OK) return status;
- }
} else /* constexpr */ {
- writeData(size);
for (const auto &t : c) {
const status_t status = writeData(t);
if (status != OK) return status;
diff --git a/libs/binder/tests/binderAllocationLimits.cpp b/libs/binder/tests/binderAllocationLimits.cpp
index a26c712..9da9f0a 100644
--- a/libs/binder/tests/binderAllocationLimits.cpp
+++ b/libs/binder/tests/binderAllocationLimits.cpp
@@ -283,22 +283,6 @@
<< "Expecting " << numStringAllocations << " string allocs";
}
-TEST(BinderAllocation, WriteListOfStrings) {
- sp<android::os::IServiceManager> sm =
- android::interface_cast<android::os::IServiceManager>(GetRemoteBinder());
- std::vector<std::string> ret;
- ASSERT_TRUE(sm->listServices(android::IServiceManager::DUMP_FLAG_PRIORITY_ALL, &ret).isOk());
- ASSERT_GT(ret.size(), 0u);
-
- size_t mallocs = 0;
- const auto on_malloc = OnMalloc([&](size_t /* bytes */) { mallocs++; });
-
- Parcel p;
- EXPECT_EQ(OK, p.writeUtf8VectorAsUtf16Vector(ret));
-
- EXPECT_EQ(mallocs, 1u);
-}
-
TEST(BinderAllocation, MakeScopeGuard) {
const auto m = ScopeDisallowMalloc();
{
