clang-tidy, part 5

.clang-tidy

@@ -123,6 +123,42 @@ Checks: '-*,
     
     hicpp-exception-baseclass,
     
+    clang-analyzer-core.CallAndMessage,
+    clang-analyzer-core.DivideZero,
+    clang-analyzer-core.NonNullParamChecker,
+    clang-analyzer-core.NullDereference,
+    clang-analyzer-core.StackAddressEscape,
+    clang-analyzer-core.UndefinedBinaryOperatorResult,
+    clang-analyzer-core.VLASize,
+    clang-analyzer-core.uninitialized.ArraySubscript,
+    clang-analyzer-core.uninitialized.Assign,
+    clang-analyzer-core.uninitialized.Branch,
+    clang-analyzer-core.uninitialized.CapturedBlockVariable,
+    clang-analyzer-core.uninitialized.UndefReturn,
+    clang-analyzer-cplusplus.InnerPointer,
+    clang-analyzer-cplusplus.NewDelete,
+    clang-analyzer-cplusplus.NewDeleteLeaks,
+    clang-analyzer-cplusplus.PlacementNewChecker,
+    clang-analyzer-cplusplus.SelfAssignment,
+    clang-analyzer-deadcode.DeadStores,
+    clang-analyzer-optin.cplusplus.VirtualCall,
+    clang-analyzer-security.insecureAPI.UncheckedReturn,
+    clang-analyzer-security.insecureAPI.bcmp,
+    clang-analyzer-security.insecureAPI.bcopy,
+    clang-analyzer-security.insecureAPI.bzero,
+    clang-analyzer-security.insecureAPI.getpw,
+    clang-analyzer-security.insecureAPI.gets,
+    clang-analyzer-security.insecureAPI.mkstemp,
+    clang-analyzer-security.insecureAPI.mktemp,
+    clang-analyzer-security.insecureAPI.rand,
+    clang-analyzer-security.insecureAPI.strcpy,
+    clang-analyzer-unix.Malloc,
+    clang-analyzer-unix.MallocSizeof,
+    clang-analyzer-unix.MismatchedDeallocator,
+    clang-analyzer-unix.Vfork,
+    clang-analyzer-unix.cstring.BadSizeArg,
+    clang-analyzer-unix.cstring.NullArg,
+
     boost-use-to-string,
 '
 WarningsAsErrors: '*'

base/common/JSON.cpp

@@ -215,7 +215,7 @@ JSON::ElementType JSON::getType() const
 
 void JSON::checkPos(Pos pos) const
 {
-    if (pos >= ptr_end)
+    if (pos >= ptr_end || ptr_begin == nullptr)
         throw JSONException("JSON: unexpected end of data.");
 }
 

base/common/LineReader.cpp

