From c460d64f51e34a56b2591c7114b2b302ff6576eb Mon Sep 17 00:00:00 2001 From: Gerrit Birkeland Date: Tue, 16 Jul 2024 00:46:20 -0600 Subject: [PATCH] [AVRO-4019] [C++] Turn on even more compiler warnings (#2966) * [C++] Turn on -Wuseless-cast and -Wconversion * Print versions of compiler/tools used in CI * Fix another compiler warning Also attempt to make build.sh continue * Fix the last of the compiler warnings * Update CodecTests.cc * Update DataFileTests.cc * Fix conversion warning on ARM64 * Hopefully make GCC 9.4 happy * [C++] Address review comments --- .github/workflows/test-lang-c++.yml | 6 ++++ lang/c++/CMakeLists.txt | 2 +- lang/c++/build.sh | 2 +- lang/c++/impl/Compiler.cc | 9 +++--- lang/c++/impl/DataFile.cc | 13 ++++----- lang/c++/impl/FileStream.cc | 2 +- lang/c++/impl/LogicalType.cc | 4 +-- lang/c++/impl/Node.cc | 2 +- lang/c++/impl/NodeImpl.cc | 12 ++++---- lang/c++/impl/Resolver.cc | 12 ++++---- lang/c++/impl/Validator.cc | 12 ++++---- lang/c++/impl/Zigzag.cc | 8 ++--- lang/c++/impl/json/JsonIO.cc | 22 +++++++------- lang/c++/impl/json/JsonIO.hh | 6 ++-- lang/c++/impl/parsing/ResolvingDecoder.cc | 29 ++++++++++--------- lang/c++/impl/parsing/Symbol.cc | 6 ++-- lang/c++/include/avro/LogicalType.hh | 12 ++++---- lang/c++/include/avro/NodeImpl.hh | 4 +-- lang/c++/include/avro/Reader.hh | 8 ++--- lang/c++/include/avro/Validator.hh | 16 +++++----- lang/c++/include/avro/buffer/Buffer.hh | 10 +++---- lang/c++/include/avro/buffer/BufferPrint.hh | 2 +- .../include/avro/buffer/BufferStreambuf.hh | 6 +++- .../avro/buffer/detail/BufferDetail.hh | 4 +-- lang/c++/test/CodecTests.cc | 6 ++-- lang/c++/test/DataFileTests.cc | 12 ++++---- lang/c++/test/StreamTests.cc | 6 ++-- lang/c++/test/buffertest.cc | 21 +++++++------- lang/c++/test/unittest.cc | 8 ++--- 29 files changed, 136 insertions(+), 126 deletions(-) diff --git a/.github/workflows/test-lang-c++.yml b/.github/workflows/test-lang-c++.yml index 3035aa66c91..61afa7ff61c 100644 --- a/.github/workflows/test-lang-c++.yml +++ b/.github/workflows/test-lang-c++.yml @@ -41,6 +41,12 @@ jobs: - name: Install Dependencies run: sudo apt update && sudo apt-get install -qqy cppcheck libboost-all-dev libsnappy-dev libfmt-dev cmake + - name: Print Versions + run: | + gcc --version + cmake --version + cppcheck --version + - name: Clean run: ./build.sh clean diff --git a/lang/c++/CMakeLists.txt b/lang/c++/CMakeLists.txt index 197d8f9856f..19059a41b13 100644 --- a/lang/c++/CMakeLists.txt +++ b/lang/c++/CMakeLists.txt @@ -68,7 +68,7 @@ if (WIN32 AND NOT CYGWIN AND NOT MSYS) endif() if (CMAKE_COMPILER_IS_GNUCXX) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -pedantic -Werror") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wduplicated-cond -Wduplicated-branches -Wlogical-op -Wuseless-cast -Wconversion -pedantic -Werror") if (AVRO_ADD_PROTECTOR_FLAGS) set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fstack-protector-all -D_GLIBCXX_DEBUG") # Unset _GLIBCXX_DEBUG for avrogencpp.cc because using Boost Program Options diff --git a/lang/c++/build.sh b/lang/c++/build.sh index 2873534c8c6..c1aaa3dd1ab 100755 --- a/lang/c++/build.sh +++ b/lang/c++/build.sh @@ -83,7 +83,7 @@ case "$target" in ;; test) - (cmake -S. -Bbuild -D CMAKE_BUILD_TYPE=Debug -D AVRO_ADD_PROTECTOR_FLAGS=1 && cmake --build build \ + (cmake -S. -Bbuild -D CMAKE_BUILD_TYPE=Debug -D AVRO_ADD_PROTECTOR_FLAGS=1 && cmake --build build -- -k \ && ./build/buffertest \ && ./build/unittest \ && ./build/AvrogencppTestReservedWords \ diff --git a/lang/c++/impl/Compiler.cc b/lang/c++/impl/Compiler.cc index 126c5c4e83f..3b287c9eeb0 100644 --- a/lang/c++/impl/Compiler.cc +++ b/lang/c++/impl/Compiler.cc @@ -345,9 +345,10 @@ static LogicalType makeLogicalType(const Entity &e, const Object &m) { if (typeField == "decimal") { LogicalType decimalType(LogicalType::DECIMAL); try { - decimalType.setPrecision(getLongField(e, m, "precision")); + // Precision probably won't go over 38 and scale beyond -77/+77 + decimalType.setPrecision(static_cast(getLongField(e, m, "precision"))); if (containsField(m, "scale")) { - decimalType.setScale(getLongField(e, m, "scale")); + decimalType.setScale(static_cast(getLongField(e, m, "scale"))); } } catch (Exception &ex) { // If any part of the logical type is malformed, per the standard we @@ -395,12 +396,12 @@ static NodePtr makeEnumNode(const Entity &e, static NodePtr makeFixedNode(const Entity &e, const Name &name, const Object &m) { - int v = static_cast(getLongField(e, m, "size")); + int64_t v = getLongField(e, m, "size"); if (v <= 0) { throw Exception("Size for fixed is not positive: {}", e.toString()); } NodePtr node = - NodePtr(new NodeFixed(asSingleAttribute(name), asSingleAttribute(v))); + NodePtr(new NodeFixed(asSingleAttribute(name), asSingleAttribute(static_cast(v)))); if (containsField(m, "doc")) { node->setDoc(getDocField(e, m)); } diff --git a/lang/c++/impl/DataFile.cc b/lang/c++/impl/DataFile.cc index 9f8827b7953..66281ae9820 100644 --- a/lang/c++/impl/DataFile.cc +++ b/lang/c++/impl/DataFile.cc @@ -195,10 +195,10 @@ void DataFileWriterBase::sync() { os.push(boost::iostreams::back_inserter(temp)); boost::iostreams::write(os, compressed.c_str(), compressed_size); } - temp.push_back((checksum >> 24) & 0xFF); - temp.push_back((checksum >> 16) & 0xFF); - temp.push_back((checksum >> 8) & 0xFF); - temp.push_back(checksum & 0xFF); + temp.push_back(static_cast((checksum >> 24) & 0xFF)); + temp.push_back(static_cast((checksum >> 16) & 0xFF)); + temp.push_back(static_cast((checksum >> 8) & 0xFF)); + temp.push_back(static_cast(checksum & 0xFF)); std::unique_ptr in = memoryInputStream( reinterpret_cast(temp.data()), temp.size()); int64_t byteCount = temp.size(); @@ -455,7 +455,7 @@ static ValidSchema makeSchema(const vector &v) { istringstream iss(toString(v)); ValidSchema vs; compileJsonSchema(iss, vs); - return ValidSchema(vs); + return vs; } void DataFileReaderBase::readHeader() { @@ -527,8 +527,7 @@ void DataFileReaderBase::sync(int64_t position) { eof_ = true; return; } - int len = - std::min(static_cast(SyncSize - i), n); + size_t len = std::min(SyncSize - i, n); memcpy(&sync_buffer[i], p, len); p += len; n -= len; diff --git a/lang/c++/impl/FileStream.cc b/lang/c++/impl/FileStream.cc index ccfb441514c..9063cf1f734 100644 --- a/lang/c++/impl/FileStream.cc +++ b/lang/c++/impl/FileStream.cc @@ -94,7 +94,7 @@ struct FileBufferCopyIn : public BufferCopyIn { } bool read(uint8_t *b, size_t toRead, size_t &actual) final { - int n = ::read(fd_, b, toRead); + auto n = ::read(fd_, b, toRead); if (n > 0) { actual = n; return true; diff --git a/lang/c++/impl/LogicalType.cc b/lang/c++/impl/LogicalType.cc index 988eed56dde..5e03a313d8f 100644 --- a/lang/c++/impl/LogicalType.cc +++ b/lang/c++/impl/LogicalType.cc @@ -28,7 +28,7 @@ LogicalType::Type LogicalType::type() const { return type_; } -void LogicalType::setPrecision(int precision) { +void LogicalType::setPrecision(int32_t precision) { if (type_ != DECIMAL) { throw Exception("Only logical type DECIMAL can have precision"); } @@ -38,7 +38,7 @@ void LogicalType::setPrecision(int precision) { precision_ = precision; } -void LogicalType::setScale(int scale) { +void LogicalType::setScale(int32_t scale) { if (type_ != DECIMAL) { throw Exception("Only logical type DECIMAL can have scale"); } diff --git a/lang/c++/impl/Node.cc b/lang/c++/impl/Node.cc index c16685a95c6..14ce6ecf05b 100644 --- a/lang/c++/impl/Node.cc +++ b/lang/c++/impl/Node.cc @@ -146,7 +146,7 @@ void Node::setLogicalType(LogicalType logicalType) { if (type_ == AVRO_FIXED) { // Max precision that can be supported by the current size of // the FIXED type. - long maxPrecision = floor(log10(2.0) * (8.0 * fixedSize() - 1)); + auto maxPrecision = static_cast(floor(log10(2.0) * (8.0 * static_cast(fixedSize()) - 1))); if (logicalType.precision() > maxPrecision) { throw Exception( "DECIMAL precision {} is too large for the " diff --git a/lang/c++/impl/NodeImpl.cc b/lang/c++/impl/NodeImpl.cc index 11a00329ebf..e3073aaaef2 100644 --- a/lang/c++/impl/NodeImpl.cc +++ b/lang/c++/impl/NodeImpl.cc @@ -71,7 +71,7 @@ string escape(const string &unescaped) { // Wrap an indentation in a struct for ostream operator<< struct indent { explicit indent(size_t depth) : d(depth) {} - int d; + size_t d; }; /// ostream operator for indent @@ -83,7 +83,7 @@ std::ostream &operator<<(std::ostream &os, indent x) { return os; } -void printCustomAttributes(const CustomAttributes &customAttributes, int depth, +void printCustomAttributes(const CustomAttributes &customAttributes, size_t depth, std::ostream &os) { std::map::const_iterator iter = customAttributes.attributes().begin(); @@ -531,9 +531,9 @@ void NodeEnum::printJson(std::ostream &os, size_t depth) const { printName(os, nameAttribute_.get(), depth); os << indent(depth) << "\"symbols\": [\n"; - int names = leafNameAttributes_.size(); + auto names = leafNameAttributes_.size(); ++depth; - for (int i = 0; i < names; ++i) { + for (size_t i = 0; i < names; ++i) { if (i > 0) { os << ",\n"; } @@ -577,9 +577,9 @@ NodeMap::NodeMap() : NodeImplMap(AVRO_MAP) { void NodeUnion::printJson(std::ostream &os, size_t depth) const { os << "[\n"; - int fields = leafAttributes_.size(); + auto fields = leafAttributes_.size(); ++depth; - for (int i = 0; i < fields; ++i) { + for (size_t i = 0; i < fields; ++i) { if (i > 0) { os << ",\n"; } diff --git a/lang/c++/impl/Resolver.cc b/lang/c++/impl/Resolver.cc index 9c5f46811c6..5fdd551a317 100644 --- a/lang/c++/impl/Resolver.cc +++ b/lang/c++/impl/Resolver.cc @@ -307,7 +307,7 @@ class EnumParser : public Resolver { void parse(Reader &reader, uint8_t *address) const final { auto val = static_cast(reader.readEnum()); - assert(static_cast(val) < mapping_.size()); + assert(val < mapping_.size()); if (mapping_[val] < readerSize_) { auto *location = reinterpret_cast(address + offset_); @@ -349,7 +349,7 @@ class UnionParser : public Resolver { *readerChoice = choiceMapping_[writerChoice]; auto *setter = reinterpret_cast(address + setFuncOffset_); - auto *value = reinterpret_cast(address + offset_); + uint8_t *value = address + offset_; uint8_t *location = (*setter)(value, *readerChoice); resolvers_[writerChoice]->parse(reader, location); @@ -397,7 +397,7 @@ class NonUnionToUnionParser : public Resolver { auto *choice = reinterpret_cast(address + choiceOffset_); *choice = choice_; auto *setter = reinterpret_cast(address + setFuncOffset_); - auto *value = reinterpret_cast(address + offset_); + uint8_t *value = address + offset_; uint8_t *location = (*setter)(value, choice_); resolver_->parse(reader, location); @@ -424,7 +424,7 @@ class FixedSkipper : public Resolver { } protected: - int size_; + size_t size_; }; class FixedParser : public Resolver { @@ -436,12 +436,12 @@ class FixedParser : public Resolver { void parse(Reader &reader, uint8_t *address) const final { DEBUG_OUT("Reading fixed"); - auto *location = reinterpret_cast(address + offset_); + uint8_t *location = address + offset_; reader.readFixed(location, size_); } protected: - int size_; + size_t size_; size_t offset_; }; diff --git a/lang/c++/impl/Validator.cc b/lang/c++/impl/Validator.cc index 04180e8e373..c00460480b1 100644 --- a/lang/c++/impl/Validator.cc +++ b/lang/c++/impl/Validator.cc @@ -62,7 +62,7 @@ bool Validator::countingSetup() { compoundStack_.pop_back(); proceed = false; } else { - counters_.push_back(static_cast(count_)); + counters_.push_back(count_); } } @@ -78,7 +78,7 @@ void Validator::countingAdvance() { setupOperation(node->leafAt(index)); } else { compoundStack_.back().pos = 0; - int count = --counters_.back(); + size_t count = --counters_.back(); if (count == 0) { counters_.pop_back(); compoundStarted_ = true; @@ -100,7 +100,7 @@ void Validator::unionAdvance() { waitingForCount_ = false; NodePtr node = compoundStack_.back().node; - if (count_ < static_cast(node->leaves())) { + if (count_ < node->leaves()) { compoundStack_.pop_back(); setupOperation(node->leafAt(static_cast(count_))); } else { @@ -116,7 +116,7 @@ void Validator::fixedAdvance() { compoundStack_.pop_back(); } -int Validator::nextSizeExpected() const { +size_t Validator::nextSizeExpected() const { return compoundStack_.back().node->fixedSize(); } @@ -168,11 +168,9 @@ void Validator::advance() { } } -void Validator::setCount(int64_t count) { +void Validator::setCount(size_t count) { if (!waitingForCount_) { throw Exception("Not expecting count"); - } else if (count_ < 0) { - throw Exception("Count cannot be negative"); } count_ = count; diff --git a/lang/c++/impl/Zigzag.cc b/lang/c++/impl/Zigzag.cc index 538a89cbaa7..7875f789bd2 100644 --- a/lang/c++/impl/Zigzag.cc +++ b/lang/c++/impl/Zigzag.cc @@ -30,11 +30,11 @@ encodeInt64(int64_t input, std::array &output) noexcept { auto v = val & mask; size_t bytesOut = 0; while (val >>= 7) { - output[bytesOut++] = (v | 0x80); + output[bytesOut++] = static_cast(v | 0x80); v = val & mask; } - output[bytesOut++] = v; + output[bytesOut++] = static_cast(v); return bytesOut; } size_t @@ -46,11 +46,11 @@ encodeInt32(int32_t input, std::array &output) noexcept { auto v = val & mask; size_t bytesOut = 0; while (val >>= 7) { - output[bytesOut++] = (v | 0x80); + output[bytesOut++] = static_cast(v | 0x80); v = val & mask; } - output[bytesOut++] = v; + output[bytesOut++] = static_cast(v); return bytesOut; } diff --git a/lang/c++/impl/json/JsonIO.cc b/lang/c++/impl/json/JsonIO.cc index 542848a0de5..8273f392e88 100644 --- a/lang/c++/impl/json/JsonIO.cc +++ b/lang/c++/impl/json/JsonIO.cc @@ -378,7 +378,7 @@ string JsonParser::decodeString(const string &s, bool binary) { if (n > 0xff) { throw Exception("Invalid byte for binary: {}{}", ch, string(startSeq, ++it)); } else { - result.push_back(n); + result.push_back(static_cast(n)); continue; } } @@ -400,19 +400,19 @@ string JsonParser::decodeString(const string &s, bool binary) { throw Exception("Invalid unicode sequence: {}", string(startSeq, it)); } if (n < 0x80) { - result.push_back(n); + result.push_back(static_cast(n)); } else if (n < 0x800) { - result.push_back((n >> 6) | 0xc0); - result.push_back((n & 0x3f) | 0x80); + result.push_back(static_cast((n >> 6) | 0xc0)); + result.push_back(static_cast((n & 0x3f) | 0x80)); } else if (n < 0x10000) { - result.push_back((n >> 12) | 0xe0); - result.push_back(((n >> 6) & 0x3f) | 0x80); - result.push_back((n & 0x3f) | 0x80); + result.push_back(static_cast((n >> 12) | 0xe0)); + result.push_back(static_cast(((n >> 6) & 0x3f) | 0x80)); + result.push_back(static_cast((n & 0x3f) | 0x80)); } else if (n < 0x110000) { - result.push_back((n >> 18) | 0xf0); - result.push_back(((n >> 12) & 0x3f) | 0x80); - result.push_back(((n >> 6) & 0x3f) | 0x80); - result.push_back((n & 0x3f) | 0x80); + result.push_back(static_cast((n >> 18) | 0xf0)); + result.push_back(static_cast(((n >> 12) & 0x3f) | 0x80)); + result.push_back(static_cast(((n >> 6) & 0x3f) | 0x80)); + result.push_back(static_cast((n & 0x3f) | 0x80)); } else { throw Exception("Invalid unicode value: {}{}", n, string(startSeq, ++it)); } diff --git a/lang/c++/impl/json/JsonIO.hh b/lang/c++/impl/json/JsonIO.hh index 4e15470f6af..203bf895fd5 100644 --- a/lang/c++/impl/json/JsonIO.hh +++ b/lang/c++/impl/json/JsonIO.hh @@ -34,7 +34,7 @@ namespace avro { namespace json { inline char toHex(unsigned int n) { - return (n < 10) ? (n + '0') : (n + 'a' - 10); + return static_cast((n < 10) ? (n + '0') : (n + 'a' - 10)); } class AVRO_DECL JsonParser : boost::noncopyable { @@ -266,8 +266,8 @@ class AVRO_DECL JsonGenerator { void escapeUnicode16(uint32_t c) { out_.write('\\'); out_.write('u'); - writeHex((c >> 8) & 0xff); - writeHex(c & 0xff); + writeHex(static_cast((c >> 8) & 0xff)); + writeHex(static_cast(c & 0xff)); } void escapeUnicode(uint32_t c) { if (c < 0x10000) { diff --git a/lang/c++/impl/parsing/ResolvingDecoder.cc b/lang/c++/impl/parsing/ResolvingDecoder.cc index 9abfd71ac16..1553b8a4b62 100644 --- a/lang/c++/impl/parsing/ResolvingDecoder.cc +++ b/lang/c++/impl/parsing/ResolvingDecoder.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -65,7 +66,7 @@ class ResolvingGrammarGenerator : public ValidatingGrammarGenerator { const NodePtr &reader, map &m, map &m2); - static int bestBranch(const NodePtr &writer, const NodePtr &reader); + static std::optional bestBranch(const NodePtr &writer, const NodePtr &reader); ProductionPtr getWriterProduction(const NodePtr &n, map &m2); @@ -90,8 +91,8 @@ Symbol ResolvingGrammarGenerator::generate( return Symbol::rootSymbol(main, backup); } -int ResolvingGrammarGenerator::bestBranch(const NodePtr &writer, - const NodePtr &reader) { +std::optional ResolvingGrammarGenerator::bestBranch(const NodePtr &writer, + const NodePtr &reader) { Type t = writer->type(); const size_t c = reader->leaves(); @@ -130,7 +131,7 @@ int ResolvingGrammarGenerator::bestBranch(const NodePtr &writer, break; } } - return -1; + return std::nullopt; } static shared_ptr> getAvroBinary( @@ -373,11 +374,11 @@ ProductionPtr ResolvingGrammarGenerator::doGenerate2( break; case AVRO_UNION: { - int j = bestBranch(writer, reader); - if (j >= 0) { - ProductionPtr p = doGenerate2(writer, reader->leafAt(j), m, m2); + auto j = bestBranch(writer, reader); + if (j) { + ProductionPtr p = doGenerate2(writer, reader->leafAt(*j), m, m2); ProductionPtr result = make_shared(); - result->push_back(Symbol::unionAdjustSymbol(j, p)); + result->push_back(Symbol::unionAdjustSymbol(*j, p)); result->push_back(Symbol::unionSymbol()); return result; } @@ -513,16 +514,18 @@ int64_t ResolvingDecoderImpl

::decodeLong() { template float ResolvingDecoderImpl

::decodeFloat() { Symbol::Kind k = parser_.advance(Symbol::Kind::Float); - return k == Symbol::Kind::Int ? base_->decodeInt() : k == Symbol::Kind::Long ? base_->decodeLong() - : base_->decodeFloat(); + return k == Symbol::Kind::Int ? static_cast(base_->decodeInt()) + : k == Symbol::Kind::Long ? static_cast(base_->decodeLong()) + : base_->decodeFloat(); } template double ResolvingDecoderImpl

::decodeDouble() { Symbol::Kind k = parser_.advance(Symbol::Kind::Double); - return k == Symbol::Kind::Int ? base_->decodeInt() : k == Symbol::Kind::Long ? base_->decodeLong() - : k == Symbol::Kind::Float ? base_->decodeFloat() - : base_->decodeDouble(); + return k == Symbol::Kind::Int ? static_cast(base_->decodeInt()) + : k == Symbol::Kind::Long ? static_cast(base_->decodeLong()) + : k == Symbol::Kind::Float ? base_->decodeFloat() + : base_->decodeDouble(); } template diff --git a/lang/c++/impl/parsing/Symbol.cc b/lang/c++/impl/parsing/Symbol.cc index b7a35517af8..fe87c5205b4 100644 --- a/lang/c++/impl/parsing/Symbol.cc +++ b/lang/c++/impl/parsing/Symbol.cc @@ -75,7 +75,7 @@ Symbol Symbol::enumAdjustSymbol(const NodePtr &writer, const NodePtr &reader) { } size_t wc = writer->names(); - vector adj; + vector adj; // enums are encoded as ints adj.reserve(wc); vector err; @@ -85,10 +85,10 @@ Symbol Symbol::enumAdjustSymbol(const NodePtr &writer, const NodePtr &reader) { vector::const_iterator it = find(rs.begin(), rs.end(), s); if (it == rs.end()) { auto pos = err.size() + 1; - adj.push_back(-pos); + adj.push_back(static_cast(-pos)); err.push_back(s); } else { - adj.push_back(it - rs.begin()); + adj.push_back(static_cast(it - rs.begin())); } } return Symbol(Kind::EnumAdjust, make_pair(adj, err)); diff --git a/lang/c++/include/avro/LogicalType.hh b/lang/c++/include/avro/LogicalType.hh index 4d06e74f635..ff430fd086a 100644 --- a/lang/c++/include/avro/LogicalType.hh +++ b/lang/c++/include/avro/LogicalType.hh @@ -47,17 +47,17 @@ public: // Precision must be positive and scale must be either positive or zero. The // setters will throw an exception if they are called on any type other // than DECIMAL. - void setPrecision(int precision); - int precision() const { return precision_; } - void setScale(int scale); - int scale() const { return scale_; } + void setPrecision(int32_t precision); + int32_t precision() const { return precision_; } + void setScale(int32_t scale); + int32_t scale() const { return scale_; } void printJson(std::ostream &os) const; private: Type type_; - int precision_; - int scale_; + int32_t precision_; + int32_t scale_; }; } // namespace avro diff --git a/lang/c++/include/avro/NodeImpl.hh b/lang/c++/include/avro/NodeImpl.hh index 0fb2ca170c6..3e5546c94ea 100644 --- a/lang/c++/include/avro/NodeImpl.hh +++ b/lang/c++/include/avro/NodeImpl.hh @@ -226,8 +226,8 @@ using LeafNames = concepts::MultiAttribute; using MultiAttributes = concepts::MultiAttribute; using NoAttributes = concepts::NoAttribute; -using NoSize = concepts::NoAttribute; -using HasSize = concepts::SingleAttribute; +using NoSize = concepts::NoAttribute; +using HasSize = concepts::SingleAttribute; using NodeImplPrimitive = NodeImpl; using NodeImplSymbolic = NodeImpl; diff --git a/lang/c++/include/avro/Reader.hh b/lang/c++/include/avro/Reader.hh index ff69f68d32f..62d81c2365f 100644 --- a/lang/c++/include/avro/Reader.hh +++ b/lang/c++/include/avro/Reader.hh @@ -176,15 +176,15 @@ private: return encoded; } - int64_t readSize() { + size_t readSize() { uint64_t encoded = readVarInt(); - int64_t size = decodeZigzag64(encoded); + auto size = static_cast(decodeZigzag64(encoded)); return size; } - int64_t readCount() { + size_t readCount() { validator_.checkTypeExpected(AVRO_LONG); - int64_t count = readSize(); + size_t count = readSize(); validator_.setCount(count); return count; } diff --git a/lang/c++/include/avro/Validator.hh b/lang/c++/include/avro/Validator.hh index 8f1bf31b068..6437a549ff1 100644 --- a/lang/c++/include/avro/Validator.hh +++ b/lang/c++/include/avro/Validator.hh @@ -35,7 +35,7 @@ public: explicit NullValidator(const ValidSchema &) {} NullValidator() = default; - void setCount(int64_t) {} + void setCount(size_t) {} static bool typeIsExpected(Type) { return true; @@ -45,7 +45,7 @@ public: return AVRO_UNKNOWN; } - static int nextSizeExpected() { + static size_t nextSizeExpected() { return 0; } @@ -58,7 +58,7 @@ public: } void checkTypeExpected(Type) {} - void checkFixedSizeExpected(int) {} + void checkFixedSizeExpected(size_t) {} }; /// This class is used by both the ValidatingSerializer and ValidationParser @@ -71,7 +71,7 @@ class AVRO_DECL Validator : private boost::noncopyable { public: explicit Validator(ValidSchema schema); - void setCount(int64_t val); + void setCount(size_t val); bool typeIsExpected(Type type) const { return (expectedTypesFlag_ & typeToFlag(type)) != 0; @@ -81,7 +81,7 @@ public: return nextType_; } - int nextSizeExpected() const; + size_t nextSizeExpected() const; bool getCurrentRecordName(std::string &name) const; bool getNextFieldName(std::string &name) const; @@ -93,7 +93,7 @@ public: advance(); } - void checkFixedSizeExpected(int size) { + void checkFixedSizeExpected(size_t size) { if (nextSizeExpected() != size) { throw Exception("Wrong size for fixed, got {}, expected {}", size, nextSizeExpected()); } @@ -104,7 +104,7 @@ private: using flag_t = uint32_t; static flag_t typeToFlag(Type type) { - flag_t flag = (1L << type); + flag_t flag = 1u << static_cast(type); return flag; } @@ -129,7 +129,7 @@ private: flag_t expectedTypesFlag_; bool compoundStarted_; bool waitingForCount_; - int64_t count_; + size_t count_; struct CompoundType { explicit CompoundType(NodePtr n) : node(std::move(n)), pos(0) {} diff --git a/lang/c++/include/avro/buffer/Buffer.hh b/lang/c++/include/avro/buffer/Buffer.hh index 45c439d6d43..16a22ef626e 100644 --- a/lang/c++/include/avro/buffer/Buffer.hh +++ b/lang/c++/include/avro/buffer/Buffer.hh @@ -276,7 +276,7 @@ public: * Returns the number of chunks that contain free space. **/ - int numChunks() const { + size_t numChunks() const { return pimpl_->numFreeChunks(); } @@ -284,7 +284,7 @@ public: * Returns the number of chunks that contain data **/ - int numDataChunks() const { + size_t numDataChunks() const { return pimpl_->numDataChunks(); } @@ -384,7 +384,7 @@ public: * Returns the number of chunks containing data. **/ - int numChunks() const { + size_t numChunks() const { return pimpl_->numDataChunks(); } @@ -476,10 +476,10 @@ inline InputBuffer OutputBuffer::extractData(size_type bytes) { template inline void toIovec(BufferType &buf, std::vector &iov) { - const int chunks = buf.numChunks(); + const size_t chunks = buf.numChunks(); iov.resize(chunks); typename BufferType::const_iterator iter = buf.begin(); - for (int i = 0; i < chunks; ++i) { + for (size_t i = 0; i < chunks; ++i) { iov[i].iov_base = const_cast(iter->data()); iov[i].iov_len = iter->size(); ++iter; diff --git a/lang/c++/include/avro/buffer/BufferPrint.hh b/lang/c++/include/avro/buffer/BufferPrint.hh index c8eb15b719a..8d4001529c9 100644 --- a/lang/c++/include/avro/buffer/BufferPrint.hh +++ b/lang/c++/include/avro/buffer/BufferPrint.hh @@ -47,7 +47,7 @@ hexPrint(std::ostream &os, BufferReader &reader) { std::ios_base::fmtflags savedFlags = os.flags(); char sixteenBytes[16]; - int offset = 0; + size_t offset = 0; os << std::setfill('0'); os << std::hex; diff --git a/lang/c++/include/avro/buffer/BufferStreambuf.hh b/lang/c++/include/avro/buffer/BufferStreambuf.hh index 2b7aea4d779..42eb20c21c6 100644 --- a/lang/c++/include/avro/buffer/BufferStreambuf.hh +++ b/lang/c++/include/avro/buffer/BufferStreambuf.hh @@ -135,7 +135,11 @@ protected: memcpy(c, gptr(), toCopy); c += toCopy; bytesCopied += toCopy; - gbump(toCopy); + while (toCopy > static_cast(std::numeric_limits::max())) { + gbump(std::numeric_limits::max()); + toCopy -= static_cast(std::numeric_limits::max()); + } + gbump(static_cast(toCopy)); } if (bytesCopied < len) { diff --git a/lang/c++/include/avro/buffer/detail/BufferDetail.hh b/lang/c++/include/avro/buffer/detail/BufferDetail.hh index b487cdb3935..652e98d51ba 100644 --- a/lang/c++/include/avro/buffer/detail/BufferDetail.hh +++ b/lang/c++/include/avro/buffer/detail/BufferDetail.hh @@ -481,13 +481,13 @@ public: } /// The number of chunks containing data. Used for debugging. - int numDataChunks() const { + size_t numDataChunks() const { return readChunks_.size(); } /// The number of chunks containing free space (note that an entire chunk /// may not be free). Used for debugging. - int numFreeChunks() const { + size_t numFreeChunks() const { return writeChunks_.size(); } diff --git a/lang/c++/test/CodecTests.cc b/lang/c++/test/CodecTests.cc index 64b900777ea..b8a3b64ddf1 100644 --- a/lang/c++/test/CodecTests.cc +++ b/lang/c++/test/CodecTests.cc @@ -160,7 +160,7 @@ static string randomString(size_t len) { if (c == '\0') { c = '\x7f'; } - result.push_back(c); + result.push_back(static_cast(c)); } return result; } @@ -169,7 +169,7 @@ static vector randomBytes(size_t len) { vector result; result.reserve(len); for (size_t i = 0; i < len; ++i) { - result.push_back(rnd()); + result.push_back(static_cast(rnd())); } return result; } @@ -525,7 +525,7 @@ ValidSchema makeValidSchema(const char *schema) { istringstream iss(schema); ValidSchema vs; compileJsonSchema(iss, vs); - return ValidSchema(vs); + return vs; } void testEncoder(const EncoderPtr &e, const char *writerCalls, diff --git a/lang/c++/test/DataFileTests.cc b/lang/c++/test/DataFileTests.cc index 08138cdb540..ad6796fbcf8 100644 --- a/lang/c++/test/DataFileTests.cc +++ b/lang/c++/test/DataFileTests.cc @@ -123,7 +123,7 @@ static ValidSchema makeValidSchema(const char *schema) { istringstream iss(schema); ValidSchema vs; compileJsonSchema(iss, vs); - return ValidSchema(vs); + return vs; } static const char sch[] = "{\"type\": \"record\"," @@ -405,7 +405,7 @@ class DataFileTest { } std::set> actual; int num = 0; - for (int i = sync_points.size() - 2; i >= 0; --i) { + for (ssize_t i = sync_points.size() - 2; i >= 0; --i) { df.seek(sync_points[i]); ComplexInteger ci; // Subtract avro::SyncSize here because sync and pastSync @@ -473,7 +473,7 @@ class DataFileTest { avro::DataFileReader df(filename, writerSchema); std::ifstream just_for_length( filename, std::ifstream::ate | std::ifstream::binary); - int length = just_for_length.tellg(); + int length = static_cast(just_for_length.tellg()); int splits = 10; int end = length; // end of split int remaining = end; // bytes remaining @@ -575,7 +575,7 @@ class DataFileTest { } { avro::DataFileReader reader(filename, dschema); - std::vector found; + std::vector found; ComplexInteger record; while (reader.read(record)) { found.push_back(record.re); @@ -948,7 +948,7 @@ void testReadRecordEfficientlyUsingLastSync(avro::Codec codec) { std::unique_ptr inputStream = avro::memoryInputStream(stitchedData.data(), stitchedData.size()); - int recordsUptoRecordToRead = recordToRead - recordsUptoLastSync; + size_t recordsUptoRecordToRead = recordToRead - recordsUptoLastSync; // Ensure this is not the first record in the chunk. BOOST_CHECK_GT(recordsUptoRecordToRead, 0); @@ -956,7 +956,7 @@ void testReadRecordEfficientlyUsingLastSync(avro::Codec codec) { avro::DataFileReader df(std::move(inputStream)); TestRecord readRecord("", 0); //::printf("\nReading %d rows until specific record is reached", recordsUptoRecordToRead); - for (int index = 0; index < recordsUptoRecordToRead; index++) { + for (size_t index = 0; index < recordsUptoRecordToRead; index++) { BOOST_CHECK_EQUAL(df.read(readRecord), true); int64_t expectedId = (recordToRead - recordsUptoRecordToRead + index); diff --git a/lang/c++/test/StreamTests.cc b/lang/c++/test/StreamTests.cc index 7eae5e12aaf..2096197ef18 100644 --- a/lang/c++/test/StreamTests.cc +++ b/lang/c++/test/StreamTests.cc @@ -51,7 +51,7 @@ struct Fill1 { StreamWriter w; w.reset(os); for (size_t i = 0; i < len; ++i) { - w.write(i % 10 + '0'); + w.write(static_cast(i % 10 + '0')); } w.flush(); } @@ -65,7 +65,7 @@ struct Fill2 { os.next(&b, &n); size_t j = 0; for (; i < len && j < n; ++j, ++i, ++b) { - *b = i % 10 + '0'; + *b = static_cast(i % 10 + '0'); } if (i == len) { os.backup(n - j); @@ -125,7 +125,7 @@ void testNonEmpty_memoryStream(const TestData &td) { void testNonEmpty2(const TestData &td) { std::vector v; for (size_t i = 0; i < td.dataSize; ++i) { - v.push_back(i % 10 + '0'); + v.push_back(static_cast(i % 10 + '0')); } uint8_t v2 = 0; diff --git a/lang/c++/test/buffertest.cc b/lang/c++/test/buffertest.cc index 3a4ede4c432..23e0e806c21 100644 --- a/lang/c++/test/buffertest.cc +++ b/lang/c++/test/buffertest.cc @@ -34,19 +34,18 @@ using detail::kMinBlockSize; using std::cout; using std::endl; +// Make a string of repeating 0123456789ABCDEF0123456789... std::string makeString(size_t len) { - std::string newstring; - newstring.reserve(len); + std::string result; + result.reserve(len); + + constexpr char chars[] = "0123456789ABCDEF"; for (size_t i = 0; i < len; ++i) { - char newchar = '0' + i % 16; - if (newchar > '9') { - newchar += 7; - } - newstring.push_back(newchar); + result.push_back(chars[i % 16]); } - return newstring; + return result; } void printBuffer(const InputBuffer &buf) { @@ -219,7 +218,7 @@ void TestDiscard() { BOOST_CHECK_EQUAL(ob.freeSpace(), kDefaultBlockSize / 2); BOOST_CHECK_EQUAL(ob.numChunks(), 1); - int chunks = 3 - (discarded / kDefaultBlockSize); + size_t chunks = 3 - (discarded / kDefaultBlockSize); BOOST_CHECK_EQUAL(ob.numDataChunks(), chunks); } @@ -331,7 +330,7 @@ void TestExtractToInput() { BOOST_CHECK_EQUAL(ob.freeSpace(), kDefaultBlockSize / 2); BOOST_CHECK_EQUAL(ob.numChunks(), 1); - int chunks = 3 - (extracted / kDefaultBlockSize); + size_t chunks = 3 - (extracted / kDefaultBlockSize); BOOST_CHECK_EQUAL(ob.numDataChunks(), chunks); } @@ -526,7 +525,7 @@ void TestSeek() { avro::InputBuffer buf(tmp1); cout << "Starting string: " << str << '\n'; - BOOST_CHECK_EQUAL(static_cast(buf.size()), str.size()); + BOOST_CHECK_EQUAL(buf.size(), str.size()); avro::istream is(buf); diff --git a/lang/c++/test/unittest.cc b/lang/c++/test/unittest.cc index 1c6799e3e66..3558a0e2f89 100644 --- a/lang/c++/test/unittest.cc +++ b/lang/c++/test/unittest.cc @@ -1031,8 +1031,8 @@ struct TestResolution { }; void testNestedArraySchema() { - ArraySchema b0 = ArraySchema(NullSchema()); - ArraySchema a0 = ArraySchema(b0); + ArraySchema b0{NullSchema()}; + ArraySchema a0 = b0; avro::ValidSchema vs(a0); std::ostringstream actual; @@ -1049,8 +1049,8 @@ void testNestedArraySchema() { } void testNestedMapSchema() { - MapSchema b0 = MapSchema(NullSchema()); - MapSchema a0 = MapSchema(b0); + MapSchema b0{NullSchema()}; + MapSchema a0 = b0; avro::ValidSchema vs(a0); std::ostringstream actual;