Merge pull request #4101 from randombit/jack/clang-tidy-fixes

Fix various issues flagged by clang-tidy 18

.github/workflows/nightly.yml

@@ -25,7 +25,7 @@ jobs:
   clang_tidy:
     name: "clang-tidy"
 
-    runs-on: ubuntu-22.04
+    runs-on: ubuntu-24.04
 
     steps:
       - uses: actions/checkout@v4
@@ -36,8 +36,8 @@ jobs:
           target: clang-tidy
           cache-key: linux-x86_64-clang-tidy
 
-      - name: Install Boost
-        run: sudo apt-get -qq install libboost-dev
+      - name: Install dependencies
+        run: sudo apt-get -qq install libboost-dev libbz2-dev liblzma-dev libsqlite3-dev
 
       - name: Configure Build
         run: python3 ./configure.py --cc=clang --build-targets=shared,cli,tests,examples,bogo_shim --build-fuzzers=test --with-boost --with-sqlite --with-zlib --with-lzma --with-bzip2

src/bogo_shim/bogo_shim.cpp

@@ -633,7 +633,7 @@ void Shim_Arguments::parse_args(char* argv[]) {
    while(argv[i] != nullptr) {
       const std::string param(argv[i]);
 
-      if(param.find('-') == 0) {
+      if(param.starts_with("-")) {
          const std::string flag_name = param.substr(1, std::string::npos);
 
          if(m_flags.contains(flag_name)) {
@@ -1811,7 +1811,7 @@ int main(int /*argc*/, char* argv[]) {
                shim_log("Offering " + offer_version.to_string());
 
                std::string host_name = args->get_string_opt_or_else("host-name", hostname);
-               if(args->test_name().find("UnsolicitedServerNameAck") == 0) {
+               if(args->test_name().starts_with("UnsolicitedServerNameAck")) {
                   host_name = "";  // avoid sending SNI for this test
                }
 

src/cli/math.cpp

@@ -96,9 +96,11 @@ class Factor final : public Command {
          std::vector<Botan::BigInt> factors = factorize(n, rng());
          std::sort(factors.begin(), factors.end());
 
-         output() << n << ": ";
-         std::copy(factors.begin(), factors.end(), std::ostream_iterator<Botan::BigInt>(output(), " "));
-         output() << std::endl;
+         output() << n << ":";
+         for(const auto& factor : factors) {
+            output() << " " << factor;
+         }
+         output() << "\n";
       }
 
    private:

src/cli/timing_tests.cpp

@@ -622,13 +622,7 @@ class MARVIN_Test_Command final : public Command {
 
                const uint64_t start = Botan::OS::get_system_timestamp_ns();
 
-   #if 0
-               try {
-                  op.decrypt(ciphertext.data(), modulus_bytes);
-               } catch(...) {}
-   #else
                op.decrypt_or_random(ciphertext.data(), modulus_bytes, expect_pt_len, rng());
-   #endif
 
                const uint64_t duration = Botan::OS::get_system_timestamp_ns() - start;
                BOTAN_ASSERT_NOMSG(measurements[testcase].size() == r);

src/cli/tls_client.cpp

@@ -200,14 +200,13 @@ class TLS_Client final : public Command {
          const std::string next_protos = get_arg("next-protocols");
          const bool use_system_cert_store = flag_set("skip-system-cert-store") == false;
          const std::string trusted_CAs = get_arg("trusted-cas");
-         const std::string trusted_pubkey_sha256 = get_arg("trusted-pubkey-sha256");
          const auto tls_version = get_arg("tls-version");
 
          if(!sessions_db.empty()) {
    #if defined(BOTAN_HAS_TLS_SQLITE3_SESSION_MANAGER)
             const std::string sessions_passphrase = get_passphrase_arg("Session DB passphrase", "session-db-pass");
-            session_mgr.reset(
-               new Botan::TLS::Session_Manager_SQLite(sessions_passphrase, rng_as_shared(), sessions_db));
+            session_mgr =
+               std::make_shared<Botan::TLS::Session_Manager_SQLite>(sessions_passphrase, rng_as_shared(), sessions_db);
    #else
             error_output() << "Ignoring session DB file, sqlite not enabled\n";
    #endif
@@ -400,7 +399,7 @@ class TLS_Client final : public Command {
                continue;
             }
 
-            if(::connect(fd, rp->ai_addr, static_cast<socklen_t>(rp->ai_addrlen)) != 0) {
+            if(::connect(fd, rp->ai_addr, rp->ai_addrlen) != 0) {
                ::close(fd);
                continue;
             }

src/cli/tls_http_server.cpp

@@ -286,11 +286,22 @@ net::awaitable<void> do_listen(tcp::endpoint endpoint,
 
    // If max_clients is zero in the beginning, we'll serve forever
    // otherwise we'll count down and stop eventually.
-   do {
+
+   const bool run_forever = (max_clients == 0);
+
+   auto done = [&] {
+      if(run_forever) {
+         return false;
+      } else {
+         return max_clients-- == 0;
+      }
+   };
+
+   while(!done()) {
       boost::asio::co_spawn(acceptor.get_executor(),
                             do_session(tcp_stream(co_await acceptor.async_accept()), tls_ctx, logger),
                             make_final_completion_handler(logger, "Session"));
-   } while(max_clients == 0 || --max_clients > 0);
+   }
 }
 
 }  // namespace
@@ -337,15 +348,15 @@ class TLS_HTTP_Server final : public Command {
          if(!sessions_db.empty()) {
    #if defined(BOTAN_HAS_TLS_SQLITE3_SESSION_MANAGER)
             const std::string sessions_passphrase = get_passphrase_arg("Session DB passphrase", "session-db-pass");
-            session_mgr.reset(
-               new Botan::TLS::Session_Manager_SQLite(sessions_passphrase, rng_as_shared(), sessions_db));
+            session_mgr =
+               std::make_shared<Botan::TLS::Session_Manager_SQLite>(sessions_passphrase, rng_as_shared(), sessions_db);
    #else
             throw CLI_Error_Unsupported("Sqlite3 support not available");
    #endif
          }
 
          if(!session_mgr) {
-            session_mgr.reset(new Botan::TLS::Session_Manager_In_Memory(rng_as_shared()));
+            session_mgr = std::make_shared<Botan::TLS::Session_Manager_In_Memory>(rng_as_shared());
          }
 
          auto logger = std::make_shared<Logger>(output(), error_output());

src/cli/tls_proxy.cpp

@@ -98,7 +98,7 @@ class ServerStatus {
 class tls_proxy_session final : public std::enable_shared_from_this<tls_proxy_session>,
                                 public Botan::TLS::Callbacks {
    public:
-      enum { readbuf_size = 17 * 1024 };
+      static constexpr size_t readbuf_size = 17 * 1024;
 
       typedef std::shared_ptr<tls_proxy_session> pointer;
 
@@ -433,12 +433,12 @@ class TLS_Proxy final : public Command {
          const std::string sessions_db = get_arg("session-db");
 
          if(!sessions_db.empty()) {
-            session_mgr.reset(
-               new Botan::TLS::Session_Manager_SQLite(sessions_passphrase, rng_as_shared(), sessions_db));
+            session_mgr =
+               std::make_shared<Botan::TLS::Session_Manager_SQLite>(sessions_passphrase, rng_as_shared(), sessions_db);
          }
    #endif
          if(!session_mgr) {
-            session_mgr.reset(new Botan::TLS::Session_Manager_In_Memory(rng_as_shared()));
+            session_mgr = std::make_shared<Botan::TLS::Session_Manager_In_Memory>(rng_as_shared());
          }
 
          tls_proxy_server server(io, listen_port, server_endpoint_iterator, creds, policy, session_mgr, max_clients);

src/cli/x509.cpp

@@ -154,8 +154,6 @@ class Cert_Info final : public Command {
       std::string description() const override { return "Parse X.509 certificate and display data fields"; }
 
       void go() override {
-         const std::string arg_file = get_arg("file");
-
          std::vector<uint8_t> data = slurp_file(get_arg("file"));
 
          Botan::DataSource_Memory in(data);

src/examples/dl_group.cpp

@@ -7,10 +7,12 @@
 int main() {
    Botan::AutoSeeded_RNG rng;
    auto group = std::make_unique<Botan::DL_Group>(rng, Botan::DL_Group::Strong, 2048);
-   std::cout << "\np: " << group->get_p();
-   std::cout << "\nq: " << group->get_q();
-   std::cout << "\ng: " << group->get_q();
-   std::cout << "\nANSI_X9_42:\n" << group->PEM_encode(Botan::DL_Group_Format::ANSI_X9_42);
+
+   std::cout << "P = " << group->get_p().to_hex_string() << "\n"
+             << "Q = " << group->get_q().to_hex_string() << "\n"
+             << "G = " << group->get_g().to_hex_string() << "\n";
+
+   std::cout << "\nPEM:\n" << group->PEM_encode(Botan::DL_Group_Format::ANSI_X9_42) << "\n";
 
    return 0;
 }

src/examples/hmac.cpp

@@ -4,7 +4,9 @@
 
 #include <assert.h>
 
-static std::string compute_mac(const std::string& msg, const Botan::secure_vector<uint8_t>& key) {
+namespace {
+
+std::string compute_mac(const std::string& msg, const Botan::secure_vector<uint8_t>& key) {
    auto hmac = Botan::MessageAuthenticationCode::create_or_throw("HMAC(SHA-256)");
 
    hmac->set_key(key);
@@ -13,6 +15,8 @@ static std::string compute_mac(const std::string& msg, const Botan::secure_vecto
    return Botan::hex_encode(hmac->final());
 }
 
+}  // namespace
+
 int main() {
    Botan::AutoSeeded_RNG rng;
 

src/examples/password_encryption.cpp

@@ -9,7 +9,7 @@
 namespace {
 
 template <typename OutT = std::vector<uint8_t>, typename... Ts>
-OutT concat(Ts&&... buffers) {
+OutT concat(const Ts&... buffers) {
    OutT out;
    out.reserve((buffers.size() + ... + 0));
    (out.insert(out.end(), buffers.begin(), buffers.end()), ...);

src/examples/tls_custom_curves_client.cpp

@@ -86,6 +86,8 @@ class Client_Policy : public Botan::TLS::Strict_Policy {
    public:
       std::vector<Botan::TLS::Group_Params> key_exchange_groups() const override {
          // modified strict policy to allow our custom curves
+
+         // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange)
          return {static_cast<Botan::TLS::Group_Params>(0xFE00)};
       }
 };

src/examples/tls_ssl_key_log_file.cpp

@@ -24,8 +24,10 @@
    #include <sys/socket.h>
 #endif
 
-constexpr static uint32_t SERVER_PORT = 5060;
-constexpr static uint32_t CLIENT_PORT = 5070;
+namespace {
+
+constexpr uint32_t SERVER_PORT = 5060;
+constexpr uint32_t CLIENT_PORT = 5070;
 
 class Client_Credential : public Botan::Credentials_Manager {
    public:
@@ -205,7 +207,7 @@ class DtlsConnection : public Botan::TLS::Callbacks {
       }
 };
 
-static void server_proc(const std::function<void(std::shared_ptr<DtlsConnection> conn)>& conn_callback) {
+void server_proc(const std::function<void(std::shared_ptr<DtlsConnection> conn)>& conn_callback) {
    std::cout << "Start Server" << std::endl;
 
    int fd = 0;
@@ -251,7 +253,7 @@ static void server_proc(const std::function<void(std::shared_ptr<DtlsConnection>
    std::cout << "Server closed" << std::endl;
 }
 
-static void client_proc(const std::function<void(std::shared_ptr<DtlsConnection> conn)>& conn_callback) {
+void client_proc(const std::function<void(std::shared_ptr<DtlsConnection> conn)>& conn_callback) {
    std::cout << "Start Client" << std::endl;
 
    int fd = 0;
@@ -296,6 +298,8 @@ static void client_proc(const std::function<void(std::shared_ptr<DtlsConnection>
    std::cout << "Client closed" << std::endl;
 }
 
+}  // namespace
+
 int main() {
    std::mutex m;
    std::condition_variable conn_cond;

src/examples/tls_stream_coroutine_client.cpp

@@ -68,9 +68,7 @@ http::request<http::string_body> create_GET_request(const std::string& host, con
    return req;
 }
 
-}  // namespace
-
-static net::awaitable<void> request(std::string host, std::string port, std::string target) {
+net::awaitable<void> request(std::string host, std::string port, std::string target) {
    // Lookup host address
    auto resolver = net::use_awaitable.as_default_on(tcp::resolver(co_await net::this_coro::executor));
    const auto dns_result = co_await resolver.async_resolve(host, port);
@@ -98,6 +96,8 @@ static net::awaitable<void> request(std::string host, std::string port, std::str
    tls_stream.next_layer().close();
 }
 
+}  // namespace
+
 int main(int argc, char* argv[]) {
    if(argc != 4) {
       std::cerr << "Usage: tls_stream_coroutine_client <host> <port> <target>\n"
@@ -110,21 +110,26 @@ int main(int argc, char* argv[]) {
    const auto port = argv[2];
    const auto target = argv[3];
 
-   net::io_context ioc;
-
    int return_code = 0;
-   net::co_spawn(ioc, request(host, port, target), [&](std::exception_ptr eptr) {
-      if(eptr) {
-         try {
-            std::rethrow_exception(eptr);
-         } catch(std::exception& ex) {
-            std::cerr << "Error: " << ex.what() << "\n";
-            return_code = 1;
+
+   try {
+      net::io_context ioc;
+
+      net::co_spawn(ioc, request(host, port, target), [&](const std::exception_ptr& eptr) {
+         if(eptr) {
+            try {
+               std::rethrow_exception(eptr);
+            } catch(std::exception& ex) {
+               std::cerr << "Error: " << ex.what() << "\n";
+               return_code = 1;
+            }
          }
-      }
-   });
+      });
 
-   ioc.run();
+      ioc.run();
+   } catch(std::exception& e) {
+      std::cerr << e.what() << "\n";
+   }
 
    return return_code;
 }

src/lib/block/camellia/camellia.cpp

@@ -239,7 +239,16 @@ void key_schedule(secure_vector<uint64_t>& SK, std::span<const uint8_t> key) {
    const uint64_t KL_L = load_be<uint64_t>(key.data(), 1);
 
    const uint64_t KR_H = (key.size() >= 24) ? load_be<uint64_t>(key.data(), 2) : 0;
-   const uint64_t KR_L = (key.size() == 32) ? load_be<uint64_t>(key.data(), 3) : ((key.size() == 24) ? ~KR_H : 0);
+
+   const uint64_t KR_L = [&]() -> uint64_t {
+      if(key.size() == 32) {
+         return load_be<uint64_t>(key.data(), 3);
+      } else if(key.size() == 24) {
+         return ~KR_H;
+      } else {
+         return 0;
+      }
+   }();
 
    uint64_t D1 = KL_H ^ KR_H;
    uint64_t D2 = KL_L ^ KR_L;

src/lib/block/threefish_512/threefish_512.cpp

@@ -53,15 +53,15 @@ class Key_Inserter {
    public:
       Key_Inserter(const uint64_t* K, const uint64_t* T) : m_K(K), m_T(T) {}
 
-      inline void e_add(size_t R,
-                        uint64_t& X0,
-                        uint64_t& X1,
-                        uint64_t& X2,
-                        uint64_t& X3,
-                        uint64_t& X4,
-                        uint64_t& X5,
-                        uint64_t& X6,
-                        uint64_t& X7) const {
+      void e_add(size_t R,
+                 uint64_t& X0,
+                 uint64_t& X1,
+                 uint64_t& X2,
+                 uint64_t& X3,
+                 uint64_t& X4,
+                 uint64_t& X5,
+                 uint64_t& X6,
+                 uint64_t& X7) const {
          X0 += m_K[(R) % 9];
          X1 += m_K[(R + 1) % 9];
          X2 += m_K[(R + 2) % 9];
@@ -72,15 +72,15 @@ class Key_Inserter {
          X7 += m_K[(R + 7) % 9] + R;
       }
 
-      inline void d_add(size_t R,
-                        uint64_t& X0,
-                        uint64_t& X1,
-                        uint64_t& X2,
-                        uint64_t& X3,
-                        uint64_t& X4,
-                        uint64_t& X5,
-                        uint64_t& X6,
-                        uint64_t& X7) const {
+      void d_add(size_t R,
+                 uint64_t& X0,
+                 uint64_t& X1,
+                 uint64_t& X2,
+                 uint64_t& X3,
+                 uint64_t& X4,
+                 uint64_t& X5,
+                 uint64_t& X6,
+                 uint64_t& X7) const {
          X0 -= m_K[(R) % 9];
          X1 -= m_K[(R + 1) % 9];
          X2 -= m_K[(R + 2) % 9];