Repository: randombit/botan
Pull request #2898 (2022-02-04 22:24:18)

Add clang-tidy script, apply fixes for clang-analzyer warnings


Patch:
diff --git a/src/build-data/compile_commands.json.in b/src/build-data/compile_commands.json.in
index 8495bec0f..fa2f4a3e8 100644
--- a/src/build-data/compile_commands.json.in
+++ b/src/build-data/compile_commands.json.in
@@ -2,7 +2,21 @@
 %{for lib_build_info}
     { "directory": "%{abs_root_dir}",
       "command": "%{cxx} %{lib_flags} %{cxx_abi_flags} %{cc_lang_flags} %{os_feature_macros} %{isa_flags} %{include_paths} %{dash_c} %{src} %{dash_o}%{obj}",
-      "file": %{src}
+      "file": "%{src}"
+    },
+%{endfor}
+
+%{for cli_build_info}
+    { "directory": "%{abs_root_dir}",
+      "command": "%{cxx} %{cxx_abi_flags} %{cc_lang_flags} %{os_feature_macros} %{isa_flags} %{include_paths} %{dash_c} %{src} %{dash_o}%{obj}",
+      "file": "%{src}"
+    },
+%{endfor}
+
+%{for test_build_info}
+    { "directory": "%{abs_root_dir}",
+      "command": "%{cxx} %{cxx_abi_flags} %{cc_lang_flags} %{os_feature_macros} %{isa_flags} %{include_paths} %{dash_c} %{src} %{dash_o}%{obj}",
+      "file": "%{src}"
     },
 %{endfor}
 ]
