This is an automated email from the ASF dual-hosted git repository. joemcdonnell pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 3d23a650fdabd780462e2e0b42cbac5230102528 Author: Daniel Becker <[email protected]> AuthorDate: Wed Apr 19 15:49:52 2023 +0200 IMPALA-12074: Add the WARN_UNUSED_RESULT attribute to methods of BitWriter that can fail Many methods of BitWriter signal failure via the return value. These return values should not be discarded because the failures are not detected then. This change adds the 'WARN_UNUSED_RESULT' attribute to these methods so the compiler can check that the return value is not discarded. Change-Id: I97ddba8057f71b6ebbed5b89b12d81c659ce98ae Reviewed-on: http://gerrit.cloudera.org:8080/19771 Reviewed-by: Csaba Ringhofer <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- be/src/exec/parquet/parquet-bool-decoder.cc | 5 ++++- be/src/util/bit-stream-utils-test.cc | 3 ++- be/src/util/bit-stream-utils.h | 19 ++++++++++--------- be/src/util/rle-encoding.h | 3 ++- be/src/util/rle-test.cc | 4 +++- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/be/src/exec/parquet/parquet-bool-decoder.cc b/be/src/exec/parquet/parquet-bool-decoder.cc index cff3254cc..02de18833 100644 --- a/be/src/exec/parquet/parquet-bool-decoder.cc +++ b/be/src/exec/parquet/parquet-bool-decoder.cc @@ -74,7 +74,10 @@ bool ParquetBoolDecoder::SkipValues(int num_values) { int num_remaining = num_values - skip_cached; if (encoding_ == parquet::Encoding::PLAIN) { int num_to_skip = BitUtil::RoundDownToPowerOf2(num_remaining, 32); - if (num_to_skip > 0) bool_values_.SkipBatch(1, num_to_skip); + if (num_to_skip > 0) { + bool skipped = bool_values_.SkipBatch(1, num_to_skip); + DCHECK(skipped); + } num_remaining -= num_to_skip; if (num_remaining > 0) { DCHECK_LE(num_remaining, UNPACKED_BUFFER_LEN); diff --git a/be/src/util/bit-stream-utils-test.cc b/be/src/util/bit-stream-utils-test.cc index 1407f51c8..c9b07de16 100644 --- a/be/src/util/bit-stream-utils-test.cc +++ b/be/src/util/bit-stream-utils-test.cc @@ -109,7 +109,8 @@ void TestSkipping(const uint8_t* buffer, const int len, const int bit_width, BatchedBitReader skipping_reader(buffer, len); T vals[MAX_LEN]; skipping_reader.UnpackBatch(bit_width, skip_at, vals); - skipping_reader.SkipBatch(bit_width, skip_count); + bool skipped = skipping_reader.SkipBatch(bit_width, skip_count); + ASSERT_TRUE(skipped); skipping_reader.UnpackBatch(bit_width, value_count - skip_count, vals + skip_at); for (int i = 0; i < skip_at; ++i) { diff --git a/be/src/util/bit-stream-utils.h b/be/src/util/bit-stream-utils.h index d0755ca59..50193b21e 100644 --- a/be/src/util/bit-stream-utils.h +++ b/be/src/util/bit-stream-utils.h @@ -54,26 +54,26 @@ class BitWriter { /// Writes a value to buffered_values_, flushing to buffer_ if necessary. This is bit /// packed. Returns false if there was not enough space. num_bits must be <= 64. - bool PutValue(uint64_t v, int num_bits); + WARN_UNUSED_RESULT bool PutValue(uint64_t v, int num_bits); /// Writes v to the next aligned byte using num_bytes. If T is larger than num_bytes, the /// extra high-order bytes will be ignored. Returns false if there was not enough space. template<typename T> - bool PutAligned(T v, int num_bytes); + WARN_UNUSED_RESULT bool PutAligned(T v, int num_bytes); /// Write an unsigned ULEB-128 encoded int to the buffer. Return false if there was not /// enough room. The value is written byte aligned. For more details on ULEB-128: /// https://en.wikipedia.org/wiki/LEB128 /// UINT_T must be an unsigned integer type. template<typename UINT_T> - bool PutUleb128(UINT_T v); + WARN_UNUSED_RESULT bool PutUleb128(UINT_T v); /// Write a ZigZag encoded int to the buffer. Return false if there was not enough /// room. The value is written byte aligned. For more details on ZigZag encoding: /// https://developers.google.com/protocol-buffers/docs/encoding#signed-integers /// INT_T must be a signed integer type. template<typename INT_T> - bool PutZigZagInteger(INT_T v); + WARN_UNUSED_RESULT bool PutZigZagInteger(INT_T v); /// Get a pointer to the next aligned byte and advance the underlying buffer /// by num_bytes. @@ -147,7 +147,8 @@ class BatchedBitReader { /// Skip 'num_values_to_skip' bit-packed values. /// 'num_values_to_skip * bit_width' is either divisible by 8, or /// 'num_values_to_skip' equals to the count of the remaining bit-packed values. - bool SkipBatch(int bit_width, int num_values_to_skip); + /// Returns false if there are not enough bytes left. + WARN_UNUSED_RESULT bool SkipBatch(int bit_width, int num_values_to_skip); /// Unpack bit-packed values in the same way as UnpackBatch() and decode them using the /// dictionary 'dict' with 'dict_len' entries. Return -1 if a decoding error is @@ -155,14 +156,14 @@ class BatchedBitReader { /// Otherwise returns the number of values decoded. The values are written to 'v' with /// a stride of 'stride' bytes. template <typename T> - int UnpackAndDecodeBatch( + WARN_UNUSED_RESULT int UnpackAndDecodeBatch( int bit_width, T* dict, int64_t dict_len, int num_values, T* v, int64_t stride); /// Reads an unpacked 'num_bytes'-sized value from the buffer and stores it in 'v'. T /// needs to be a little-endian native type and big enough to store 'num_bytes'. /// Returns false if there are not enough bytes left. template<typename T> - bool GetBytes(int num_bytes, T* v); + WARN_UNUSED_RESULT bool GetBytes(int num_bytes, T* v); /// Read an unsigned ULEB-128 encoded int from the stream. The encoded int must start /// at the beginning of a byte. Return false if there were not enough bytes in the @@ -170,7 +171,7 @@ class BatchedBitReader { /// https://en.wikipedia.org/wiki/LEB128 /// UINT_T must be an unsigned integer type. template<typename UINT_T> - bool GetUleb128(UINT_T* v); + WARN_UNUSED_RESULT bool GetUleb128(UINT_T* v); /// Read a ZigZag encoded int from the stream. The encoded int must start at the /// beginning of a byte. Return false if there were not enough bytes in the buffer or @@ -178,7 +179,7 @@ class BatchedBitReader { /// https://developers.google.com/protocol-buffers/docs/encoding#signed-integers /// INT_T must be a signed integer type. template<typename INT_T> - bool GetZigZagInteger(INT_T* v); + WARN_UNUSED_RESULT bool GetZigZagInteger(INT_T* v); /// Returns the number of bytes left in the stream. int bytes_left() { return buffer_end_ - buffer_pos_; } diff --git a/be/src/util/rle-encoding.h b/be/src/util/rle-encoding.h index 4ea3769f8..1fb8a607c 100644 --- a/be/src/util/rle-encoding.h +++ b/be/src/util/rle-encoding.h @@ -585,7 +585,8 @@ inline bool RleBatchDecoder<T>::SkipLiteralValues(int32_t num_literals_to_skip) int32_t num_to_skip = std::min<int32_t>(literal_count_, BitUtil::RoundDownToPowerOf2(num_remaining, 32)); if (num_to_skip > 0) { - bit_reader_.SkipBatch(bit_width_, num_to_skip); + bool skipped = bit_reader_.SkipBatch(bit_width_, num_to_skip); + DCHECK(skipped); literal_count_ -= num_to_skip; num_remaining -= num_to_skip; } diff --git a/be/src/util/rle-test.cc b/be/src/util/rle-test.cc index f74b8c357..d4534888b 100644 --- a/be/src/util/rle-test.cc +++ b/be/src/util/rle-test.cc @@ -623,7 +623,9 @@ TEST_F(RleTest, RepeatCountOverflow) { // are 1. const uint32_t REPEATED_RUN_HEADER = 0xfffffffe; const uint32_t LITERAL_RUN_HEADER = 0xffffffff; - writer.PutUleb128<uint32_t>(literal_run ? LITERAL_RUN_HEADER : REPEATED_RUN_HEADER); + bool success = writer.PutUleb128<uint32_t>(literal_run ? + LITERAL_RUN_HEADER : REPEATED_RUN_HEADER); + ASSERT_TRUE(success); writer.Flush(); RleBatchDecoder<uint64_t> decoder(buffer, BUFFER_LEN, 1);
