commit 12e05312241f9ca0a2d032f962ee6c306a1b1010
author Kelvin Zhang <zhangkelvin@google.com> Tue Dec 05 10:22:51 2023 -0800
committer Kelvin Zhang <zhangkelvin@google.com> Tue Dec 05 10:28:12 2023 -0800
tree 54311b7c091bbac79f8434c0cb38f79ebb3c9804
parent 7d526560df250f1636de75cff5d4ff6e0f3a8e3e

Fix multiple calls to set_[source/type]

Since currently implementation just does an bitwise or with the
storage unit, multiple calls to set_source would result in overlapping
bits. Fix by first clear the existing storage.

Test: th
Bug: 313962438
Change-Id: Iecfe8dd244c0f65ecd3cacb0404fdc39ef836d97

fs_mgr/libsnapshot/include/libsnapshot/cow_format.h

diff --git a/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h b/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h
index 717ad2d..6b34152 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h
@@ -226,6 +226,9 @@
     uint64_t source_info_;
     constexpr uint64_t source() const { return source_info_ & kCowOpSourceInfoDataMask; }
     constexpr void set_source(uint64_t source) {
+        // Clear the first 48 bit first
+        source_info_ &= ~kCowOpSourceInfoDataMask;
+        // Set the actual source field
         source_info_ |= source & kCowOpSourceInfoDataMask;
     }
     constexpr CowOperationType type() const {
@@ -234,6 +237,9 @@
         return static_cast<CowOperationType>(type);
     }
     constexpr void set_type(CowOperationType type) {
+        // Clear the top 4 bits first
+        source_info_ &= ((1ULL << kCowOpSourceInfoTypeBit) - 1);
+        // set the actual type bits
         source_info_ |= (static_cast<uint64_t>(type) & kCowOpSourceInfoTypeMask)
                         << kCowOpSourceInfoTypeBit;
     }

fs_mgr/libsnapshot/libsnapshot_cow/test_v3.cpp

diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/test_v3.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/test_v3.cpp
index ea34c27..aceed24 100644
--- a/fs_mgr/libsnapshot/libsnapshot_cow/test_v3.cpp
+++ b/fs_mgr/libsnapshot/libsnapshot_cow/test_v3.cpp
@@ -621,5 +621,42 @@
     ASSERT_EQ(expected_block, 0);
     ASSERT_TRUE(iter->AtEnd());
 }
+
+TEST_F(CowTestV3, SetSourceManyTimes) {
+    CowOperationV3 op{};
+    op.set_source(1);
+    ASSERT_EQ(op.source(), 1);
+    op.set_source(2);
+    ASSERT_EQ(op.source(), 2);
+    op.set_source(4);
+    ASSERT_EQ(op.source(), 4);
+    op.set_source(8);
+    ASSERT_EQ(op.source(), 8);
+}
+
+TEST_F(CowTestV3, SetTypeManyTimes) {
+    CowOperationV3 op{};
+    op.set_type(kCowCopyOp);
+    ASSERT_EQ(op.type(), kCowCopyOp);
+    op.set_type(kCowReplaceOp);
+    ASSERT_EQ(op.type(), kCowReplaceOp);
+    op.set_type(kCowZeroOp);
+    ASSERT_EQ(op.type(), kCowZeroOp);
+    op.set_type(kCowXorOp);
+    ASSERT_EQ(op.type(), kCowXorOp);
+}
+
+TEST_F(CowTestV3, SetTypeSourceInverleave) {
+    CowOperationV3 op{};
+    op.set_type(kCowCopyOp);
+    ASSERT_EQ(op.type(), kCowCopyOp);
+    op.set_source(0x010203040506);
+    ASSERT_EQ(op.source(), 0x010203040506);
+    ASSERT_EQ(op.type(), kCowCopyOp);
+    op.set_type(kCowReplaceOp);
+    ASSERT_EQ(op.source(), 0x010203040506);
+    ASSERT_EQ(op.type(), kCowReplaceOp);
+}
+
 }  // namespace snapshot
 }  // namespace android