summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Derek Mauro <dmauro@google.com>2022-11-29 09:19:04 -0800
committerGravatar Copybara-Service <copybara-worker@google.com>2022-11-29 09:19:57 -0800
commitd03cced7d552ece168c6db92acb3a7c379aae0c0 (patch)
treef5975b279620e410ad017ee09bceaee4c676fec1
parent82196f059f213c50738142a799bb166b2971950d (diff)
CRC: Make crc32c_t as a class for explicit control of operators
The motivation is to explicitly remove and document dangerous operations like adding crc32c_t to a set, because equality is not enough to guarantee uniqueness. PiperOrigin-RevId: 491656425 Change-Id: I7b4dadc1a59ea9861e6ec7a929d64b5746467832
-rw-r--r--absl/crc/crc32c.cc11
-rw-r--r--absl/crc/crc32c.h46
-rw-r--r--absl/crc/crc32c_benchmark.cc8
-rw-r--r--absl/crc/crc32c_test.cc22
-rw-r--r--absl/crc/internal/crc_memcpy.h4
-rw-r--r--absl/crc/internal/crc_memcpy_test.cc4
6 files changed, 52 insertions, 43 deletions
diff --git a/absl/crc/crc32c.cc b/absl/crc/crc32c.cc
index 82865df5..169826f5 100644
--- a/absl/crc/crc32c.cc
+++ b/absl/crc/crc32c.cc
@@ -55,7 +55,7 @@ crc32c_t ExtendCrc32cInternal(crc32c_t initial_crc,
} // namespace crc_internal
crc32c_t ComputeCrc32c(absl::string_view buf) {
- return ExtendCrc32c(ToCrc32c(0), buf);
+ return ExtendCrc32c(crc32c_t{0}, buf);
}
crc32c_t ExtendCrc32cByZeroes(crc32c_t initial_crc, size_t length) {
@@ -67,7 +67,7 @@ crc32c_t ExtendCrc32cByZeroes(crc32c_t initial_crc, size_t length) {
crc32c_t ConcatCrc32c(crc32c_t lhs_crc, crc32c_t rhs_crc, size_t rhs_len) {
uint32_t result = static_cast<uint32_t>(lhs_crc);
CrcEngine()->ExtendByZeroes(&result, rhs_len);
- return static_cast<crc32c_t>(result) ^ rhs_crc;
+ return crc32c_t{result ^ static_cast<uint32_t>(rhs_crc)};
}
crc32c_t RemoveCrc32cPrefix(crc32c_t crc_a, crc32c_t crc_ab, size_t length_b) {
@@ -89,9 +89,10 @@ crc32c_t MemcpyCrc32c(void* dest, const void* src, size_t count,
// This operation has a runtime cost of O(log(`suffix_len`))
crc32c_t RemoveCrc32cSuffix(crc32c_t full_string_crc, crc32c_t suffix_crc,
size_t suffix_len) {
- crc32c_t crc_with_suffix_zeroed =
- suffix_crc ^ full_string_crc ^
- ExtendCrc32cByZeroes(ToCrc32c(0), suffix_len);
+ crc32c_t crc_with_suffix_zeroed = crc32c_t{
+ static_cast<uint32_t>(suffix_crc) ^
+ static_cast<uint32_t>(full_string_crc) ^
+ static_cast<uint32_t>(ExtendCrc32cByZeroes(crc32c_t{0}, suffix_len))};
return crc_internal::UnextendCrc32cByZeroes(
crc_with_suffix_zeroed, suffix_len);
}
diff --git a/absl/crc/crc32c.h b/absl/crc/crc32c.h
index 8b030732..7be42be5 100644
--- a/absl/crc/crc32c.h
+++ b/absl/crc/crc32c.h
@@ -39,24 +39,32 @@ ABSL_NAMESPACE_BEGIN
// crc32c_t
//-----------------------------------------------------------------------------
-// `crc32c_t` defines a strongly typed integer type for holding a CRC32C value.
-enum class crc32c_t : uint32_t {};
-
-// ToCrc32c()
-//
-// Converts a uint32_t value to crc32c_t. This API is necessary in C++14
-// and earlier. Code targeting C++17-or-later can instead use `crc32c_t{n}`.
-inline crc32c_t ToCrc32c(uint32_t n) {
- return static_cast<crc32c_t>(n);
-}
-// operator^
+// `crc32c_t` defines a strongly-typed integer for holding a CRC32C value.
//
-// Performs a bitwise XOR on two CRC32C values
-inline crc32c_t operator^(crc32c_t lhs, crc32c_t rhs) {
- const auto lhs_int = static_cast<uint32_t>(lhs);
- const auto rhs_int = static_cast<uint32_t>(rhs);
- return ToCrc32c(lhs_int ^ rhs_int);
-}
+// Some operators are intentionally omitted. Only equality operators are defined
+// so that `crc32c_t` can be directly compared. Methods for putting `crc32c_t`
+// directly into a set are omitted because this is bug-prone due to checksum
+// collisions. Use an explicit conversion to the `uint32_t` space for operations
+// that treat `crc32c_t` as an integer.
+class crc32c_t final {
+ public:
+ crc32c_t() = default;
+ constexpr explicit crc32c_t(uint32_t crc) : crc_(crc) {}
+
+ crc32c_t(const crc32c_t&) = default;
+ crc32c_t& operator=(const crc32c_t&) = default;
+
+ explicit operator uint32_t() const { return crc_; }
+
+ friend bool operator==(crc32c_t lhs, crc32c_t rhs) {
+ return static_cast<uint32_t>(lhs) == static_cast<uint32_t>(rhs);
+ }
+
+ friend bool operator!=(crc32c_t lhs, crc32c_t rhs) { return !(lhs == rhs); }
+
+ private:
+ uint32_t crc_;
+};
namespace crc_internal {
// Non-inline code path for `absl::ExtendCrc32c()`. Do not call directly.
@@ -92,7 +100,7 @@ inline crc32c_t ExtendCrc32c(crc32c_t initial_crc,
uint32_t crc = static_cast<uint32_t>(initial_crc);
if (crc_internal::ExtendCrc32cInline(&crc, buf_to_add.data(),
buf_to_add.size())) {
- return ToCrc32c(crc);
+ return crc32c_t{crc};
}
}
return crc_internal::ExtendCrc32cInternal(initial_crc, buf_to_add);
@@ -116,7 +124,7 @@ crc32c_t ExtendCrc32cByZeroes(crc32c_t initial_crc, size_t length);
// Using `MemcpyCrc32c()` is potentially faster than performing the `memcpy()`
// and `ComputeCrc32c()` operations separately.
crc32c_t MemcpyCrc32c(void* dest, const void* src, size_t count,
- crc32c_t initial_crc = ToCrc32c(0));
+ crc32c_t initial_crc = crc32c_t{0});
// -----------------------------------------------------------------------------
// CRC32C Arithmetic Functions
diff --git a/absl/crc/crc32c_benchmark.cc b/absl/crc/crc32c_benchmark.cc
index 2c7ac594..df99d5cf 100644
--- a/absl/crc/crc32c_benchmark.cc
+++ b/absl/crc/crc32c_benchmark.cc
@@ -44,7 +44,7 @@ BENCHMARK(BM_Calculate)->Arg(0)->Arg(1)->Arg(100)->Arg(10000)->Arg(500000);
void BM_Extend(benchmark::State& state) {
int len = state.range(0);
std::string extension = TestString(len);
- absl::crc32c_t base = absl::ToCrc32c(0xC99465AA); // CRC32C of "Hello World"
+ absl::crc32c_t base = absl::crc32c_t{0xC99465AA}; // CRC32C of "Hello World"
for (auto s : state) {
benchmark::DoNotOptimize(base);
benchmark::DoNotOptimize(extension);
@@ -55,7 +55,7 @@ void BM_Extend(benchmark::State& state) {
BENCHMARK(BM_Extend)->Arg(0)->Arg(1)->Arg(100)->Arg(10000)->Arg(500000);
void BM_ExtendByZeroes(benchmark::State& state) {
- absl::crc32c_t base = absl::ToCrc32c(0xC99465AA); // CRC32C of "Hello World"
+ absl::crc32c_t base = absl::crc32c_t{0xC99465AA}; // CRC32C of "Hello World"
int num_zeroes = state.range(0);
for (auto s : state) {
benchmark::DoNotOptimize(base);
@@ -70,7 +70,7 @@ BENCHMARK(BM_ExtendByZeroes)
->Range(1, 1 << 20);
void BM_UnextendByZeroes(benchmark::State& state) {
- absl::crc32c_t base = absl::ToCrc32c(0xdeadbeef);
+ absl::crc32c_t base = absl::crc32c_t{0xdeadbeef};
int num_zeroes = state.range(0);
for (auto s : state) {
benchmark::DoNotOptimize(base);
@@ -90,7 +90,7 @@ void BM_Concat(benchmark::State& state) {
std::string string_b = TestString(string_b_len);
// CRC32C of "Hello World"
- absl::crc32c_t crc_a = absl::ToCrc32c(0xC99465AA);
+ absl::crc32c_t crc_a = absl::crc32c_t{0xC99465AA};
absl::crc32c_t crc_b = absl::ComputeCrc32c(string_b);
for (auto s : state) {
diff --git a/absl/crc/crc32c_test.cc b/absl/crc/crc32c_test.cc
index 98e5fea3..0b9dc683 100644
--- a/absl/crc/crc32c_test.cc
+++ b/absl/crc/crc32c_test.cc
@@ -34,22 +34,22 @@ TEST(CRC32C, RFC3720) {
// 32 bytes of ones.
memset(data, 0, sizeof(data));
EXPECT_EQ(absl::ComputeCrc32c(absl::string_view(data, sizeof(data))),
- absl::ToCrc32c(0x8a9136aa));
+ absl::crc32c_t{0x8a9136aa});
// 32 bytes of ones.
memset(data, 0xff, sizeof(data));
EXPECT_EQ(absl::ComputeCrc32c(absl::string_view(data, sizeof(data))),
- absl::ToCrc32c(0x62a8ab43));
+ absl::crc32c_t{0x62a8ab43});
// 32 incrementing bytes.
for (int i = 0; i < 32; ++i) data[i] = static_cast<char>(i);
EXPECT_EQ(absl::ComputeCrc32c(absl::string_view(data, sizeof(data))),
- absl::ToCrc32c(0x46dd794e));
+ absl::crc32c_t{0x46dd794e});
// 32 decrementing bytes.
for (int i = 0; i < 32; ++i) data[i] = static_cast<char>(31 - i);
EXPECT_EQ(absl::ComputeCrc32c(absl::string_view(data, sizeof(data))),
- absl::ToCrc32c(0x113fdb5c));
+ absl::crc32c_t{0x113fdb5c});
// An iSCSI - SCSI Read (10) Command PDU.
constexpr uint8_t cmd[48] = {
@@ -60,7 +60,7 @@ TEST(CRC32C, RFC3720) {
};
EXPECT_EQ(absl::ComputeCrc32c(absl::string_view(
reinterpret_cast<const char*>(cmd), sizeof(cmd))),
- absl::ToCrc32c(0xd9963a56));
+ absl::crc32c_t{0xd9963a56});
}
std::string TestString(size_t len) {
@@ -73,8 +73,8 @@ std::string TestString(size_t len) {
}
TEST(CRC32C, Compute) {
- EXPECT_EQ(absl::ComputeCrc32c(""), absl::ToCrc32c(0));
- EXPECT_EQ(absl::ComputeCrc32c("hello world"), absl::ToCrc32c(0xc99465aa));
+ EXPECT_EQ(absl::ComputeCrc32c(""), absl::crc32c_t{0});
+ EXPECT_EQ(absl::ComputeCrc32c("hello world"), absl::crc32c_t{0xc99465aa});
}
TEST(CRC32C, Extend) {
@@ -82,13 +82,13 @@ TEST(CRC32C, Extend) {
std::string extension = "Extension String";
EXPECT_EQ(
- absl::ExtendCrc32c(absl::ToCrc32c(base), extension),
- absl::ToCrc32c(0xD2F65090)); // CRC32C of "Hello WorldExtension String"
+ absl::ExtendCrc32c(absl::crc32c_t{base}, extension),
+ absl::crc32c_t{0xD2F65090}); // CRC32C of "Hello WorldExtension String"
}
TEST(CRC32C, ExtendByZeroes) {
std::string base = "hello world";
- absl::crc32c_t base_crc = absl::ToCrc32c(0xc99465aa);
+ absl::crc32c_t base_crc = absl::crc32c_t{0xc99465aa};
for (const size_t extend_by : {100, 10000, 100000}) {
SCOPED_TRACE(extend_by);
@@ -98,7 +98,7 @@ TEST(CRC32C, ExtendByZeroes) {
}
TEST(CRC32C, UnextendByZeroes) {
- for (auto seed_crc : {absl::ToCrc32c(0), absl::ToCrc32c(0xc99465aa)}) {
+ for (auto seed_crc : {absl::crc32c_t{0}, absl::crc32c_t{0xc99465aa}}) {
SCOPED_TRACE(seed_crc);
for (const size_t size_1 : {2, 200, 20000, 200000, 20000000}) {
for (const size_t size_2 : {0, 100, 10000, 100000, 10000000}) {
diff --git a/absl/crc/internal/crc_memcpy.h b/absl/crc/internal/crc_memcpy.h
index 8e728a6e..ae9cccad 100644
--- a/absl/crc/internal/crc_memcpy.h
+++ b/absl/crc/internal/crc_memcpy.h
@@ -40,7 +40,7 @@ class CrcMemcpy {
public:
static crc32c_t CrcAndCopy(void* __restrict dst, const void* __restrict src,
std::size_t length,
- crc32c_t initial_crc = ToCrc32c(0),
+ crc32c_t initial_crc = crc32c_t{0},
bool non_temporal = false) {
static const ArchSpecificEngines engines = GetArchSpecificEngines();
auto* engine = non_temporal ? engines.non_temporal : engines.temporal;
@@ -100,7 +100,7 @@ class CrcNonTemporalMemcpyAVXEngine : public CrcMemcpyEngine {
// the generic fallback version.
inline crc32c_t Crc32CAndCopy(void* __restrict dst, const void* __restrict src,
std::size_t length,
- crc32c_t initial_crc = ToCrc32c(0),
+ crc32c_t initial_crc = crc32c_t{0},
bool non_temporal = false) {
return CrcMemcpy::CrcAndCopy(dst, src, length, initial_crc, non_temporal);
}
diff --git a/absl/crc/internal/crc_memcpy_test.cc b/absl/crc/internal/crc_memcpy_test.cc
index 708e8666..bbdcd205 100644
--- a/absl/crc/internal/crc_memcpy_test.cc
+++ b/absl/crc/internal/crc_memcpy_test.cc
@@ -108,7 +108,7 @@ TEST_P(x86ParamTest, SmallCorrectnessCheckSourceAlignment) {
static_cast<char>(absl::Uniform<unsigned char>(gen_));
}
absl::crc32c_t initial_crc =
- absl::ToCrc32c(absl::Uniform<uint32_t>(gen_));
+ absl::crc32c_t{absl::Uniform<uint32_t>(gen_)};
absl::crc32c_t experiment_crc =
engine_->Compute(destination_.get(), source_.get() + source_alignment,
size, initial_crc);
@@ -139,7 +139,7 @@ TEST_P(x86ParamTest, SmallCorrectnessCheckDestAlignment) {
static_cast<char>(absl::Uniform<unsigned char>(gen_));
}
absl::crc32c_t initial_crc =
- absl::ToCrc32c(absl::Uniform<uint32_t>(gen_));
+ absl::crc32c_t{absl::Uniform<uint32_t>(gen_)};
absl::crc32c_t experiment_crc =
engine_->Compute(destination_.get() + dest_alignment, source_.get(),
size, initial_crc);