From 1ccb347e87a9647f9ecf026e4207e9617cf9aca3 Mon Sep 17 00:00:00 2001 From: Kelvin Zhang Date: Tue, 28 Nov 2023 11:13:55 -0800 Subject: [PATCH] Turn CowOperationType into an enum There's a bug previsouly where we compare return value of GetCowOpSourceInfoData() with CowOperationType. Such bugs are possible because cow operation enums are weakly typed integers. Turn CowOperationType into strongly typed enum to prevent such bugs. Test: th Bug: 304602386 Change-Id: If6941a4740c374ed066cf0aee9e52f4df05a9b38 --- .../include/libsnapshot/cow_format.h | 36 ++++++++++++------- .../libsnapshot_cow/cow_format.cpp | 2 +- .../libsnapshot/libsnapshot_cow/writer_v2.cpp | 2 +- .../libsnapshot/libsnapshot_cow/writer_v2.h | 2 +- .../libsnapshot/libsnapshot_cow/writer_v3.cpp | 2 +- .../libsnapshot/libsnapshot_cow/writer_v3.h | 2 +- .../dm-snapshot-merge/snapuserd_worker.cpp | 3 +- .../user-space-merge/read_worker.cpp | 3 +- 8 files changed, 33 insertions(+), 19 deletions(-) diff --git a/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h b/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h index 5e5546d44..debe87eb1 100644 --- a/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h +++ b/fs_mgr/libsnapshot/include/libsnapshot/cow_format.h @@ -16,6 +16,7 @@ #include +#include #include #include @@ -119,10 +120,30 @@ struct CowHeaderV3 : public CowHeader { uint32_t compression_algorithm; } __attribute__((packed)); +enum class CowOperationType : uint8_t { + kCowCopyOp = 1, + kCowReplaceOp = 2, + kCowZeroOp = 3, + kCowLabelOp = 4, + kCowClusterOp = 5, + kCowXorOp = 6, + kCowSequenceOp = 7, + kCowFooterOp = std::numeric_limits::max(), +}; + +static constexpr CowOperationType kCowCopyOp = CowOperationType::kCowCopyOp; +static constexpr CowOperationType kCowReplaceOp = CowOperationType::kCowReplaceOp; +static constexpr CowOperationType kCowZeroOp = CowOperationType::kCowZeroOp; +static constexpr CowOperationType kCowLabelOp = CowOperationType::kCowLabelOp; +static constexpr CowOperationType kCowClusterOp = CowOperationType::kCowClusterOp; +static constexpr CowOperationType kCowXorOp = CowOperationType::kCowXorOp; +static constexpr CowOperationType kCowSequenceOp = CowOperationType::kCowSequenceOp; +static constexpr CowOperationType kCowFooterOp = CowOperationType::kCowFooterOp; + // This structure is the same size of a normal Operation, but is repurposed for the footer. struct CowFooterOperation { // The operation code (always kCowFooterOp). - uint8_t type; + CowOperationType type; // If this operation reads from the data section of the COW, this contains // the compression type of that data (see constants below). @@ -141,7 +162,7 @@ struct CowFooterOperation { // V2 version of COW. On disk format for older devices struct CowOperationV2 { // The operation code (see the constants and structures below). - uint8_t type; + CowOperationType type; // If this operation reads from the data section of the COW, this contains // the compression type of that data (see constants below). @@ -176,7 +197,7 @@ struct CowOperationV2 { // The on disk format of cow (currently == CowOperation) struct CowOperationV3 { // The operation code (see the constants and structures below). - uint8_t type; + CowOperationType type; // If this operation reads from the data section of the COW, this contains // the length. @@ -201,15 +222,6 @@ struct CowOperationV3 { static_assert(sizeof(CowOperationV2) == sizeof(CowFooterOperation)); -static constexpr uint8_t kCowCopyOp = 1; -static constexpr uint8_t kCowReplaceOp = 2; -static constexpr uint8_t kCowZeroOp = 3; -static constexpr uint8_t kCowLabelOp = 4; -static constexpr uint8_t kCowClusterOp = 5; -static constexpr uint8_t kCowXorOp = 6; -static constexpr uint8_t kCowSequenceOp = 7; -static constexpr uint8_t kCowFooterOp = -1; - enum CowCompressionAlgorithm : uint8_t { kCowCompressNone = 0, kCowCompressGz = 1, diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/cow_format.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/cow_format.cpp index 937065d7d..4afd02685 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/cow_format.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/cow_format.cpp @@ -30,7 +30,7 @@ namespace snapshot { using android::base::unique_fd; -std::ostream& EmitCowTypeString(std::ostream& os, uint8_t cow_type) { +std::ostream& EmitCowTypeString(std::ostream& os, CowOperationType cow_type) { switch (cow_type) { case kCowCopyOp: return os << "kCowCopyOp"; diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v2.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v2.cpp index 37324c767..f9a4e479e 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v2.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v2.cpp @@ -369,7 +369,7 @@ bool CowWriterV2::CompressBlocks(size_t num_blocks, const void* data) { } bool CowWriterV2::EmitBlocks(uint64_t new_block_start, const void* data, size_t size, - uint64_t old_block, uint16_t offset, uint8_t type) { + uint64_t old_block, uint16_t offset, CowOperationType type) { CHECK(!merge_in_progress_); const uint8_t* iter = reinterpret_cast(data); diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v2.h b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v2.h index 24170ebbd..50e635ffd 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v2.h +++ b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v2.h @@ -42,7 +42,7 @@ class CowWriterV2 : public CowWriterBase { bool EmitCluster(); bool EmitClusterIfNeeded(); bool EmitBlocks(uint64_t new_block_start, const void* data, size_t size, uint64_t old_block, - uint16_t offset, uint8_t type); + uint16_t offset, CowOperationType type); void SetupHeaders(); void SetupWriteOptions(); bool ParseOptions(); diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp index b36c6f3cf..767f3d589 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp +++ b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.cpp @@ -230,7 +230,7 @@ bool CowWriterV3::EmitXorBlocks(uint32_t new_block_start, const void* data, size } bool CowWriterV3::EmitBlocks(uint64_t new_block_start, const void* data, size_t size, - uint64_t old_block, uint16_t offset, uint8_t type) { + uint64_t old_block, uint16_t offset, CowOperationType type) { const size_t num_blocks = (size / header_.block_size); for (size_t i = 0; i < num_blocks; i++) { const uint8_t* const iter = diff --git a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.h b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.h index 3dfc33cea..340218fd2 100644 --- a/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.h +++ b/fs_mgr/libsnapshot/libsnapshot_cow/writer_v3.h @@ -46,7 +46,7 @@ class CowWriterV3 : public CowWriterBase { bool OpenForAppend(uint64_t label); bool WriteOperation(const CowOperationV3& op, const void* data = nullptr, size_t size = 0); bool EmitBlocks(uint64_t new_block_start, const void* data, size_t size, uint64_t old_block, - uint16_t offset, uint8_t type); + uint16_t offset, CowOperationType type); bool CompressBlocks(size_t num_blocks, const void* data); private: diff --git a/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd_worker.cpp b/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd_worker.cpp index 571b35238..b24844df6 100644 --- a/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/dm-snapshot-merge/snapuserd_worker.cpp @@ -191,7 +191,8 @@ bool WorkerThread::ProcessCowOp(const CowOperation* cow_op) { } default: { - SNAP_LOG(ERROR) << "Unsupported operation-type found: " << cow_op->type; + SNAP_LOG(ERROR) << "Unsupported operation-type found: " + << static_cast(cow_op->type); } } return false; diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp index 5cb13e8bc..906316eec 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/read_worker.cpp @@ -183,7 +183,8 @@ bool ReadWorker::ProcessCowOp(const CowOperation* cow_op, void* buffer) { } default: { - SNAP_LOG(ERROR) << "Unknown operation-type found: " << cow_op->type; + SNAP_LOG(ERROR) << "Unknown operation-type found: " + << static_cast(cow_op->type); } } return false;