Fix some misc additional clang-tidy warnings

8 files changed, 84 insertions, 53 deletions

diff --git a/src/lib/filters/b64_filt.cpp b/src/lib/filters/b64_filt.cpp
index 8cbba1a6e..8f6c89ec2 100644
--- a/src/lib/filters/b64_filt.cpp
+++ b/src/lib/filters/b64_filt.cpp
@@ -15,9 +15,11 @@ namespace Botan {
 /*
 * Base64_Encoder Constructor
 */
-Base64_Encoder::Base64_Encoder(bool breaks, size_t length, bool t_n) :
-   m_line_length(breaks ? length : 0),
-   m_trailing_newline(t_n && breaks),
+Base64_Encoder::Base64_Encoder(bool line_breaks,
+                               size_t line_length,
+                               bool trailing_newline) :
+   m_line_length(line_breaks ? line_length : 0),
+   m_trailing_newline(trailing_newline && line_breaks),
    m_in(48),
    m_out(64),
    m_position(0),
diff --git a/src/lib/filters/filters.h b/src/lib/filters/filters.h
index e0caf5e4f..4f0170b8b 100644
--- a/src/lib/filters/filters.h
+++ b/src/lib/filters/filters.h
@@ -517,12 +517,12 @@ class BOTAN_PUBLIC_API(2,0) Base64_Encoder final : public Filter
 
       /**
       * Create a base64 encoder.
-      * @param breaks whether to use line breaks in the output
-      * @param length the length of the lines of the output
-      * @param t_n whether to use a trailing newline
+      * @param line_breaks whether to use line breaks in the output
+      * @param line_length the length of the lines of the output
+      * @param trailing_newline whether to use a trailing newline
       */
-      Base64_Encoder(bool breaks = false, size_t length = 72,
-                     bool t_n = false);
+      Base64_Encoder(bool line_break = false, size_t line_length = 72,
+                     bool trailing_newline = false);
    private:
       void encode_and_send(const uint8_t input[], size_t length,
                            bool final_inputs = false);
diff --git a/src/lib/pubkey/pem/pem.cpp b/src/lib/pubkey/pem/pem.cpp
index 0e8629fc9..59b834fd3 100644
--- a/src/lib/pubkey/pem/pem.cpp
+++ b/src/lib/pubkey/pem/pem.cpp
@@ -78,7 +78,7 @@ secure_vector<uint8_t> decode(DataSource& source, std::string& label)
       uint8_t b;
       if(!source.read_byte(b))
          throw Decoding_Error("PEM: No PEM header found");
-      if(b == PEM_HEADER1[position])
+      if(static_cast<char>(b) == PEM_HEADER1[position])
          ++position;
       else if(position >= RANDOM_CHAR_LIMIT)
          throw Decoding_Error("PEM: Malformed PEM header");
@@ -91,7 +91,7 @@ secure_vector<uint8_t> decode(DataSource& source, std::string& label)
       uint8_t b;
       if(!source.read_byte(b))
          throw Decoding_Error("PEM: No PEM header found");
-      if(b == PEM_HEADER2[position])
+      if(static_cast<char>(b) == PEM_HEADER2[position])
          ++position;
       else if(position)
          throw Decoding_Error("PEM: Malformed PEM header");
@@ -109,7 +109,7 @@ secure_vector<uint8_t> decode(DataSource& source, std::string& label)
       uint8_t b;
       if(!source.read_byte(b))
          throw Decoding_Error("PEM: No PEM trailer found");
-      if(b == PEM_TRAILER[position])
+      if(static_cast<char>(b) == PEM_TRAILER[position])
          ++position;
       else if(position)
          throw Decoding_Error("PEM: Malformed PEM trailer");
@@ -143,7 +143,7 @@ bool matches(DataSource& source, const std::string& extra,
    const std::string PEM_HEADER = "-----BEGIN " + extra;
 
    secure_vector<uint8_t> search_buf(search_range);
-   size_t got = source.peek(search_buf.data(), search_buf.size(), 0);
+   const size_t got = source.peek(search_buf.data(), search_buf.size(), 0);
 
    if(got < PEM_HEADER.length())
       return false;
@@ -152,13 +152,21 @@ bool matches(DataSource& source, const std::string& extra,
 
    for(size_t j = 0; j != got; ++j)
       {
-      if(search_buf[j] == PEM_HEADER[index])
+      if(static_cast<char>(search_buf[j]) == PEM_HEADER[index])
+         {
          ++index;
+         }
       else
+         {
          index = 0;
+         }
+
       if(index == PEM_HEADER.size())
+         {
          return true;
+         }
       }
+
    return false;
    }
 
diff --git a/src/lib/utils/http_util/http_util.cpp b/src/lib/utils/http_util/http_util.cpp
index 8ce3b1b94..5527f5041 100644
--- a/src/lib/utils/http_util/http_util.cpp
+++ b/src/lib/utils/http_util/http_util.cpp
@@ -23,7 +23,7 @@ namespace {
 * closes the socket.
 */
 std::string http_transact(const std::string& hostname,
-		                  const std::string& service,
+                          const std::string& service,
                           const std::string& message,
                           std::chrono::milliseconds timeout)
    {
@@ -67,6 +67,19 @@ std::string http_transact(const std::string& hostname,
    return oss.str();
    }
 
+bool needs_url_encoding(char c)
+   {
+   if(c >= 'A' && c <= 'Z')
+      return false;
+   if(c >= 'a' && c <= 'z')
+      return false;
+   if(c >= '0' && c <= '9')
+      return false;
+   if(c == '-' || c == '_' || c == '.' || c == '~')
+      return false;
+   return true;
+   }
+
 }
 
 std::string url_encode(const std::string& in)
@@ -75,16 +88,10 @@ std::string url_encode(const std::string& in)
 
    for(auto c : in)
       {
-      if(c >= 'A' && c <= 'Z')
-         out << c;
-      else if(c >= 'a' && c <= 'z')
-         out << c;
-      else if(c >= '0' && c <= '9')
-         out << c;
-      else if(c == '-' || c == '_' || c == '.' || c == '~')
-         out << c;
-      else
+      if(needs_url_encoding(c))
          out << '%' << hex_encode(cast_char_ptr_to_uint8(&c), 1);
+      else
+         out << c;
       }
 
    return out.str();
diff --git a/src/lib/utils/mem_pool/mem_pool.cpp b/src/lib/utils/mem_pool/mem_pool.cpp
index 872196c86..bc6d97f73 100644
--- a/src/lib/utils/mem_pool/mem_pool.cpp
+++ b/src/lib/utils/mem_pool/mem_pool.cpp
@@ -149,7 +149,7 @@ class BitMap final
       BitMap(size_t bits) : m_len(bits)
          {
          m_bits.resize((bits + BITMASK_BITS - 1) / BITMASK_BITS);
-         m_main_mask = static_cast<bitmask_type>(~0);
+         m_main_mask = ~static_cast<bitmask_type>(0);
          m_last_mask = m_main_mask;
 
          if(bits % BITMASK_BITS != 0)
diff --git a/src/lib/utils/scan_name.cpp b/src/lib/utils/scan_name.cpp
index e2a8112c8..5b1390923 100644
--- a/src/lib/utils/scan_name.cpp
+++ b/src/lib/utils/scan_name.cpp
@@ -60,8 +60,12 @@ SCAN_Name::SCAN_Name(const char* algo_spec) : SCAN_Name(std::string(algo_spec))
    {
    }
 
-SCAN_Name::SCAN_Name(std::string algo_spec) : m_orig_algo_spec(algo_spec), m_alg_name(), m_args(), m_mode_info()
-   { 
+SCAN_Name::SCAN_Name(const std::string& algo_spec) :
+   m_orig_algo_spec(algo_spec),
+   m_alg_name(),
+   m_args(),
+   m_mode_info()
+   {
    if(algo_spec.empty())
       throw Invalid_Argument("Expected algorithm name, got empty string");
 
diff --git a/src/lib/utils/scan_name.h b/src/lib/utils/scan_name.h
index 7135a0eb3..65e02f7ac 100644
--- a/src/lib/utils/scan_name.h
+++ b/src/lib/utils/scan_name.h
@@ -31,7 +31,7 @@ class SCAN_Name final
       * Create a SCAN_Name
       * @param algo_spec A SCAN-format name
       */
-      explicit SCAN_Name(std::string algo_spec);
+      explicit SCAN_Name(const std::string& algo_spec);
 
       /**
       * @return original input string
diff --git a/src/scripts/run_clang_tidy.py b/src/scripts/run_clang_tidy.py
index c970097b6..c53ab22f0 100755
--- a/src/scripts/run_clang_tidy.py
+++ b/src/scripts/run_clang_tidy.py
@@ -10,57 +10,67 @@ import re
 from multiprocessing.pool import ThreadPool
 
 enabled_checks = [
-    #'clang-analyzer-*',
-    #'performance-*',
-    #'bugprone-*',
-    #'cert-*',
+    'bugprone-*',
+    'cert-*',
+    'clang-analyzer-*',
+    'modernize-concat-nested-namespaces',
+    'performance-*',
+    'portability-*',
+    'readability-container-size-empty',
+    'readability-static-definition-in-anonymous-namespace',
+
     #'cppcoreguidelines-*',
     #'hicpp-*',
     #'modernize-*',
-    #'portability-*',
     #'readability-*',
-    #'readability-container-size-empty',
-    #'readability-static-definition-in-anonymous-namespace',
     #'modernize-make-unique',
-    'modernize-concat-nested-namespaces',
     #'readability-inconsistent-declaration-parameter-name',
 ]
 
-disabled_checks = [
-    '*-array-to-pointer-decay',
-    '*-avoid-c-arrays',
-    '*-braces-around-statements', # should fix
-    '*-else-after-return',
-    '*-function-size', # don't care
-    '*-magic-numbers', # not a problem
-    '*-no-array-decay',
-    '*-use-auto', # not universally a good idea
+# these might be worth being clan-y
+disabled_needs_work = [
+    '*-braces-around-statements', # should fix (need clang-format)
     'bugprone-easily-swappable-parameters',
     'bugprone-implicit-widening-of-multiplication-result',
     'bugprone-macro-parentheses', # should be fixed (using inline/constexpr)
-    'cert-err58-cpp', # shut up whiner
+    'bugprone-narrowing-conversions', # should be fixed
     'cppcoreguidelines-init-variables',
-    'cppcoreguidelines-no-malloc',
     'cppcoreguidelines-owning-memory',
-    'cppcoreguidelines-pro-bounds-constant-array-index',
     'cppcoreguidelines-pro-bounds-pointer-arithmetic',
-    'cppcoreguidelines-pro-type-reinterpret-cast', # not possible thanks though
     'hicpp-signed-bitwise', # djb shit
-    'modernize-loop-convert', # sometimes very ugly
     'modernize-pass-by-value',
-    'modernize-return-braced-init-list', # thanks I hate it
-    'modernize-use-nodiscard', # maybe
+    'modernize-use-nodiscard',
     'modernize-use-trailing-return-type',
     'performance-inefficient-string-concatenation',
     'performance-no-int-to-ptr',
-    'portability-simd-intrinsics', # not a problem
-    'readability-function-cognitive-complexity', # bogus
     'readability-implicit-bool-conversion', # maybe fix this
     'readability-inconsistent-declaration-parameter-name', # should fix this
     'readability-isolate-declaration',
     'readability-simplify-boolean-expr', # sometimes ok
 ]
 
+# these we are not interested in ever being clang-tidy clean for
+disabled_not_interested = [
+    '*-array-to-pointer-decay',
+    '*-avoid-c-arrays',
+    '*-else-after-return',
+    '*-function-size',
+    '*-magic-numbers', # can't stop the magic
+    '*-no-array-decay',
+    '*-use-auto', # not universally a good idea
+    'bugprone-branch-clone', # doesn't interact well with feature macros
+    'cert-err58-cpp',
+    'cppcoreguidelines-no-malloc',
+    'cppcoreguidelines-pro-bounds-constant-array-index',
+    'cppcoreguidelines-pro-type-reinterpret-cast', # not possible thanks though
+    'modernize-loop-convert', # sometimes very ugly
+    'modernize-return-braced-init-list', # thanks I hate it
+    'portability-simd-intrinsics',
+    'readability-function-cognitive-complexity',
+]
+
+disabled_checks = disabled_needs_work + disabled_not_interested
+
 def create_check_option(enabled, disabled):
     return ','.join(enabled) + ',' + ','.join(['-' + d for d in disabled])