@@ -30,7 +30,7 @@ void trim(String & s)
 bool hasInputData()
 {
     timeval timeout = {0, 0};
-    fd_set fds;
+    fd_set fds{};
     FD_ZERO(&fds);
     FD_SET(STDIN_FILENO, &fds);
     return select(1, &fds, nullptr, nullptr, &timeout) == 1;

dbms/src/Common/MemoryTracker.cpp

@@ -93,7 +93,7 @@ void MemoryTracker::alloc(Int64 size)
         free(size);
 
         /// Prevent recursion. Exception::ctor -> std::string -> new[] -> MemoryTracker::alloc
-        auto untrack_lock = blocker.cancel();
+        auto untrack_lock = blocker.cancel(); // NOLINT
 
         std::stringstream message;
         message << "Memory tracker";
@@ -118,7 +118,7 @@ void MemoryTracker::alloc(Int64 size)
         free(size);
 
         /// Prevent recursion. Exception::ctor -> std::string -> new[] -> MemoryTracker::alloc
-        auto untrack_lock = blocker.cancel();
+        auto untrack_lock = blocker.cancel(); // NOLINT
 
         std::stringstream message;
         message << "Memory limit";

dbms/src/Common/NamePrompter.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <Core/Types.h>
+#include <Common/PODArray.h>
 
 #include <algorithm>
 #include <cctype>
@@ -32,15 +33,7 @@ private:
         size_t m = lhs.size();
         size_t n = rhs.size();
 
-        static constexpr size_t small_buffer_size = 64;
+        PODArrayWithStackMemory<size_t, 64> row(n + 1);
-        size_t small_buffer[small_buffer_size];
-        std::unique_ptr<size_t[]> alloc_buffer;
-        size_t * row = small_buffer;
-        if (n + 1 > small_buffer_size)
-        {
-            row = new size_t[n + 1];
-            alloc_buffer.reset(row);
-        }
 
         for (size_t i = 1; i <= n; ++i)
             row[i] = i;

dbms/src/Common/ZooKeeper/ZooKeeper.cpp

@@ -5,9 +5,9 @@
 
 #include <random>
 #include <functional>
-#include <boost/algorithm/string.hpp>
 
 #include <common/logger_useful.h>
+#include <common/find_symbols.h>
 #include <Common/StringUtils/StringUtils.h>
 #include <Common/PODArray.h>
 #include <Common/thread_local_rng.h>
@@ -61,7 +61,21 @@ void ZooKeeper::init(const std::string & implementation, const std::string & hos
             throw KeeperException("No addresses passed to ZooKeeper constructor.", Coordination::ZBADARGUMENTS);
 
         std::vector<std::string> addresses_strings;
-        boost::split(addresses_strings, hosts, boost::is_any_of(","));
+
+        const char * pos = hosts.data();
+        const char * end = pos + hosts.size();
+        while (pos < end)
+        {
+            const char * comma_or_end = find_first_symbols<','>(pos, end);
+
+            addresses_strings.emplace_back(pos, comma_or_end);
+
+            if (comma_or_end < end)
+                pos = comma_or_end + 1;
+            else
+                pos = comma_or_end;
+        }
+
         Coordination::ZooKeeper::Addresses addresses;
         addresses.reserve(addresses_strings.size());
 

dbms/src/Common/malloc.cpp

@@ -30,15 +30,15 @@ static void dummyFunctionForInterposing() __attribute__((used));
 static void dummyFunctionForInterposing()
 {
     void* dummy;
-    /// Suppression for PVS-Studio.
+    /// Suppression for PVS-Studio and clang-tidy.
-    free(nullptr); // -V575
+    free(nullptr); // -V575 NOLINT
-    ignore(malloc(0)); // -V575
+    ignore(malloc(0)); // -V575 NOLINT
-    ignore(calloc(0, 0)); // -V575
+    ignore(calloc(0, 0)); // -V575 NOLINT
-    ignore(realloc(nullptr, 0)); // -V575
+    ignore(realloc(nullptr, 0)); // -V575 NOLINT
-    ignore(posix_memalign(&dummy, 0, 0)); // -V575
+    ignore(posix_memalign(&dummy, 0, 0)); // -V575 NOLINT
-    ignore(aligned_alloc(0, 0)); // -V575
+    ignore(aligned_alloc(0, 0)); // -V575 NOLINT
-    ignore(valloc(0)); // -V575
+    ignore(valloc(0)); // -V575 NOLINT
-    ignore(memalign(0, 0)); // -V575
+    ignore(memalign(0, 0)); // -V575 NOLINT
-    ignore(pvalloc(0)); // -V575
+    ignore(pvalloc(0)); // -V575 NOLINT
 }
 #endif

dbms/src/Common/parseAddress.cpp

@@ -19,7 +19,7 @@ std::pair<std::string, UInt16> parseAddress(const std::string & str, UInt16 defa
 
     const char * begin = str.data();
     const char * end = begin + str.size();
-    const char * port = end;
+    const char * port = end; // NOLINT
 
     if (begin[0] == '[')
     {

dbms/src/Compression/CompressedReadBufferBase.cpp

@@ -1,7 +1,8 @@
 #include "CompressedReadBufferBase.h"
 
 #include <vector>
-#include <string.h>
+#include <cstring>
+#include <cassert>
 #include <city.h>
 #include <Common/PODArray.h>
 #include <Common/ProfileEvents.h>
@@ -118,6 +119,8 @@ size_t CompressedReadBufferBase::readCompressedData(size_t & size_decompressed,
     size_compressed_without_checksum = ICompressionCodec::readCompressedBlockSize(own_compressed_buffer.data());
     size_decompressed = ICompressionCodec::readDecompressedBlockSize(own_compressed_buffer.data());
 
+    assert(size_decompressed > 0);
+
     if (size_compressed_without_checksum > DBMS_MAX_COMPRESSED_SIZE)
         throw Exception("Too large size_compressed_without_checksum: "
                         + toString(size_compressed_without_checksum)

dbms/src/Compression/CompressedReadBufferFromFile.cpp

@@ -1,3 +1,5 @@
+#include <cassert>
+
 #include "CompressedReadBufferFromFile.h"
 
 #include <Compression/CompressionInfo.h>
@@ -16,12 +18,14 @@ namespace ErrorCodes
 
 bool CompressedReadBufferFromFile::nextImpl()
 {
-    size_t size_decompressed;
+    size_t size_decompressed = 0;
     size_t size_compressed_without_checksum;
     size_compressed = readCompressedData(size_decompressed, size_compressed_without_checksum);
     if (!size_compressed)
         return false;
 
+    assert(size_decompressed > 0);
+
     memory.resize(size_decompressed + codec->getAdditionalSizeAtTheEndOfBuffer());
     working_buffer = Buffer(memory.data(), &memory[size_decompressed]);
 

dbms/src/DataStreams/CollapsingFinalBlockInputStream.h

@@ -5,6 +5,8 @@
 #include <Columns/ColumnsNumber.h>
 #include <Common/typeid_cast.h>
 #include <queue>
+#include <cassert>
+
 
 namespace DB
 {
@@ -115,6 +117,8 @@ private:
 
         MergingBlockPtr & operator=(const MergingBlockPtr & rhs)
         {
+            assert(ptr != rhs.ptr);
+
             destroy();
             ptr = rhs.ptr;
             if (ptr)

dbms/src/Dictionaries/HashedDictionary.cpp

@@ -444,7 +444,7 @@ void HashedDictionary::addAttributeSize(const Attribute & attribute)
         bucket_count = map_ref->bucket_count();
 
         /** TODO: more accurate calculation */
-        bytes_allocated += sizeof(CollectionType<T>);
+        bytes_allocated += sizeof(SparseCollectionType<T>);
         bytes_allocated += bucket_count;
         bytes_allocated += map_ref->size() * (sizeof(Key) + sizeof(T));
     }

dbms/src/IO/BitHelpers.h

@@ -4,7 +4,8 @@
 #include <Common/BitHelpers.h>
 #include <Common/Exception.h>
 
-#include <string.h>
+#include <cstring>
+#include <cassert>
 
 #if defined(__OpenBSD__) || defined(__FreeBSD__)
 #   include <sys/endian.h>
@@ -115,6 +116,8 @@ private:
     template <GetBitsMode mode>
     inline UInt64 getBitsFromBitBuffer(UInt8 bits_to_read)
     {
+        assert(bits_to_read > 0);
+
         // push down the high-bits
         const UInt64 result = static_cast<UInt64>(bits_buffer >> (sizeof(bits_buffer) * 8 - bits_to_read));
 
@@ -186,6 +189,8 @@ public:
     // write `bits_to_write` low-bits of `value` to the buffer
     inline void writeBits(UInt8 bits_to_write, UInt64 value)
     {
+        assert(bits_to_write > 0);
+
         UInt32 capacity = BIT_BUFFER_SIZE - bits_count;
         if (capacity < bits_to_write)
         {

dbms/src/IO/ReadBufferAIO.h

@@ -25,7 +25,7 @@ namespace DB
 
 /** Class for asynchronous data reading.
   */
-class ReadBufferAIO : public ReadBufferFromFileBase
+class ReadBufferAIO final : public ReadBufferFromFileBase
 {
 public:
     ReadBufferAIO(const std::string & filename_, size_t buffer_size_ = DBMS_DEFAULT_BUFFER_SIZE, int flags_ = -1,

dbms/src/IO/WriteBufferAIO.h

@@ -24,7 +24,7 @@ namespace DB
 
 /** Class for asynchronous data writing.
   */
-class WriteBufferAIO : public WriteBufferFromFileBase
+class WriteBufferAIO final : public WriteBufferFromFileBase
 {
 public:
     WriteBufferAIO(const std::string & filename_, size_t buffer_size_ = DBMS_DEFAULT_BUFFER_SIZE, int flags_ = -1, mode_t mode_ = 0666,

dbms/src/IO/WriteBufferFromHTTPServerResponse.h

@@ -41,7 +41,7 @@ namespace DB
 /// Also this class write and flush special X-ClickHouse-Progress HTTP headers
 ///  if no data was sent at the time of progress notification.
 /// This allows to implement progress bar in HTTP clients.
-class WriteBufferFromHTTPServerResponse : public BufferWithOwnMemory<WriteBuffer>
+class WriteBufferFromHTTPServerResponse final : public BufferWithOwnMemory<WriteBuffer>
 {
 private:
     Poco::Net::HTTPServerRequest & request;

dbms/src/IO/WriteBufferFromVector.h

@@ -65,7 +65,7 @@ public:
         set(reinterpret_cast<Position>(vector.data() + old_size), (size - old_size) * sizeof(typename VectorType::value_type));
     }
 
-    void finalize() override
+    void finalize() override final
     {
         if (is_finished)
             return;

dbms/src/IO/WriteBufferValidUTF8.h

@@ -11,7 +11,7 @@ namespace DB
     * If the valid UTF-8 is already written, it works faster.
     * Note: before using the resulting string, destroy this object.
     */
-class WriteBufferValidUTF8 : public BufferWithOwnMemory<WriteBuffer>
+class WriteBufferValidUTF8 final : public BufferWithOwnMemory<WriteBuffer>
 {
 private:
     WriteBuffer & output_buffer;

dbms/src/IO/tests/gtest_DateTime64_parsing_and_writing.cpp

@@ -37,7 +37,7 @@ TEST_P(DateTime64StringParseTest, readDateTime64Text)
     const auto & param = GetParam();
     ReadBufferFromMemory read_buffer(param.string.data(), param.string.size());
 
-    DateTime64 actual;
+    DateTime64 actual{};
     EXPECT_TRUE(tryReadDateTime64Text(actual, param.scale, read_buffer, param.timezone));
 
     EXPECT_EQ(param.dt64, actual);