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)
Merged-In: I720699093d5c5a584283e5b76851938f449ffa21
Change-Id: I720699093d5c5a584283e5b76851938f449ffa21

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
+        // we can compact ourselves on the wire.
-        // to ensure our slots stay aligned
+        size_t slotsSize = sizeOfSlots();
-        size_t slotsSize = mSize - mSlotsOffset;
+        size_t compactedSize = sizeInUse();
-        size_t compactedSize = mAllocOffset + slotsSize;
-        compactedSize = (compactedSize + 3) & ~3;
         if (parcel->writeUint32(compactedSize)) goto fail;
         if (parcel->writeBool(false)) goto fail;
         void* dest = parcel->writeInplace(compactedSize);

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);
 

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