From 05e7398fec0818aa311b649005747cf58dcba29d Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Mon, 5 Aug 2024 09:17:52 -0700 Subject: [PATCH] Add functions for handling short (`uint16_t`) values Cleaned up `#includes` PiperOrigin-RevId: 659576041 --- src/google/protobuf/io/coded_stream.cc | 36 +++++++-- src/google/protobuf/io/coded_stream.h | 44 +++++++++-- .../protobuf/io/coded_stream_unittest.cc | 78 +++++++++++++++++++ 3 files changed, 145 insertions(+), 13 deletions(-) diff --git a/src/google/protobuf/io/coded_stream.cc b/src/google/protobuf/io/coded_stream.cc index 23a692a0bc..f8d7b71a15 100644 --- a/src/google/protobuf/io/coded_stream.cc +++ b/src/google/protobuf/io/coded_stream.cc @@ -344,17 +344,36 @@ bool CodedInputStream::ReadCord(absl::Cord* output, int size) { } +bool CodedInputStream::ReadLittleEndian16Fallback(uint16_t* value) { + constexpr size_t kSize = sizeof(*value); + uint8_t bytes[kSize]; + + const uint8_t* ptr; + if (BufferSize() >= static_cast(kSize)) { + // Fast path: Enough bytes in the buffer to read directly. + ptr = buffer_; + Advance(kSize); + } else { + // Slow path: Had to read past the end of the buffer. + if (!ReadRaw(bytes, kSize)) return false; + ptr = bytes; + } + ReadLittleEndian16FromArray(ptr, value); + return true; +} + bool CodedInputStream::ReadLittleEndian32Fallback(uint32_t* value) { - uint8_t bytes[sizeof(*value)]; + constexpr size_t kSize = sizeof(*value); + uint8_t bytes[kSize]; const uint8_t* ptr; - if (BufferSize() >= static_cast(sizeof(*value))) { + if (BufferSize() >= static_cast(kSize)) { // Fast path: Enough bytes in the buffer to read directly. ptr = buffer_; - Advance(sizeof(*value)); + Advance(kSize); } else { // Slow path: Had to read past the end of the buffer. - if (!ReadRaw(bytes, sizeof(*value))) return false; + if (!ReadRaw(bytes, kSize)) return false; ptr = bytes; } ReadLittleEndian32FromArray(ptr, value); @@ -362,16 +381,17 @@ bool CodedInputStream::ReadLittleEndian32Fallback(uint32_t* value) { } bool CodedInputStream::ReadLittleEndian64Fallback(uint64_t* value) { - uint8_t bytes[sizeof(*value)]; + constexpr size_t kSize = sizeof(*value); + uint8_t bytes[kSize]; const uint8_t* ptr; - if (BufferSize() >= static_cast(sizeof(*value))) { + if (BufferSize() >= static_cast(kSize)) { // Fast path: Enough bytes in the buffer to read directly. ptr = buffer_; - Advance(sizeof(*value)); + Advance(kSize); } else { // Slow path: Had to read past the end of the buffer. - if (!ReadRaw(bytes, sizeof(*value))) return false; + if (!ReadRaw(bytes, kSize)) return false; ptr = bytes; } ReadLittleEndian64FromArray(ptr, value); diff --git a/src/google/protobuf/io/coded_stream.h b/src/google/protobuf/io/coded_stream.h index 3a40dedd80..dc03435b22 100644 --- a/src/google/protobuf/io/coded_stream.h +++ b/src/google/protobuf/io/coded_stream.h @@ -104,15 +104,13 @@ #pragma runtime_checks("c", off) #endif +#include "absl/log/absl_log.h" // Replace with vlog_is_on.h after Abseil LTS 20240722 -#include "google/protobuf/stubs/common.h" -#include "absl/base/attributes.h" #include "absl/log/absl_check.h" #include "absl/numeric/bits.h" #include "absl/strings/cord.h" #include "absl/strings/string_view.h" #include "google/protobuf/endian.h" -#include "google/protobuf/port.h" // Must be included last. #include "google/protobuf/port_def.inc" @@ -199,6 +197,8 @@ class PROTOBUF_EXPORT CodedInputStream { bool ReadCord(absl::Cord* output, int size); + // Read a 16-bit little-endian integer. + bool ReadLittleEndian16(uint16_t* value); // Read a 32-bit little-endian integer. bool ReadLittleEndian32(uint32_t* value); // Read a 64-bit little-endian integer. @@ -206,6 +206,9 @@ class PROTOBUF_EXPORT CodedInputStream { // These methods read from an externally provided buffer. The caller is // responsible for ensuring that the buffer has sufficient space. + // Read a 16-bit little-endian integer. + static const uint8_t* ReadLittleEndian16FromArray(const uint8_t* buffer, + uint16_t* value); // Read a 32-bit little-endian integer. static const uint8_t* ReadLittleEndian32FromArray(const uint8_t* buffer, uint32_t* value); @@ -592,6 +595,7 @@ class PROTOBUF_EXPORT CodedInputStream { bool ReadVarint32Slow(uint32_t* value); bool ReadVarint64Slow(uint64_t* value); int ReadVarintSizeAsIntSlow(); + bool ReadLittleEndian16Fallback(uint16_t* value); bool ReadLittleEndian32Fallback(uint32_t* value); bool ReadLittleEndian64Fallback(uint64_t* value); @@ -1127,19 +1131,26 @@ class PROTOBUF_EXPORT CodedOutputStream { static uint8_t* WriteCordToArray(const absl::Cord& cord, uint8_t* target); + // Write a 16-bit little-endian integer. + void WriteLittleEndian16(uint16_t value) { + cur_ = impl_.EnsureSpace(cur_); + SetCur(WriteLittleEndian16ToArray(value, Cur())); + } + // Like WriteLittleEndian16() but writing directly to the target array. + static uint8_t* WriteLittleEndian16ToArray(uint16_t value, uint8_t* target); // Write a 32-bit little-endian integer. void WriteLittleEndian32(uint32_t value) { cur_ = impl_.EnsureSpace(cur_); SetCur(WriteLittleEndian32ToArray(value, Cur())); } - // Like WriteLittleEndian32() but writing directly to the target array. + // Like WriteLittleEndian32() but writing directly to the target array. static uint8_t* WriteLittleEndian32ToArray(uint32_t value, uint8_t* target); // Write a 64-bit little-endian integer. void WriteLittleEndian64(uint64_t value) { cur_ = impl_.EnsureSpace(cur_); SetCur(WriteLittleEndian64ToArray(value, Cur())); } - // Like WriteLittleEndian64() but writing directly to the target array. + // Like WriteLittleEndian64() but writing directly to the target array. static uint8_t* WriteLittleEndian64ToArray(uint64_t value, uint8_t* target); // Write an unsigned integer with Varint encoding. Writing a 32-bit value @@ -1321,6 +1332,13 @@ inline bool CodedInputStream::ReadVarintSizeAsInt(int* value) { return *value >= 0; } +// static +inline const uint8_t* CodedInputStream::ReadLittleEndian16FromArray( + const uint8_t* buffer, uint16_t* value) { + memcpy(value, buffer, sizeof(*value)); + *value = google::protobuf::internal::little_endian::ToHost(*value); + return buffer + sizeof(*value); +} // static inline const uint8_t* CodedInputStream::ReadLittleEndian32FromArray( const uint8_t* buffer, uint32_t* value) { @@ -1336,6 +1354,15 @@ inline const uint8_t* CodedInputStream::ReadLittleEndian64FromArray( return buffer + sizeof(*value); } +inline bool CodedInputStream::ReadLittleEndian16(uint16_t* value) { + if (PROTOBUF_PREDICT_TRUE(BufferSize() >= static_cast(sizeof(*value)))) { + buffer_ = ReadLittleEndian16FromArray(buffer_, value); + return true; + } else { + return ReadLittleEndian16Fallback(value); + } +} + inline bool CodedInputStream::ReadLittleEndian32(uint32_t* value) { #if defined(ABSL_IS_LITTLE_ENDIAN) && \ !defined(PROTOBUF_DISABLE_LITTLE_ENDIAN_OPT_FOR_TEST) @@ -1629,6 +1656,13 @@ inline uint8_t* CodedOutputStream::WriteVarint32SignExtendedToArray( return WriteVarint64ToArray(static_cast(value), target); } +inline uint8_t* CodedOutputStream::WriteLittleEndian16ToArray(uint16_t value, + uint8_t* target) { + uint16_t little_endian_value = google::protobuf::internal::little_endian::ToHost(value); + memcpy(target, &little_endian_value, sizeof(value)); + return target + sizeof(value); +} + inline uint8_t* CodedOutputStream::WriteLittleEndian32ToArray(uint32_t value, uint8_t* target) { #if defined(ABSL_IS_LITTLE_ENDIAN) && \ diff --git a/src/google/protobuf/io/coded_stream_unittest.cc b/src/google/protobuf/io/coded_stream_unittest.cc index 295bcbf1b9..dea7b149a8 100644 --- a/src/google/protobuf/io/coded_stream_unittest.cc +++ b/src/google/protobuf/io/coded_stream_unittest.cc @@ -513,6 +513,11 @@ TEST_F(CodedStreamTest, VarintSize64PowersOfTwo) { // ------------------------------------------------------------------- // Fixed-size int tests +struct Fixed16Case { + uint8_t bytes[sizeof(uint16_t)]; // Encoded bytes. + uint32_t value; // Parsed value. +}; + struct Fixed32Case { uint8_t bytes[sizeof(uint32_t)]; // Encoded bytes. uint32_t value; // Parsed value. @@ -523,6 +528,13 @@ struct Fixed64Case { uint64_t value; // Parsed value. }; +class Fixed16Cases : public CodedStreamTest, + public testing::WithParamInterface {}; + +class Fixed16CasesWithSizes + : public CodedStreamTest, + public testing::WithParamInterface> {}; + class Fixed32Cases : public CodedStreamTest, public testing::WithParamInterface {}; @@ -545,6 +557,11 @@ inline std::ostream& operator<<(std::ostream& os, const Fixed64Case& c) { return os << "0x" << std::hex << c.value << std::dec; } +Fixed16Case kFixed16Cases[] = { + {{0xef, 0xcd}, 0xcdefu}, + {{0x12, 0x34}, 0x3412u}, +}; + Fixed32Case kFixed32Cases[] = { {{0xef, 0xcd, 0xab, 0x90}, 0x90abcdefu}, {{0x12, 0x34, 0x56, 0x78}, 0x78563412u}, @@ -557,6 +574,23 @@ Fixed64Case kFixed64Cases[] = { uint64_t{0x8877665544332211u}}, }; +TEST_P(Fixed16CasesWithSizes, ReadLittleEndian16) { + Fixed16Case kFixed16Cases_case = std::get<0>(GetParam()); + int kBlockSizes_case = std::get<1>(GetParam()); + memcpy(buffer_, kFixed16Cases_case.bytes, sizeof(kFixed16Cases_case.bytes)); + ArrayInputStream input(buffer_, sizeof(buffer_), kBlockSizes_case); + + { + CodedInputStream coded_input(&input); + + uint16_t value; + EXPECT_TRUE(coded_input.ReadLittleEndian16(&value)); + EXPECT_EQ(kFixed16Cases_case.value, value); + } + + EXPECT_EQ(sizeof(uint16_t), input.ByteCount()); +} + TEST_P(Fixed32CasesWithSizes, ReadLittleEndian32) { Fixed32Case kFixed32Cases_case = std::get<0>(GetParam()); int kBlockSizes_case = std::get<1>(GetParam()); @@ -591,6 +625,24 @@ TEST_P(Fixed64CasesWithSizes, ReadLittleEndian64) { EXPECT_EQ(sizeof(uint64_t), input.ByteCount()); } +TEST_P(Fixed16CasesWithSizes, WriteLittleEndian16) { + Fixed16Case kFixed16Cases_case = std::get<0>(GetParam()); + int kBlockSizes_case = std::get<1>(GetParam()); + ArrayOutputStream output(buffer_, sizeof(buffer_), kBlockSizes_case); + + { + CodedOutputStream coded_output(&output); + + coded_output.WriteLittleEndian16(kFixed16Cases_case.value); + EXPECT_FALSE(coded_output.HadError()); + + EXPECT_EQ(sizeof(uint16_t), coded_output.ByteCount()); + } + + EXPECT_EQ(sizeof(uint16_t), output.ByteCount()); + EXPECT_EQ(0, memcmp(buffer_, kFixed16Cases_case.bytes, sizeof(uint16_t))); +} + TEST_P(Fixed32CasesWithSizes, WriteLittleEndian32) { Fixed32Case kFixed32Cases_case = std::get<0>(GetParam()); int kBlockSizes_case = std::get<1>(GetParam()); @@ -629,6 +681,17 @@ TEST_P(Fixed64CasesWithSizes, WriteLittleEndian64) { // Tests using the static methods to read fixed-size values from raw arrays. +TEST_P(Fixed16Cases, ReadLittleEndian16FromArray) { + Fixed16Case kFixed16Cases_case = GetParam(); + memcpy(buffer_, kFixed16Cases_case.bytes, sizeof(kFixed16Cases_case.bytes)); + + uint16_t value; + const uint8_t* end = + CodedInputStream::ReadLittleEndian16FromArray(buffer_, &value); + EXPECT_EQ(kFixed16Cases_case.value, value); + EXPECT_TRUE(end == buffer_ + sizeof(value)); +} + TEST_P(Fixed32Cases, ReadLittleEndian32FromArray) { Fixed32Case kFixed32Cases_case = GetParam(); memcpy(buffer_, kFixed32Cases_case.bytes, sizeof(kFixed32Cases_case.bytes)); @@ -1599,6 +1662,11 @@ INSTANTIATE_TEST_SUITE_P( std::abs(std::get<0>(param_info.param)), "_BlockSize_", std::get<1>(param_info.param)); }); +INSTANTIATE_TEST_SUITE_P( + CodedStreamUnitTest, Fixed16Cases, testing::ValuesIn(kFixed16Cases), + [](const testing::TestParamInfo& param_info) { + return absl::StrCat("Fixed16Case_Value_", param_info.param.value); + }); INSTANTIATE_TEST_SUITE_P( CodedStreamUnitTest, Fixed32Cases, testing::ValuesIn(kFixed32Cases), [](const testing::TestParamInfo& param_info) { @@ -1609,6 +1677,16 @@ INSTANTIATE_TEST_SUITE_P( [](const testing::TestParamInfo& param_info) { return absl::StrCat("Fixed64Case_Value_", param_info.param.value); }); +INSTANTIATE_TEST_SUITE_P( + CodedStreamUnitTest, Fixed16CasesWithSizes, + testing::Combine(testing::ValuesIn(kFixed16Cases), + testing::ValuesIn(kBlockSizes)), + [](const testing::TestParamInfo& + param_info) { + return absl::StrCat("Fixed16Case_Value_", + std::get<0>(param_info.param).value, "_BlockSize_", + std::get<1>(param_info.param)); + }); INSTANTIATE_TEST_SUITE_P( CodedStreamUnitTest, Fixed32CasesWithSizes, testing::Combine(testing::ValuesIn(kFixed32Cases),