diff --git a/src/lib/hash/hash.cpp b/src/lib/hash/hash.cpp
index 194535ff6..f9955f54d 100644
--- a/src/lib/hash/hash.cpp
+++ b/src/lib/hash/hash.cpp
@@ -251,11 +251,11 @@ std::unique_ptr<HashFunction> HashFunction::create(const std::string& algo_spec,
 #if defined(BOTAN_HAS_STREEBOG)
    if(algo_spec == "Streebog-256")
       {
-      return std::make_unique<Streebog_256>();
+      return std::make_unique<Streebog>(256);
       }
    if(algo_spec == "Streebog-512")
       {
-      return std::make_unique<Streebog_512>();
+      return std::make_unique<Streebog>(512);
       }
 #endif
 
diff --git a/src/lib/hash/streebog/streebog.h b/src/lib/hash/streebog/streebog.h
index 523a20221..a40720070 100644
--- a/src/lib/hash/streebog/streebog.h
+++ b/src/lib/hash/streebog/streebog.h
@@ -16,7 +16,7 @@ namespace Botan {
 * Streebog (GOST R 34.11-2012)
 * RFC 6986
 */
-class Streebog : public HashFunction
+class Streebog final : public HashFunction
    {
    public:
       size_t output_length() const override { return m_output_bits / 8; }
@@ -46,25 +46,6 @@ class Streebog : public HashFunction
       secure_vector<uint64_t> m_S;
    };
 
-
-/**
-* Streebog-256
-*/
-class Streebog_256 final : public Streebog
-   {
-   public:
-      Streebog_256() : Streebog(256) {}
-   };
-
-/**
-* Streebog-512
-*/
-class Streebog_512 final : public Streebog
-   {
-   public:
-      Streebog_512() : Streebog(512) {}
-   };
-
 }
 
 #endif
diff --git a/src/lib/math/bigint/big_ops2.cpp b/src/lib/math/bigint/big_ops2.cpp
index df5437b72..db54a409d 100644
--- a/src/lib/math/bigint/big_ops2.cpp
+++ b/src/lib/math/bigint/big_ops2.cpp
@@ -77,6 +77,7 @@ BigInt& BigInt::mod_add(const BigInt& s, const BigInt& mod, secure_vector<word>&
 
    word borrow = bigint_sub3(&ws[0], mod.data(), mod_sw, s.data(), mod_sw);
    BOTAN_DEBUG_ASSERT(borrow == 0);
+   BOTAN_UNUSED(borrow);
 
    // Compute t - ws
    borrow = bigint_sub3(&ws[mod_sw], this->data(), mod_sw, &ws[0], mod_sw);
diff --git a/src/lib/modes/aead/eax/eax.h b/src/lib/modes/aead/eax/eax.h
index be888a900..ba0015c6a 100644
--- a/src/lib/modes/aead/eax/eax.h
+++ b/src/lib/modes/aead/eax/eax.h
@@ -24,7 +24,7 @@ class EAX_Mode : public AEAD_Mode
    public:
       void set_associated_data(const uint8_t ad[], size_t ad_len) override;
 
-      std::string name() const override;
+      std::string name() const override final;
 
       size_t update_granularity() const override;
 
diff --git a/src/lib/modes/aead/gcm/gcm.h b/src/lib/modes/aead/gcm/gcm.h
index 6a4a0c8b6..84eeb7491 100644
--- a/src/lib/modes/aead/gcm/gcm.h
+++ b/src/lib/modes/aead/gcm/gcm.h
@@ -26,7 +26,7 @@ class GCM_Mode : public AEAD_Mode
    public:
       void set_associated_data(const uint8_t ad[], size_t ad_len) override;
 
-      std::string name() const override;
+      std::string name() const override final;
 
       size_t update_granularity() const override;
 
@@ -34,7 +34,7 @@ class GCM_Mode : public AEAD_Mode
 
       bool valid_nonce_length(size_t len) const override;
 
-      size_t tag_size() const override { return m_tag_size; }
+      size_t tag_size() const override final { return m_tag_size; }
 
       void clear() override;
 
diff --git a/src/lib/modes/cbc/cbc.cpp b/src/lib/modes/cbc/cbc.cpp
index d4f44739e..8e454e315 100644
--- a/src/lib/modes/cbc/cbc.cpp
+++ b/src/lib/modes/cbc/cbc.cpp
@@ -22,7 +22,7 @@ CBC_Mode::CBC_Mode(std::unique_ptr<BlockCipher> cipher,
    if(m_padding && !m_padding->valid_blocksize(m_block_size))
       throw Invalid_Argument("Padding " + m_padding->name() +
                              " cannot be used with " +
-                             cipher->name() + "/CBC");
+                             m_cipher->name() + "/CBC");
    }
 
 void CBC_Mode::clear()
diff --git a/src/lib/modes/xts/xts.cpp b/src/lib/modes/xts/xts.cpp
index f8f36d522..97279dca2 100644
--- a/src/lib/modes/xts/xts.cpp
+++ b/src/lib/modes/xts/xts.cpp
@@ -18,7 +18,7 @@ XTS_Mode::XTS_Mode(std::unique_ptr<BlockCipher> cipher) :
    {
    if(poly_double_supported_size(m_cipher_block_size) == false)
       {
-      throw Invalid_Argument("Cannot use " + cipher->name() + " with XTS");
+      throw Invalid_Argument("Cannot use " + m_cipher->name() + " with XTS");
       }
 
    m_tweak_cipher = m_cipher->new_object();
diff --git a/src/scripts/run_clang_tidy.py b/src/scripts/run_clang_tidy.py
new file mode 100755
index 000000000..0652f8651
--- /dev/null
+++ b/src/scripts/run_clang_tidy.py
@@ -0,0 +1,118 @@
+#!/usr/bin/python
+
+import subprocess
+import sys
+import json
+import optparse
+import os
+import multiprocessing
+import re
+from multiprocessing.pool import ThreadPool
+
+enabled_checks = [
+    'clang-analyzer-*',
+    #'performance-*',
+    #'bugprone-*',
+    #'cert-*',
+    #'cppcoreguidelines-*',
+    #'hicpp-*',
+    #'modernize-*',
+    #'portability-*',
+    #'readability-*',
+    #'readability-container-size-empty'
+]
+
+disabled_checks = [
+    '*-array-to-pointer-decay',
+    '*-avoid-c-arrays',
+    '*-braces-around-statements', # should fix
+    '*-magic-numbers',
+    '*-no-array-decay',
+    '-*else-after-return',
+    'bugprone-easily-swappable-parameters',
+    'cppcoreguidelines-pro-bounds-pointer-arithmetic',
+    'modernize-pass-by-value',
+    'modernize-use-trailing-return-type',
+    'readability-function-cognitive-complexity', # bogus
+]
+
+def create_check_option(enabled, disabled):
+    return ','.join(enabled) + ',' + ','.join(['-' + d for d in disabled])
+
+def load_compile_commands(build_dir):
+    compile_commands_file = os.path.join(build_dir, 'compile_commands.json')
+
+    compile_commands = open(compile_commands_file).read()
+    # hack for configure.py generating invalid JSON
+    compile_commands = re.sub(r',\n+]', '\n]', compile_commands)
+    return (compile_commands_file, json.loads(compile_commands))
+
+def run_clang_tidy(compile_commands_file,
+                   check_config,
+                   source_file,
+                   options):
+
+    cmdline = ['clang-tidy',
+               '--quiet',
+               '-checks=%s' % (check_config),
+               '-p', compile_commands_file]
+
+    if options.fixit:
+        cmdline.append('-fix')
+
+    cmdline.append(source_file)
+
+    clang_tidy = subprocess.Popen(cmdline,
+                                  stdout=subprocess.PIPE,
+                                  stderr=subprocess.PIPE)
+
+    (stdout, stderr) = clang_tidy.communicate()
+
+    stdout = stdout.decode('utf8')
+    # stderr discarded
+
+    if options.verbose:
+        print(source_file)
+    if stdout != "":
+        print(stdout)
+
+def main(args = None):
+    if args is None:
+        args = sys.argv
+
+    parser = optparse.OptionParser()
+
+    parser.add_option('-j', '--jobs', action='store', type='int', default=0)
+    parser.add_option('--verbose', action='store_true', default=False)
+    parser.add_option('--fixit', action='store_true', default=False)
+    parser.add_option('--build-dir', default='build')
+
+    (options, args) = parser.parse_args(args)
+
+    jobs = options.jobs
+    if jobs == 0:
+        jobs = multiprocessing.cpu_count()
+
+    (compile_commands_file, compile_commands) = load_compile_commands(options.build_dir)
+
+    check_config = create_check_option(enabled_checks, disabled_checks)
+
+    pool = ThreadPool(jobs)
+
+    results = []
+    for info in compile_commands:
+        results.append(pool.apply_async(
+            run_clang_tidy,
+            (compile_commands_file,
+             check_config,
+             info['file'],
+             options)))
+
+    for result in results:
+        result.get()
+
+    return 0
+
+if __name__ == '__main__':
+    sys.exit(main())
+


Test patch:
diff --git a/src/tests/test_rng.cpp b/src/tests/test_rng.cpp
index f099040ee..deebea37e 100644
--- a/src/tests/test_rng.cpp
+++ b/src/tests/test_rng.cpp
@@ -357,6 +357,7 @@ class Stateful_RNG_Tests : public Test
             rng->randomize(&child_bytes[0], child_bytes.size());
             count = counting_rng.randomize_count();
             ssize_t written = ::write(fd[1], &count, sizeof(count));
+            BOTAN_UNUSED(written);
             try
                {
                rng->randomize(&child_bytes[0], child_bytes.size());
diff --git a/src/tests/test_uri.cpp b/src/tests/test_uri.cpp
index 4a14061f8..243b9d581 100644
--- a/src/tests/test_uri.cpp
+++ b/src/tests/test_uri.cpp
@@ -44,20 +44,20 @@ class URI_Tests final : public Test
          struct
             {
             std::string uri;
-            Botan::URI::Type type;
             std::string host;
+            Botan::URI::Type type;
             unsigned port;
             } tests []
             {
-               {"localhost::80", Botan::URI::Type::NotSet, {}, 0},
-               {"localhost:70000", Botan::URI::Type::NotSet, {}, 0},
-               {"[::1]:a", Botan::URI::Type::NotSet, {}, 0},
-               {"[::1]:70000", Botan::URI::Type::NotSet, {}, 0},
-               {"localhost:80", Botan::URI::Type::Domain, "localhost", 80},
-               {"www.example.com", Botan::URI::Type::Domain, "www.example.com", 0},
-               {"192.168.1.1", Botan::URI::Type::IPv4, "192.168.1.1", 0},
-               {"192.168.1.1:34567", Botan::URI::Type::IPv4, "192.168.1.1", 34567},
-               {"[::1]:61234", Botan::URI::Type::IPv6, "::1", 61234},
+               {"localhost::80", {}, Botan::URI::Type::NotSet, 0},
+               {"localhost:70000", {}, Botan::URI::Type::NotSet, 0},
+               {"[::1]:a", {}, Botan::URI::Type::NotSet, 0},
+               {"[::1]:70000", {}, Botan::URI::Type::NotSet, 0},
+               {"localhost:80", "localhost", Botan::URI::Type::Domain, 80},
+               {"www.example.com", "www.example.com", Botan::URI::Type::Domain, 0},
+               {"192.168.1.1", "192.168.1.1", Botan::URI::Type::IPv4, 0},
+               {"192.168.1.1:34567", "192.168.1.1", Botan::URI::Type::IPv4, 34567},
+               {"[::1]:61234", "::1", Botan::URI::Type::IPv6, 61234},
             };
 
          for(const auto& t : tests)