diff options
author | Lee Shombert <shombert@google.com> | 2023-05-19 15:52:00 -0700 |
---|---|---|
committer | Android Build Coastguard Worker <android-build-coastguard-worker@google.com> | 2023-06-14 00:38:29 +0000 |
commit | cc113910a65067597c9e89755b718a05fd5bc8c1 (patch) | |
tree | 37765e063e485b840090db3cd9a8ad551e7127b2 | |
parent | 18a514f4ac9fa922e0e6f8e81ef7c88a72788dd4 (diff) | |
download | base-cc113910a65067597c9e89755b718a05fd5bc8c1.tar.gz |
Remove unnecessary padding code
Bug: 213170822
Remove the code that CursorWindow::writeToParcel() uses to ensure slot
data is 4-byte aligned. Because mAllocOffset and mSlotsOffset are
already 4-byte aligned, the alignment step here is unnecessary.
CursorWindow::spaceInUse() returns the total space used. The tests
verify that the total space used is always a multiple of 4 bytes.
Test: atest
* libandroidfw_tests
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5d4afa0986cbc440f458b4b8db05fd176ef3e6d2)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:c7e1590ea236576e5ae2ed1e1bcc14756c3363f2)
Merged-In: I720699093d5c5a584283e5b76851938f449ffa21
Change-Id: I720699093d5c5a584283e5b76851938f449ffa21
-rw-r--r-- | libs/androidfw/CursorWindow.cpp | 10 | ||||
-rw-r--r-- | libs/androidfw/include/androidfw/CursorWindow.h | 3 | ||||
-rw-r--r-- | libs/androidfw/tests/CursorWindow_test.cpp | 31 |
3 files changed, 35 insertions, 9 deletions
diff --git a/libs/androidfw/CursorWindow.cpp b/libs/androidfw/CursorWindow.cpp index 3527eeead1d5..2a6dc7b95c07 100644 --- a/libs/androidfw/CursorWindow.cpp +++ b/libs/androidfw/CursorWindow.cpp @@ -108,7 +108,7 @@ status_t CursorWindow::maybeInflate() { { // Migrate existing contents into new ashmem region - uint32_t slotsSize = mSize - mSlotsOffset; + uint32_t slotsSize = sizeOfSlots(); uint32_t newSlotsOffset = mInflatedSize - slotsSize; memcpy(static_cast<uint8_t*>(newData), static_cast<uint8_t*>(mData), mAllocOffset); @@ -216,11 +216,9 @@ status_t CursorWindow::writeToParcel(Parcel* parcel) { if (parcel->writeDupFileDescriptor(mAshmemFd)) goto fail; } else { // Since we know we're going to be read-only on the remote side, - // we can compact ourselves on the wire, with just enough padding - // to ensure our slots stay aligned - size_t slotsSize = mSize - mSlotsOffset; - size_t compactedSize = mAllocOffset + slotsSize; - compactedSize = (compactedSize + 3) & ~3; + // we can compact ourselves on the wire. + size_t slotsSize = sizeOfSlots(); + size_t compactedSize = sizeInUse(); if (parcel->writeUint32(compactedSize)) goto fail; if (parcel->writeBool(false)) goto fail; void* dest = parcel->writeInplace(compactedSize); diff --git a/libs/androidfw/include/androidfw/CursorWindow.h b/libs/androidfw/include/androidfw/CursorWindow.h index 6e55a9a0eb8b..9ec026a19c4c 100644 --- a/libs/androidfw/include/androidfw/CursorWindow.h +++ b/libs/androidfw/include/androidfw/CursorWindow.h @@ -90,6 +90,9 @@ public: inline uint32_t getNumRows() { return mNumRows; } inline uint32_t getNumColumns() { return mNumColumns; } + inline size_t sizeOfSlots() const { return mSize - mSlotsOffset; } + inline size_t sizeInUse() const { return mAllocOffset + sizeOfSlots(); } + status_t clear(); status_t setNumColumns(uint32_t numColumns); diff --git a/libs/androidfw/tests/CursorWindow_test.cpp b/libs/androidfw/tests/CursorWindow_test.cpp index 15be80c48192..9ac427b66cb3 100644 --- a/libs/androidfw/tests/CursorWindow_test.cpp +++ b/libs/androidfw/tests/CursorWindow_test.cpp @@ -20,9 +20,16 @@ #include "TestHelpers.h" +// Verify that the memory in use is a multiple of 4 bytes +#define ASSERT_ALIGNED(w) \ + ASSERT_EQ(((w)->sizeInUse() & 3), 0); \ + ASSERT_EQ(((w)->freeSpace() & 3), 0); \ + ASSERT_EQ(((w)->sizeOfSlots() & 3), 0) + #define CREATE_WINDOW_1K \ CursorWindow* w; \ - CursorWindow::create(String8("test"), 1 << 10, &w); + CursorWindow::create(String8("test"), 1 << 10, &w); \ + ASSERT_ALIGNED(w); #define CREATE_WINDOW_1K_3X3 \ CursorWindow* w; \ @@ -30,11 +37,13 @@ ASSERT_EQ(w->setNumColumns(3), OK); \ ASSERT_EQ(w->allocRow(), OK); \ ASSERT_EQ(w->allocRow(), OK); \ - ASSERT_EQ(w->allocRow(), OK); + ASSERT_EQ(w->allocRow(), OK); \ + ASSERT_ALIGNED(w); #define CREATE_WINDOW_2M \ CursorWindow* w; \ - CursorWindow::create(String8("test"), 1 << 21, &w); + CursorWindow::create(String8("test"), 1 << 21, &w); \ + ASSERT_ALIGNED(w); static constexpr const size_t kHalfInlineSize = 8192; static constexpr const size_t kGiantSize = 1048576; @@ -48,6 +57,7 @@ TEST(CursorWindowTest, Empty) { ASSERT_EQ(w->getNumColumns(), 0); ASSERT_EQ(w->size(), 1 << 10); ASSERT_EQ(w->freeSpace(), 1 << 10); + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, SetNumColumns) { @@ -59,6 +69,7 @@ TEST(CursorWindowTest, SetNumColumns) { ASSERT_NE(w->setNumColumns(5), OK); ASSERT_NE(w->setNumColumns(3), OK); ASSERT_EQ(w->getNumColumns(), 4); + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, SetNumColumnsAfterRow) { @@ -69,6 +80,7 @@ TEST(CursorWindowTest, SetNumColumnsAfterRow) { ASSERT_EQ(w->allocRow(), OK); ASSERT_NE(w->setNumColumns(4), OK); ASSERT_EQ(w->getNumColumns(), 0); + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, AllocRow) { @@ -82,14 +94,17 @@ TEST(CursorWindowTest, AllocRow) { ASSERT_EQ(w->allocRow(), OK); ASSERT_LT(w->freeSpace(), before); ASSERT_EQ(w->getNumRows(), 1); + ASSERT_ALIGNED(w); // Verify we can unwind ASSERT_EQ(w->freeLastRow(), OK); ASSERT_EQ(w->freeSpace(), before); ASSERT_EQ(w->getNumRows(), 0); + ASSERT_ALIGNED(w); // Can't unwind when no rows left ASSERT_NE(w->freeLastRow(), OK); + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, AllocRowBounds) { @@ -99,6 +114,7 @@ TEST(CursorWindowTest, AllocRowBounds) { ASSERT_EQ(w->setNumColumns(60), OK); ASSERT_EQ(w->allocRow(), OK); ASSERT_NE(w->allocRow(), OK); + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, StoreNull) { @@ -115,6 +131,7 @@ TEST(CursorWindowTest, StoreNull) { auto field = w->getFieldSlot(0, 0); ASSERT_EQ(w->getFieldSlotType(field), CursorWindow::FIELD_TYPE_NULL); } + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, StoreLong) { @@ -133,6 +150,7 @@ TEST(CursorWindowTest, StoreLong) { ASSERT_EQ(w->getFieldSlotType(field), CursorWindow::FIELD_TYPE_INTEGER); ASSERT_EQ(w->getFieldSlotValueLong(field), 0xcafe); } + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, StoreString) { @@ -154,6 +172,7 @@ TEST(CursorWindowTest, StoreString) { auto actual = w->getFieldSlotValueString(field, &size); ASSERT_EQ(std::string(actual), "cafe"); } + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, StoreBounds) { @@ -174,6 +193,7 @@ TEST(CursorWindowTest, StoreBounds) { ASSERT_EQ(w->getFieldSlot(-1, 0), nullptr); ASSERT_EQ(w->getFieldSlot(0, -1), nullptr); ASSERT_EQ(w->getFieldSlot(-1, -1), nullptr); + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, Inflate) { @@ -233,6 +253,7 @@ TEST(CursorWindowTest, Inflate) { ASSERT_NE(actual, buf); ASSERT_EQ(memcmp(buf, actual, kHalfInlineSize), 0); } + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, ParcelEmpty) { @@ -248,10 +269,12 @@ TEST(CursorWindowTest, ParcelEmpty) { ASSERT_EQ(w->getNumColumns(), 0); ASSERT_EQ(w->size(), 0); ASSERT_EQ(w->freeSpace(), 0); + ASSERT_ALIGNED(w); // We can't mutate the window after parceling ASSERT_NE(w->setNumColumns(4), OK); ASSERT_NE(w->allocRow(), OK); + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, ParcelSmall) { @@ -310,6 +333,7 @@ TEST(CursorWindowTest, ParcelSmall) { ASSERT_EQ(actualSize, 0); ASSERT_NE(actual, nullptr); } + ASSERT_ALIGNED(w); } TEST(CursorWindowTest, ParcelLarge) { @@ -362,6 +386,7 @@ TEST(CursorWindowTest, ParcelLarge) { ASSERT_EQ(actualSize, 0); ASSERT_NE(actual, nullptr); } + ASSERT_ALIGNED(w); } } // android |