From bceb246d8e1eadca78acf7fc6a9896b92d7576fe Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Wed, 18 Mar 2020 05:02:24 +0300 Subject: [PATCH] Added most of bugprone checks --- .clang-tidy | 32 ++++++ dbms/programs/obfuscator/Obfuscator.cpp | 2 +- dbms/programs/odbc-bridge/ODBCBridge.cpp | 4 +- dbms/programs/server/Server.cpp | 2 +- dbms/src/Access/AccessRights.cpp | 3 + dbms/src/Access/QuotaContext.cpp | 3 + ...ggregateFunctionSimpleLinearRegression.cpp | 2 +- dbms/src/Columns/ColumnAggregateFunction.cpp | 2 +- dbms/src/Common/ThreadPool.cpp | 2 +- dbms/src/Common/tests/auto_array.cpp | 4 +- dbms/src/Common/tests/pod_array.cpp | 4 +- dbms/src/Core/SettingsCollection.cpp | 4 +- dbms/src/Dictionaries/CacheDictionary.cpp | 4 +- ...acheDictionary_createAttributeWithType.cpp | 4 +- dbms/src/Dictionaries/HashedDictionary.cpp | 2 +- .../Dictionaries/RedisDictionarySource.cpp | 2 +- dbms/src/Formats/ProtobufReader.cpp | 99 ++++++++++--------- dbms/src/Functions/GeoUtils.cpp | 10 +- dbms/src/Functions/array/arrayUniq.cpp | 2 +- dbms/src/Functions/trim.cpp | 4 +- dbms/src/IO/parseDateTimeBestEffort.cpp | 4 +- dbms/src/Interpreters/Aggregator.cpp | 4 +- .../InterpreterKillQueryQuery.cpp | 2 +- dbms/src/Interpreters/SetVariants.cpp | 6 +- .../Interpreters/tests/hash_map_string_2.cpp | 6 +- .../Interpreters/tests/hash_map_string_3.cpp | 4 +- dbms/src/Parsers/ASTTablesInSelectQuery.cpp | 2 +- dbms/src/Parsers/CommonParsers.cpp | 2 +- dbms/src/Processors/ForkProcessor.cpp | 2 +- dbms/src/Storages/Kafka/StorageKafka.cpp | 4 +- .../src/Storages/LiveView/StorageLiveView.cpp | 2 +- dbms/src/Storages/MergeTree/KeyCondition.cpp | 2 +- .../MergeTree/MergeTreeDataSelectExecutor.cpp | 6 +- .../Storages/MergeTree/MergeTreeSettings.cpp | 2 +- dbms/src/Storages/StorageInMemoryMetadata.cpp | 4 +- 35 files changed, 147 insertions(+), 96 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 49773ad31c9..7dd495237a7 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -57,6 +57,38 @@ Checks: '-*, bugprone-inaccurate-erase, bugprone-incorrect-roundings, bugprone-infinite-loop, + bugprone-integer-division, + bugprone-macro-parentheses, + bugprone-macro-repeated-side-effects, + bugprone-misplaced-operator-in-strlen-in-alloc, + bugprone-misplaced-pointer-artithmetic-in-alloc, + bugprone-misplaced-widening-cast, + bugprone-move-forwarding-reference, + bugprone-multiple-statement-macro, + bugprone-parent-virtual-call, + bugprone-posix-return, + bugprone-reserved-identifier, + bugprone-signed-char-misuse, + bugprone-sizeof-container, + bugprone-sizeof-expression, + bugprone-string-constructor, + bugprone-string-integer-assignment, + bugprone-string-literal-with-embedded-nul, + bugprone-suspicious-enum-usage, + bugprone-suspicious-include, + bugprone-suspicious-memset-usage, + bugprone-suspicious-missing-comma, + bugprone-suspicious-string-compare, + bugprone-swapped-arguments, + bugprone-terminating-continue, + bugprone-throw-keyword-missing, + bugprone-too-small-loop-variable, + bugprone-undefined-memory-manipulation, + bugprone-unhandled-self-assignment, + bugprone-unused-raii, + bugprone-unused-return-value, + bugprone-use-after-move, + bugprone-virtual-near-miss, boost-use-to-string, ' diff --git a/dbms/programs/obfuscator/Obfuscator.cpp b/dbms/programs/obfuscator/Obfuscator.cpp index b67041f44d6..a92a0d03287 100644 --- a/dbms/programs/obfuscator/Obfuscator.cpp +++ b/dbms/programs/obfuscator/Obfuscator.cpp @@ -677,7 +677,7 @@ public: if (!histogram.total) continue; - double average = histogram.total / histogram.buckets.size(); + double average = double(histogram.total) / histogram.buckets.size(); UInt64 new_total = 0; for (auto & bucket : histogram.buckets) diff --git a/dbms/programs/odbc-bridge/ODBCBridge.cpp b/dbms/programs/odbc-bridge/ODBCBridge.cpp index 565ee5602ca..76949cfa483 100644 --- a/dbms/programs/odbc-bridge/ODBCBridge.cpp +++ b/dbms/programs/odbc-bridge/ODBCBridge.cpp @@ -111,7 +111,7 @@ void ODBCBridge::defineOptions(Poco::Util::OptionSet & options) .binding("help") .callback(Poco::Util::OptionCallback(this, &Me::handleHelp))); - ServerApplication::defineOptions(options); /// Don't need complex BaseDaemon's .xml config + ServerApplication::defineOptions(options); // NOLINT Don't need complex BaseDaemon's .xml config } void ODBCBridge::initialize(Application & self) @@ -138,7 +138,7 @@ void ODBCBridge::initialize(Application & self) initializeTerminationAndSignalProcessing(); - ServerApplication::initialize(self); + ServerApplication::initialize(self); // NOLINT } void ODBCBridge::uninitialize() diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index 81c2de8ce3a..aaf19888f5e 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -175,7 +175,7 @@ int Server::run() std::cout << DBMS_NAME << " server version " << VERSION_STRING << VERSION_OFFICIAL << "." << std::endl; return 0; } - return Application::run(); + return Application::run(); // NOLINT } void Server::initialize(Poco::Util::Application & self) diff --git a/dbms/src/Access/AccessRights.cpp b/dbms/src/Access/AccessRights.cpp index 4f92d8c31c9..80de185ed8f 100644 --- a/dbms/src/Access/AccessRights.cpp +++ b/dbms/src/Access/AccessRights.cpp @@ -75,6 +75,9 @@ public: Node & operator =(const Node & src) { + if (this == &src) + return *this; + node_name = src.node_name; level = src.level; inherited_access = src.inherited_access; diff --git a/dbms/src/Access/QuotaContext.cpp b/dbms/src/Access/QuotaContext.cpp index 815d9440eaa..a48c41dc419 100644 --- a/dbms/src/Access/QuotaContext.cpp +++ b/dbms/src/Access/QuotaContext.cpp @@ -135,6 +135,9 @@ struct QuotaContext::Impl QuotaContext::Interval & QuotaContext::Interval::operator =(const Interval & src) { + if (this == &src) + return *this; + randomize_interval = src.randomize_interval; duration = src.duration; end_of_interval.store(src.end_of_interval.load()); diff --git a/dbms/src/AggregateFunctions/AggregateFunctionSimpleLinearRegression.cpp b/dbms/src/AggregateFunctions/AggregateFunctionSimpleLinearRegression.cpp index 64f37cd2e14..46c9402c36e 100644 --- a/dbms/src/AggregateFunctions/AggregateFunctionSimpleLinearRegression.cpp +++ b/dbms/src/AggregateFunctions/AggregateFunctionSimpleLinearRegression.cpp @@ -56,7 +56,7 @@ AggregateFunctionPtr createAggregateFunctionSimpleLinearRegression( FOR_LEASTSQR_TYPES_2(M, Float64) #define DISPATCH(T1, T2) \ if (which_x.idx == TypeIndex::T1 && which_y.idx == TypeIndex::T2) \ - return std::make_shared>( \ + return std::make_shared>( /* NOLINT */ \ arguments, \ params \ ); diff --git a/dbms/src/Columns/ColumnAggregateFunction.cpp b/dbms/src/Columns/ColumnAggregateFunction.cpp index 7ea2a3f9dfe..ce07acd1c0d 100644 --- a/dbms/src/Columns/ColumnAggregateFunction.cpp +++ b/dbms/src/Columns/ColumnAggregateFunction.cpp @@ -501,7 +501,7 @@ MutableColumns ColumnAggregateFunction::scatter(IColumn::ColumnIndex num_columns size_t num_rows = size(); { - size_t reserve_size = num_rows / num_columns * 1.1; /// 1.1 is just a guess. Better to use n-sigma rule. + size_t reserve_size = double(num_rows) / num_columns * 1.1; /// 1.1 is just a guess. Better to use n-sigma rule. if (reserve_size > 1) for (auto & column : columns) diff --git a/dbms/src/Common/ThreadPool.cpp b/dbms/src/Common/ThreadPool.cpp index c1cad465ed2..7334188952c 100644 --- a/dbms/src/Common/ThreadPool.cpp +++ b/dbms/src/Common/ThreadPool.cpp @@ -225,7 +225,7 @@ void ThreadPoolImpl::worker(typename std::list::iterator thread_ { std::unique_lock lock(mutex); if (!first_exception) - first_exception = std::current_exception(); + first_exception = std::current_exception(); // NOLINT shutdown = true; --scheduled_jobs; } diff --git a/dbms/src/Common/tests/auto_array.cpp b/dbms/src/Common/tests/auto_array.cpp index bbb533b65e8..3dc490796fa 100644 --- a/dbms/src/Common/tests/auto_array.cpp +++ b/dbms/src/Common/tests/auto_array.cpp @@ -43,7 +43,7 @@ int main(int argc, char ** argv) Arr arr2 = std::move(arr); - std::cerr << arr.size() << ", " << arr2.size() << std::endl; + std::cerr << arr.size() << ", " << arr2.size() << std::endl; // NOLINT for (auto & elem : arr2) std::cerr << elem << std::endl; @@ -182,7 +182,7 @@ int main(int argc, char ** argv) } arr2 = std::move(arr1); - arr1.resize(n); + arr1.resize(n); // NOLINT std::cerr << "arr1.size(): " << arr1.size() << ", arr2.size(): " << arr2.size() << std::endl diff --git a/dbms/src/Common/tests/pod_array.cpp b/dbms/src/Common/tests/pod_array.cpp index 2a3093b3de7..de15b485411 100644 --- a/dbms/src/Common/tests/pod_array.cpp +++ b/dbms/src/Common/tests/pod_array.cpp @@ -409,7 +409,7 @@ static void test3() Array arr2{std::move(arr)}; - ASSERT_CHECK((arr.empty()), res); + ASSERT_CHECK((arr.empty()), res); // NOLINT ASSERT_CHECK((arr2.size() == 3), res); ASSERT_CHECK((arr2[0] == 1), res); @@ -428,7 +428,7 @@ static void test3() Array arr2{std::move(arr)}; - ASSERT_CHECK((arr.empty()), res); + ASSERT_CHECK((arr.empty()), res); // NOLINT ASSERT_CHECK((arr2.size() == 5), res); ASSERT_CHECK((arr2[0] == 1), res); diff --git a/dbms/src/Core/SettingsCollection.cpp b/dbms/src/Core/SettingsCollection.cpp index b830c35b81c..d45c082eb0b 100644 --- a/dbms/src/Core/SettingsCollection.cpp +++ b/dbms/src/Core/SettingsCollection.cpp @@ -465,7 +465,7 @@ void SettingURI::deserialize(ReadBuffer & buf, SettingsBinaryFormat) case static_cast(EnumType::NAME): return IO_NAME; #define IMPLEMENT_SETTING_ENUM_FROM_STRING_HELPER_(NAME, IO_NAME) \ - if (s == IO_NAME) \ + if (s == (IO_NAME)) \ { \ set(EnumType::NAME); \ return; \ @@ -474,7 +474,7 @@ void SettingURI::deserialize(ReadBuffer & buf, SettingsBinaryFormat) #define IMPLEMENT_SETTING_ENUM_CONCAT_NAMES_HELPER_(NAME, IO_NAME) \ if (!all_io_names.empty()) \ all_io_names += ", "; \ - all_io_names += String("'") + IO_NAME + "'"; + all_io_names += String("'") + (IO_NAME) + "'"; #define LOAD_BALANCING_LIST_OF_NAMES(M) \ diff --git a/dbms/src/Dictionaries/CacheDictionary.cpp b/dbms/src/Dictionaries/CacheDictionary.cpp index 2294c99c111..36a8c704f4f 100644 --- a/dbms/src/Dictionaries/CacheDictionary.cpp +++ b/dbms/src/Dictionaries/CacheDictionary.cpp @@ -447,8 +447,8 @@ CacheDictionary::Attribute CacheDictionary::createAttributeWithType(const Attrib { #define DISPATCH(TYPE) \ case AttributeUnderlyingType::ut##TYPE: \ - attr.null_values = TYPE(null_value.get>()); \ - attr.arrays = std::make_unique>(size); \ + attr.null_values = TYPE(null_value.get>()); /* NOLINT */ \ + attr.arrays = std::make_unique>(size); /* NOLINT */ \ bytes_allocated += size * sizeof(TYPE); \ break; DISPATCH(UInt8) diff --git a/dbms/src/Dictionaries/ComplexKeyCacheDictionary_createAttributeWithType.cpp b/dbms/src/Dictionaries/ComplexKeyCacheDictionary_createAttributeWithType.cpp index e15a6fb3014..ba9f8d014fd 100644 --- a/dbms/src/Dictionaries/ComplexKeyCacheDictionary_createAttributeWithType.cpp +++ b/dbms/src/Dictionaries/ComplexKeyCacheDictionary_createAttributeWithType.cpp @@ -11,8 +11,8 @@ ComplexKeyCacheDictionary::createAttributeWithType(const AttributeUnderlyingType { #define DISPATCH(TYPE) \ case AttributeUnderlyingType::ut##TYPE: \ - attr.null_values = TYPE(null_value.get>()); \ - attr.arrays = std::make_unique>(size); \ + attr.null_values = TYPE(null_value.get>()); /* NOLINT */ \ + attr.arrays = std::make_unique>(size); /* NOLINT */ \ bytes_allocated += size * sizeof(TYPE); \ break; DISPATCH(UInt8) diff --git a/dbms/src/Dictionaries/HashedDictionary.cpp b/dbms/src/Dictionaries/HashedDictionary.cpp index 722a6e3584c..2bdd33a3d2e 100644 --- a/dbms/src/Dictionaries/HashedDictionary.cpp +++ b/dbms/src/Dictionaries/HashedDictionary.cpp @@ -446,7 +446,7 @@ void HashedDictionary::addAttributeSize(const Attribute & attribute) /** TODO: more accurate calculation */ bytes_allocated += sizeof(CollectionType); bytes_allocated += bucket_count; - bytes_allocated += map_ref->size() * sizeof(Key) * sizeof(T); + bytes_allocated += map_ref->size() * (sizeof(Key) + sizeof(T)); } } diff --git a/dbms/src/Dictionaries/RedisDictionarySource.cpp b/dbms/src/Dictionaries/RedisDictionarySource.cpp index c51e5cdadd4..3c5aaf4bb6b 100644 --- a/dbms/src/Dictionaries/RedisDictionarySource.cpp +++ b/dbms/src/Dictionaries/RedisDictionarySource.cpp @@ -183,7 +183,7 @@ namespace DB /// Do not store more than max_block_size values for one request. if (primary_with_secondary.size() == max_block_size + 1) { - hkeys.add(std::move(primary_with_secondary)); + hkeys.add(primary_with_secondary); primary_with_secondary.clear(); primary_with_secondary.addRedisType(key); } diff --git a/dbms/src/Formats/ProtobufReader.cpp b/dbms/src/Formats/ProtobufReader.cpp index 3874ec3e447..5426e8fac62 100644 --- a/dbms/src/Formats/ProtobufReader.cpp +++ b/dbms/src/Formats/ProtobufReader.cpp @@ -273,30 +273,35 @@ UInt64 ProtobufReader::SimpleReader::continueReadingVarint(UInt64 first_byte) char c; #define PROTOBUF_READER_READ_VARINT_BYTE(byteNo) \ - in.readStrict(c); \ - ++cursor; \ - if constexpr (byteNo < 10) \ + do \ { \ - result |= static_cast(static_cast(c)) << (7 * (byteNo - 1)); \ - if (likely(!(c & 0x80))) \ - return result; \ - } \ - else \ - { \ - if (likely(c == 1)) \ - return result; \ - } \ - if constexpr (byteNo < 9) \ - result &= ~(static_cast(0x80) << (7 * (byteNo - 1))); - PROTOBUF_READER_READ_VARINT_BYTE(2) - PROTOBUF_READER_READ_VARINT_BYTE(3) - PROTOBUF_READER_READ_VARINT_BYTE(4) - PROTOBUF_READER_READ_VARINT_BYTE(5) - PROTOBUF_READER_READ_VARINT_BYTE(6) - PROTOBUF_READER_READ_VARINT_BYTE(7) - PROTOBUF_READER_READ_VARINT_BYTE(8) - PROTOBUF_READER_READ_VARINT_BYTE(9) - PROTOBUF_READER_READ_VARINT_BYTE(10) + in.readStrict(c); \ + ++cursor; \ + if constexpr ((byteNo) < 10) \ + { \ + result |= static_cast(static_cast(c)) << (7 * ((byteNo) - 1)); \ + if (likely(!(c & 0x80))) \ + return result; \ + } \ + else \ + { \ + if (likely(c == 1)) \ + return result; \ + } \ + if constexpr ((byteNo) < 9) \ + result &= ~(static_cast(0x80) << (7 * ((byteNo) - 1))); \ + } while (false) + + PROTOBUF_READER_READ_VARINT_BYTE(2); + PROTOBUF_READER_READ_VARINT_BYTE(3); + PROTOBUF_READER_READ_VARINT_BYTE(4); + PROTOBUF_READER_READ_VARINT_BYTE(5); + PROTOBUF_READER_READ_VARINT_BYTE(6); + PROTOBUF_READER_READ_VARINT_BYTE(7); + PROTOBUF_READER_READ_VARINT_BYTE(8); + PROTOBUF_READER_READ_VARINT_BYTE(9); + PROTOBUF_READER_READ_VARINT_BYTE(10); + #undef PROTOBUF_READER_READ_VARINT_BYTE throwUnknownFormat(); @@ -307,28 +312,32 @@ void ProtobufReader::SimpleReader::ignoreVarint() char c; #define PROTOBUF_READER_IGNORE_VARINT_BYTE(byteNo) \ - in.readStrict(c); \ - ++cursor; \ - if constexpr (byteNo < 10) \ + do \ { \ - if (likely(!(c & 0x80))) \ - return; \ - } \ - else \ - { \ - if (likely(c == 1)) \ - return; \ - } - PROTOBUF_READER_IGNORE_VARINT_BYTE(1) - PROTOBUF_READER_IGNORE_VARINT_BYTE(2) - PROTOBUF_READER_IGNORE_VARINT_BYTE(3) - PROTOBUF_READER_IGNORE_VARINT_BYTE(4) - PROTOBUF_READER_IGNORE_VARINT_BYTE(5) - PROTOBUF_READER_IGNORE_VARINT_BYTE(6) - PROTOBUF_READER_IGNORE_VARINT_BYTE(7) - PROTOBUF_READER_IGNORE_VARINT_BYTE(8) - PROTOBUF_READER_IGNORE_VARINT_BYTE(9) - PROTOBUF_READER_IGNORE_VARINT_BYTE(10) + in.readStrict(c); \ + ++cursor; \ + if constexpr ((byteNo) < 10) \ + { \ + if (likely(!(c & 0x80))) \ + return; \ + } \ + else \ + { \ + if (likely(c == 1)) \ + return; \ + } \ + } while (false) + + PROTOBUF_READER_IGNORE_VARINT_BYTE(1); + PROTOBUF_READER_IGNORE_VARINT_BYTE(2); + PROTOBUF_READER_IGNORE_VARINT_BYTE(3); + PROTOBUF_READER_IGNORE_VARINT_BYTE(4); + PROTOBUF_READER_IGNORE_VARINT_BYTE(5); + PROTOBUF_READER_IGNORE_VARINT_BYTE(6); + PROTOBUF_READER_IGNORE_VARINT_BYTE(7); + PROTOBUF_READER_IGNORE_VARINT_BYTE(8); + PROTOBUF_READER_IGNORE_VARINT_BYTE(9); + PROTOBUF_READER_IGNORE_VARINT_BYTE(10); #undef PROTOBUF_READER_IGNORE_VARINT_BYTE throwUnknownFormat(); @@ -846,7 +855,7 @@ private: std::unique_ptr ProtobufReader::createConverter( \ const google::protobuf::FieldDescriptor * field) \ { \ - return std::make_unique>(simple_reader, field); \ + return std::make_unique>(simple_reader, field); /* NOLINT */ \ } PROTOBUF_READER_CREATE_CONVERTER_SPECIALIZATION_FOR_NUMBERS(google::protobuf::FieldDescriptor::TYPE_INT32, Int64); PROTOBUF_READER_CREATE_CONVERTER_SPECIALIZATION_FOR_NUMBERS(google::protobuf::FieldDescriptor::TYPE_SINT32, Int64); diff --git a/dbms/src/Functions/GeoUtils.cpp b/dbms/src/Functions/GeoUtils.cpp index 488a102e208..bbd942a9b0d 100644 --- a/dbms/src/Functions/GeoUtils.cpp +++ b/dbms/src/Functions/GeoUtils.cpp @@ -132,17 +132,17 @@ inline std::tuple split(const Encoded & combined, uint8_t prec lat.fill(0); lon.fill(0); - uint8_t i = 0; + size_t i = 0; for (; i < precision * BITS_PER_SYMBOL - 1; i += 2) { // longitude is even bits - lon[i/2] = combined[i]; - lat[i/2] = combined[i + 1]; + lon[i / 2] = combined[i]; + lat[i / 2] = combined[i + 1]; } // precision is even, read the last bit as lat. if (precision & 0x1) { - lon[i/2] = combined[precision * BITS_PER_SYMBOL - 1]; + lon[i / 2] = combined[precision * BITS_PER_SYMBOL - 1]; } return std::tie(lon, lat); @@ -152,7 +152,7 @@ inline void base32Encode(const Encoded & binary, uint8_t precision, char * out) { extern const char geohash_base32_encode_lookup_table[32]; - for (uint8_t i = 0; i < precision * BITS_PER_SYMBOL; i += BITS_PER_SYMBOL) + for (size_t i = 0; i < precision * BITS_PER_SYMBOL; i += BITS_PER_SYMBOL) { uint8_t v = binary[i]; v <<= 1; diff --git a/dbms/src/Functions/array/arrayUniq.cpp b/dbms/src/Functions/array/arrayUniq.cpp index d5aedb20883..d94efc47970 100644 --- a/dbms/src/Functions/array/arrayUniq.cpp +++ b/dbms/src/Functions/array/arrayUniq.cpp @@ -214,7 +214,7 @@ void FunctionArrayUniq::executeMethodImpl( for (ColumnArray::Offset j = prev_off; j < off; ++j) { if constexpr (has_null_map) - { + { // NOLINT if ((*null_map)[j]) { found_null = true; diff --git a/dbms/src/Functions/trim.cpp b/dbms/src/Functions/trim.cpp index 46f69530005..f674afbd310 100644 --- a/dbms/src/Functions/trim.cpp +++ b/dbms/src/Functions/trim.cpp @@ -79,14 +79,14 @@ private: const char * char_end = char_data + size; if constexpr (mode::trim_left) - { + { // NOLINT const char * found = find_first_not_symbols<' '>(char_data, char_end); size_t num_chars = found - char_data; char_data += num_chars; } if constexpr (mode::trim_right) - { + { // NOLINT const char * found = find_last_not_symbols_or_null<' '>(char_data, char_end); if (found) char_end = found + 1; diff --git a/dbms/src/IO/parseDateTimeBestEffort.cpp b/dbms/src/IO/parseDateTimeBestEffort.cpp index 24d05f73aa0..6e747b13b3f 100644 --- a/dbms/src/IO/parseDateTimeBestEffort.cpp +++ b/dbms/src/IO/parseDateTimeBestEffort.cpp @@ -68,7 +68,7 @@ inline void readDecimalNumber(T & res, const char * src) template inline void readDecimalNumber(T & res, size_t num_digits, const char * src) { -#define READ_DECIMAL_NUMBER(N) res *= common::exp10_i32(N); readDecimalNumber(res, src); src += N; num_digits -= N; break +#define READ_DECIMAL_NUMBER(N) do { res *= common::exp10_i32(N); readDecimalNumber(res, src); src += (N); num_digits -= (N); } while (false) while (num_digits) { @@ -77,7 +77,7 @@ inline void readDecimalNumber(T & res, size_t num_digits, const char * src) case 3: READ_DECIMAL_NUMBER(3); break; case 2: READ_DECIMAL_NUMBER(2); break; case 1: READ_DECIMAL_NUMBER(1); break; - default: READ_DECIMAL_NUMBER(4); + default: READ_DECIMAL_NUMBER(4); break; } } #undef DECIMAL_NUMBER_CASE diff --git a/dbms/src/Interpreters/Aggregator.cpp b/dbms/src/Interpreters/Aggregator.cpp index 0ab4949371b..1a40b7cefc3 100644 --- a/dbms/src/Interpreters/Aggregator.cpp +++ b/dbms/src/Interpreters/Aggregator.cpp @@ -82,8 +82,8 @@ void AggregatedDataVariants::convertToTwoLevel() { #define M(NAME) \ case Type::NAME: \ - NAME ## _two_level = std::make_unique(*NAME); \ - NAME.reset(); \ + NAME ## _two_level = std::make_unique(*(NAME)); \ + (NAME).reset(); \ type = Type::NAME ## _two_level; \ break; diff --git a/dbms/src/Interpreters/InterpreterKillQueryQuery.cpp b/dbms/src/Interpreters/InterpreterKillQueryQuery.cpp index f384e005e3c..81a093f4eae 100644 --- a/dbms/src/Interpreters/InterpreterKillQueryQuery.cpp +++ b/dbms/src/Interpreters/InterpreterKillQueryQuery.cpp @@ -102,7 +102,7 @@ static QueryDescriptors extractQueriesExceptMeAndCheckAccess(const Block & proce res.emplace_back(std::move(query_id), std::move(query_user), i, false); } - if (res.empty() && !query_user.empty()) + if (res.empty() && !query_user.empty()) // NOLINT throw Exception("User " + my_client.current_user + " attempts to kill query created by " + query_user, ErrorCodes::ACCESS_DENIED); return res; diff --git a/dbms/src/Interpreters/SetVariants.cpp b/dbms/src/Interpreters/SetVariants.cpp index 56f2ff04230..52f54d2442a 100644 --- a/dbms/src/Interpreters/SetVariants.cpp +++ b/dbms/src/Interpreters/SetVariants.cpp @@ -23,7 +23,7 @@ void SetVariantsTemplate::init(Type type_) case Type::EMPTY: break; #define M(NAME) \ - case Type::NAME: NAME = std::make_unique(); break; + case Type::NAME: (NAME) = std::make_unique(); break; APPLY_FOR_SET_VARIANTS(M) #undef M } @@ -37,7 +37,7 @@ size_t SetVariantsTemplate::getTotalRowCount() const case Type::EMPTY: return 0; #define M(NAME) \ - case Type::NAME: return NAME->data.size(); + case Type::NAME: return (NAME)->data.size(); APPLY_FOR_SET_VARIANTS(M) #undef M } @@ -53,7 +53,7 @@ size_t SetVariantsTemplate::getTotalByteCount() const case Type::EMPTY: return 0; #define M(NAME) \ - case Type::NAME: return NAME->data.getBufferSizeInBytes(); + case Type::NAME: return (NAME)->data.getBufferSizeInBytes(); APPLY_FOR_SET_VARIANTS(M) #undef M } diff --git a/dbms/src/Interpreters/tests/hash_map_string_2.cpp b/dbms/src/Interpreters/tests/hash_map_string_2.cpp index 32b723c1187..8e13ee46e6d 100644 --- a/dbms/src/Interpreters/tests/hash_map_string_2.cpp +++ b/dbms/src/Interpreters/tests/hash_map_string_2.cpp @@ -54,16 +54,16 @@ struct STRUCT : public StringRef {}; \ namespace ZeroTraits \ { \ template <> \ - inline bool check(STRUCT x) { return 0 == x.size; } \ + inline bool check(STRUCT x) { return 0 == x.size; } /* NOLINT */ \ \ template <> \ - inline void set(STRUCT & x) { x.size = 0; } \ + inline void set(STRUCT & x) { x.size = 0; } /* NOLINT */ \ } \ \ template <> \ struct DefaultHash \ { \ - size_t operator() (STRUCT x) const \ + size_t operator() (STRUCT x) const /* NOLINT */ \ { \ return CityHash_v1_0_2::CityHash64(x.data, x.size); \ } \ diff --git a/dbms/src/Interpreters/tests/hash_map_string_3.cpp b/dbms/src/Interpreters/tests/hash_map_string_3.cpp index 62ed0584d3f..cc21129a6a6 100644 --- a/dbms/src/Interpreters/tests/hash_map_string_3.cpp +++ b/dbms/src/Interpreters/tests/hash_map_string_3.cpp @@ -57,10 +57,10 @@ struct STRUCT : public StringRef {}; \ namespace ZeroTraits \ { \ template <> \ - inline bool check(STRUCT x) { return nullptr == x.data; } \ + inline bool check(STRUCT x) { return nullptr == x.data; } /* NOLINT */ \ \ template <> \ - inline void set(STRUCT & x) { x.data = nullptr; } \ + inline void set(STRUCT & x) { x.data = nullptr; } /* NOLINT */ \ } \ \ template <> \ diff --git a/dbms/src/Parsers/ASTTablesInSelectQuery.cpp b/dbms/src/Parsers/ASTTablesInSelectQuery.cpp index b085f5a28ae..60cb0475be7 100644 --- a/dbms/src/Parsers/ASTTablesInSelectQuery.cpp +++ b/dbms/src/Parsers/ASTTablesInSelectQuery.cpp @@ -11,7 +11,7 @@ do \ { \ if (member) \ { \ - res->member = member->clone(); \ + res->member = (member)->clone(); \ res->children.push_back(res->member); \ } \ } \ diff --git a/dbms/src/Parsers/CommonParsers.cpp b/dbms/src/Parsers/CommonParsers.cpp index ddbf1b17966..47868f5df48 100644 --- a/dbms/src/Parsers/CommonParsers.cpp +++ b/dbms/src/Parsers/CommonParsers.cpp @@ -50,7 +50,7 @@ bool ParserKeyword::parseImpl(Pos & pos, ASTPtr & /*node*/, Expected & expected) if (word_length != pos->size()) return false; - if (strncasecmp(pos->begin, current_word, word_length)) + if (0 != strncasecmp(pos->begin, current_word, word_length)) return false; ++pos; diff --git a/dbms/src/Processors/ForkProcessor.cpp b/dbms/src/Processors/ForkProcessor.cpp index 913e7c2d1c7..7fa21c4236d 100644 --- a/dbms/src/Processors/ForkProcessor.cpp +++ b/dbms/src/Processors/ForkProcessor.cpp @@ -65,7 +65,7 @@ ForkProcessor::Status ForkProcessor::prepare() { ++num_processed_outputs; if (num_processed_outputs == num_active_outputs) - output.push(std::move(data)); /// Can push because no full or unneeded outputs. + output.push(std::move(data)); // NOLINT Can push because no full or unneeded outputs. else output.push(data.clone()); } diff --git a/dbms/src/Storages/Kafka/StorageKafka.cpp b/dbms/src/Storages/Kafka/StorageKafka.cpp index 1c988840abf..002f072f004 100644 --- a/dbms/src/Storages/Kafka/StorageKafka.cpp +++ b/dbms/src/Storages/Kafka/StorageKafka.cpp @@ -431,7 +431,7 @@ void registerStorageKafka(StorageFactory & factory) // Check arguments and settings #define CHECK_KAFKA_STORAGE_ARGUMENT(ARG_NUM, PAR_NAME) \ /* One of the four required arguments is not specified */ \ - if (args_count < ARG_NUM && ARG_NUM <= 4 && \ + if (args_count < (ARG_NUM) && (ARG_NUM) <= 4 && \ !kafka_settings.PAR_NAME.changed) \ { \ throw Exception( \ @@ -442,7 +442,7 @@ void registerStorageKafka(StorageFactory & factory) /* The same argument is given in two places */ \ if (has_settings && \ kafka_settings.PAR_NAME.changed && \ - args_count >= ARG_NUM) \ + args_count >= (ARG_NUM)) \ { \ throw Exception( \ "The argument №" #ARG_NUM " of storage Kafka " \ diff --git a/dbms/src/Storages/LiveView/StorageLiveView.cpp b/dbms/src/Storages/LiveView/StorageLiveView.cpp index 1faa5e04dc9..29eb896bcee 100644 --- a/dbms/src/Storages/LiveView/StorageLiveView.cpp +++ b/dbms/src/Storages/LiveView/StorageLiveView.cpp @@ -323,7 +323,7 @@ ASTPtr StorageLiveView::getInnerBlocksQuery() /// Rewrite inner query with right aliases for JOIN. /// It cannot be done in constructor or startup() because InterpreterSelectQuery may access table, /// which is not loaded yet during server startup, so we do it lazily - InterpreterSelectQuery(inner_blocks_query, *live_view_context, SelectQueryOptions().modify().analyze()); + InterpreterSelectQuery(inner_blocks_query, *live_view_context, SelectQueryOptions().modify().analyze()); // NOLINT auto table_id = getStorageID(); extractDependentTable(inner_blocks_query, global_context, table_id.table_name, inner_subquery); } diff --git a/dbms/src/Storages/MergeTree/KeyCondition.cpp b/dbms/src/Storages/MergeTree/KeyCondition.cpp index c74ca41f054..e994d254958 100644 --- a/dbms/src/Storages/MergeTree/KeyCondition.cpp +++ b/dbms/src/Storages/MergeTree/KeyCondition.cpp @@ -516,7 +516,7 @@ void KeyCondition::traverseAST(const ASTPtr & node, const Context & context, Blo * - in this case `n - 1` elements are added (where `n` is the number of arguments). */ if (i != 0 || element.function == RPNElement::FUNCTION_NOT) - rpn.emplace_back(std::move(element)); + rpn.emplace_back(element); } return; diff --git a/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp b/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp index 2b02aad8970..0b87b241d85 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp @@ -1091,8 +1091,10 @@ Pipes MergeTreeDataSelectExecutor::spreadMarkRangesAmongStreamsFinal( { auto merged_processor = std::make_shared(header, pipes.size(), sort_description, max_block_size); - pipes.emplace_back(std::move(pipes), std::move(merged_processor)); - break; + Pipe pipe(std::move(pipes), std::move(merged_processor)); + pipes = Pipes(); + pipes.emplace_back(std::move(pipe)); + return pipes; } case MergeTreeData::MergingParams::Collapsing: diff --git a/dbms/src/Storages/MergeTree/MergeTreeSettings.cpp b/dbms/src/Storages/MergeTree/MergeTreeSettings.cpp index 93f5ff20045..5c4113c1565 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeSettings.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeSettings.cpp @@ -70,7 +70,7 @@ void MergeTreeSettings::loadFromQuery(ASTStorage & storage_def) if (std::find_if(changes.begin(), changes.end(), \ [](const SettingChange & c) { return c.name == #NAME; }) \ == changes.end()) \ - changes.push_back(SettingChange{#NAME, NAME.value}); + changes.push_back(SettingChange{#NAME, (NAME).value}); APPLY_FOR_IMMUTABLE_MERGE_TREE_SETTINGS(ADD_IF_ABSENT) #undef ADD_IF_ABSENT diff --git a/dbms/src/Storages/StorageInMemoryMetadata.cpp b/dbms/src/Storages/StorageInMemoryMetadata.cpp index 86c6551896a..a05872234de 100644 --- a/dbms/src/Storages/StorageInMemoryMetadata.cpp +++ b/dbms/src/Storages/StorageInMemoryMetadata.cpp @@ -35,11 +35,13 @@ StorageInMemoryMetadata::StorageInMemoryMetadata(const StorageInMemoryMetadata & StorageInMemoryMetadata & StorageInMemoryMetadata::operator=(const StorageInMemoryMetadata & other) { + if (this == &other) + return *this; + columns = other.columns; indices = other.indices; constraints = other.constraints; - if (other.partition_by_ast) partition_by_ast = other.partition_by_ast->clone(); else