fix: issues from -Weverything

src/CMakeLists.txt

@@ -7,6 +7,7 @@ include(ExternalProject)
 include(FindPkgConfig)
 
 add_compile_options(-Wall -Wextra -pedantic)
+# add_compile_options(-Weverything -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-pre-c++17-compat -Wno-c++20-compat -Wno-clang-diagnostic-unused-macros)
 
 set(CMAKE_CXX_STANDARD 20)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
@@ -53,6 +54,7 @@ add_executable(
     serversocket_p.cpp
     tlsconfigurator.cpp
     tlsconfigurator_p.cpp
+    tlsexception.cpp
     tlsservercontext.cpp
     tlsservercontext_p.cpp
     tlsutils.cpp

src/clienthandler.h

@@ -12,7 +12,7 @@ struct tls;
 class ClientHandler {
 public:
     ClientHandler(
-        tls* context, const ev::loop_ref& loop, const std::shared_ptr<Server>& server,
+        tls* ctx, const ev::loop_ref& loop, const std::shared_ptr<Server>& server,
         const std::shared_ptr<Database>& database
     );
     ClientHandler(const ClientHandler&)            = delete;

src/clienthandler_p.cpp

@@ -2,12 +2,13 @@
 #include "clienthandler_p.h"
 #include "database.h"
 #include "server.h"
+#include "utils.h"
 
 ClientHandlerPrivate::ClientHandlerPrivate(
-    ClientHandler* q_ptr, tls* ctx, const ev::loop_ref& loop, const std::shared_ptr<Server>& server,
+    ClientHandler* q, tls* ctx, const ev::loop_ref& loop, const std::shared_ptr<Server>& server,
     const std::shared_ptr<Database>& database
 )
-    : q_ptr(q_ptr), m_ctx(ctx, &dispose_tls_context), m_loop(loop), m_server(server), m_database(database), m_io(loop),
+    : q_ptr(q), m_ctx(ctx, &dispose_tls_context), m_loop(loop), m_server(server), m_database(database), m_io(loop),
       m_timer(loop)
 {
     llhttp_settings_init(&this->m_settings);
@@ -49,16 +50,16 @@ int ClientHandlerPrivate::accept(int fd)
 
 void ClientHandlerPrivate::on_read(ev::io& watcher, int revents)
 {
-    if (revents & ev::ERROR) [[unlikely]] {
+    if (is_ev_error(revents)) [[unlikely]] {
         this->close_connection("Error: failed to read from socket: EV_ERROR");
         return;
     }
 
     constexpr std::size_t BUFFER_SIZE = 8192;
     std::array<char, BUFFER_SIZE> buffer{};
-    int new_events = 0;
-    ssize_t received;
-    std::string error;
+    int new_events   = 0;
+    ssize_t received = 0;
+    std::string error{};
 
     if (this->m_ctx) {
         received = tls_read(this->m_ctx.get(), buffer.data(), sizeof(buffer));
@@ -114,17 +115,26 @@ void ClientHandlerPrivate::on_read(ev::io& watcher, int revents)
 
 void ClientHandlerPrivate::on_write(ev::io& watcher, int revents)
 {
-    if (revents & ev::ERROR) [[unlikely]] {
+    if (is_ev_error(revents)) [[unlikely]] {
         this->close_connection("Error: failed to write to socket: EV_ERROR");
         return;
     }
 
-    // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
-    const auto* buf = this->m_response.data() + this->m_offset;
-    auto len        = this->m_response.size() - this->m_offset;
+    const char* buf = nullptr;
+    std::size_t len = 0;
+
+    if (this->m_offset <= this->m_response.size()) {
+        buf = &this->m_response[this->m_offset];
+        len = this->m_response.size() - this->m_offset;
+    }
+    else [[unlikely]] {
+        this->close_connection("Error: failed to write to socket: offset > response size");
+        return;
+    }
+
     int new_events  = 0;
-    std::string error;
-    ssize_t written;
+    ssize_t written = 0;
+    std::string error{};
 
     if (this->m_ctx) {
         written = tls_write(this->m_ctx.get(), buf, len);
@@ -179,19 +189,18 @@ void ClientHandlerPrivate::on_write(ev::io& watcher, int revents)
 
 void ClientHandlerPrivate::on_tls_close(ev::io& watcher, int revents)
 {
-    if (revents & ev::ERROR) [[unlikely]] {
-        watcher.stop();
+    if (is_ev_error(revents)) [[unlikely]] {
         std::cerr << "Error: failed to close TLS connection gracefully: EV_ERROR\n";
         this->m_ctx.reset();
         this->terminate();
     }
 
-    auto res = tls_close(this->m_ctx.get());
+    const auto res = tls_close(this->m_ctx.get());
     if (res == TLS_WANT_POLLIN || res == TLS_WANT_POLLOUT) {
         watcher.start(watcher.fd, res == TLS_WANT_POLLIN ? ev::READ : ev::WRITE);
     }
     else {
-        if (res == -1) {
+        if (res == -1) [[unlikely]] {
             std::cerr << std::format(
                 "Warning: failed to close TLS connection gracefully: {}\n", tls_error(this->m_ctx.get())
             );
@@ -203,21 +212,21 @@ void ClientHandlerPrivate::on_tls_close(ev::io& watcher, int revents)
     }
 }
 
-void ClientHandlerPrivate::on_timeout(ev::timer& watcher, int)
+void ClientHandlerPrivate::on_timeout(ev::timer& timer, int)
 {
     // Stop the watcher here because close_connection() may take some time to complete
-    watcher.stop();
+    timer.stop();
     this->close_connection();
 }
 
 void ClientHandlerPrivate::close_connection(const std::string& error)
 {
-    if (!error.empty()) {
+    if (!error.empty()) [[unlikely]] {
         std::cerr << error << '\n';
     }
 
     if (this->m_ctx) {
-        if (int res = tls_close(this->m_ctx.get()); res == TLS_WANT_POLLIN || res == TLS_WANT_POLLOUT) {
+        if (const int res = tls_close(this->m_ctx.get()); res == TLS_WANT_POLLIN || res == TLS_WANT_POLLOUT) {
             this->m_timer.again();
             this->m_io.stop();
             this->m_io.set<ClientHandlerPrivate, &ClientHandlerPrivate::on_tls_close>(this);

src/clienthandler_p.h

@@ -16,7 +16,7 @@ struct tls;
 class ClientHandlerPrivate {
 public:
     ClientHandlerPrivate(
-        ClientHandler* q_ptr, tls* ctx, const ev::loop_ref& loop, const std::shared_ptr<Server>& server,
+        ClientHandler* q, tls* ctx, const ev::loop_ref& loop, const std::shared_ptr<Server>& server,
         const std::shared_ptr<Database>& database
     );
     ClientHandlerPrivate(const ClientHandlerPrivate&)            = delete;
@@ -39,12 +39,12 @@ class ClientHandlerPrivate {
     llhttp_settings_t m_settings{};
     ev::io m_io;
     ev::timer m_timer;
-    llhttp_method_t m_method = HTTP_GET;
     ada::url_aggregator m_url{};
     std::string m_body{};
     std::string m_response;
-    std::size_t m_offset = 0;
-    bool m_done          = false;
+    std::size_t m_offset     = 0;
+    llhttp_method_t m_method = HTTP_GET;
+    bool m_done              = false;
 
     void on_read(ev::io& watcher, int revents);
     void on_write(ev::io& watcher, int revents);

src/database_p.cpp

@@ -129,11 +129,14 @@ std::string DatabasePrivate::generate_uuid()
     );
 }
 
-void DatabasePrivate::run_query(const char* query, std::initializer_list<std::string> args)
+void DatabasePrivate::run_query(const char* query, const std::initializer_list<std::string>& args)
 {
     sqlite3pp::command cmd(this->m_db, query);
-    for (int i = 0; i < static_cast<int>(args.size()); ++i) {
-        cmd.bind(i + 1, *(args.begin() + i), sqlite3pp::nocopy);
+
+    int i = 1;
+    // NOLINTNEXTLINE(*-bounds-pointer-arithmetic)
+    for (const auto* it = args.begin(); it != args.end(); ++it, ++i) {
+        cmd.bind(i, *it, sqlite3pp::nocopy);
     }
 
     if (cmd.execute() != SQLITE_OK) [[unlikely]] {

src/database_p.h

@@ -26,7 +26,7 @@ class DatabasePrivate {
     [[nodiscard]] bool has_lock(const std::string& slug, const std::string& lock_id);
 
     [[nodiscard]] static std::string generate_uuid();
-    void run_query(const char* query, std::initializer_list<std::string> args);
+    void run_query(const char* query, const std::initializer_list<std::string>& args);
     void execute(const char* query);
 
     template<typename F, typename... Args>

src/main.cpp

@@ -99,7 +99,9 @@ int main()
         auto ctx = tlsconf.configure();
         if (ctx) {
             ctx->get_context();
-            tlsconf.watch([&ctx](std::shared_ptr<TLSServerContext> new_ctx) { replace_tls_context(ctx, new_ctx); });
+            tlsconf.watch([&ctx](const std::shared_ptr<TLSServerContext>& new_ctx) {
+                replace_tls_context(ctx, new_ctx);
+            });
         }
 
         auto server = Server::create(loop, address, port, database);

src/server_p.cpp

@@ -8,10 +8,10 @@
 #include "tlsservercontext.h"
 
 ServerPrivate::ServerPrivate(
-    Server* q_ptr, const ev::loop_ref& loop, const std::string& ip, std::uint16_t port,
+    Server* q, const ev::loop_ref& loop, const std::string& ip, std::uint16_t port,
     const std::shared_ptr<Database>& database
 )
-    : q_ptr(q_ptr), m_loop(loop), m_socket(ip, port), m_accept_watcher(loop), m_database(database)
+    : q_ptr(q), m_loop(loop), m_socket(ip, port), m_accept_watcher(loop), m_database(database)
 {}
 
 std::uint16_t ServerPrivate::run()

src/server_p.h

@@ -15,7 +15,7 @@ class TLSServerContext;
 class ServerPrivate {
 public:
     ServerPrivate(
-        Server* q_ptr, const ev::loop_ref& loop, const std::string& ip, std::uint16_t port,
+        Server* q, const ev::loop_ref& loop, const std::string& ip, std::uint16_t port,
         const std::shared_ptr<Database>& database
     );
 

src/stdafx.h

@@ -24,6 +24,7 @@
 #include <string>
 #include <string_view>
 #include <system_error>
+#include <type_traits>
 #include <unordered_set>
 #include <vector>
 #include <ada.h>

src/tlsconfigurator.cpp

@@ -13,7 +13,7 @@ std::shared_ptr<TLSServerContext> TLSConfigurator::configure() const
     return this->d_func()->configure();
 }
 
-void TLSConfigurator::watch(watch_callback_t callback)
+void TLSConfigurator::watch(const watch_callback_t& callback)
 {
     this->d_func()->watch(callback);
 }

src/tlsconfigurator.h

@@ -9,13 +9,17 @@ class TLSServerContext;
 
 class TLSConfigurator {
 public:
-    using watch_callback_t = std::function<void(std::shared_ptr<TLSServerContext>)>;
+    using watch_callback_t = std::function<void(const std::shared_ptr<TLSServerContext>&)>;
 
     explicit TLSConfigurator(const std::string& env_prefix);
+    TLSConfigurator(const TLSConfigurator&)            = delete;
+    TLSConfigurator(TLSConfigurator&&)                 = delete;
+    TLSConfigurator& operator=(const TLSConfigurator&) = delete;
+    TLSConfigurator& operator=(TLSConfigurator&&)      = delete;
     ~TLSConfigurator();
 
-    std::shared_ptr<TLSServerContext> configure() const;
-    void watch(watch_callback_t callback);
+    [[nodiscard]] std::shared_ptr<TLSServerContext> configure() const;
+    void watch(const watch_callback_t& callback);
     void stop();
 
 private:

src/tlsconfigurator_p.cpp

@@ -3,10 +3,11 @@
 #include <string>
 #include "tlsexception.h"
 #include "tlsservercontext.h"
+#include "utils.h"
 
 namespace {
 
-std::string getenv(const std::string& prefix, const char* name)
+std::string get_env(const std::string& prefix, const char* name)
 {
     const std::string env_name = prefix + name;
     const char* value          = std::getenv(env_name.c_str());  // NOLINT(concurrency-mt-unsafe)
@@ -15,24 +16,24 @@ std::string getenv(const std::string& prefix, const char* name)
 
 }  // namespace
 
-TLSConfiguratorPrivate::TLSConfiguratorPrivate(const std::string& env_prefix) : m_env_prefix(env_prefix)
+TLSConfiguratorPrivate::TLSConfiguratorPrivate(const std::string& env_prefix)
 {
-    this->m_https               = getenv(this->m_env_prefix, "HTTPS") == "1";
-    this->m_certificate         = getenv(this->m_env_prefix, "CERTIFICATE");
-    this->m_key                 = getenv(this->m_env_prefix, "PRIVATE_KEY");
-    this->m_ca                  = getenv(this->m_env_prefix, "CA_CERTIFICATE");
-    this->m_trusted_certificate = getenv(this->m_env_prefix, "TRUSTED_CERTIFICATE");
-    this->m_tls_protocols       = getenv(this->m_env_prefix, "TLS_PROTOCOLS");
-    this->m_tls_ciphers         = getenv(this->m_env_prefix, "TLS_CIPHERS");
-    this->m_tls_curves          = getenv(this->m_env_prefix, "TLS_CURVES");
-    this->m_tls_verify_client   = getenv(this->m_env_prefix, "TLS_VERIFY_CLIENT") == "1";
-    this->m_tls_enable_dhe      = getenv(this->m_env_prefix, "TLS_ENABLE_DHE") == "1";
+    this->m_https               = get_env(env_prefix, "HTTPS") == "1";
+    this->m_certificate         = get_env(env_prefix, "CERTIFICATE");
+    this->m_key                 = get_env(env_prefix, "PRIVATE_KEY");
+    this->m_ca                  = get_env(env_prefix, "CA_CERTIFICATE");
+    this->m_trusted_certificate = get_env(env_prefix, "TRUSTED_CERTIFICATE");
+    this->m_tls_protocols       = get_env(env_prefix, "TLS_PROTOCOLS");
+    this->m_tls_ciphers         = get_env(env_prefix, "TLS_CIPHERS");
+    this->m_tls_curves          = get_env(env_prefix, "TLS_CURVES");
+    this->m_tls_verify_client   = get_env(env_prefix, "TLS_VERIFY_CLIENT") == "1";
+    this->m_tls_enable_dhe      = get_env(env_prefix, "TLS_ENABLE_DHE") == "1";
 }
 
 std::shared_ptr<TLSServerContext> TLSConfiguratorPrivate::configure() const
 {
     if (!this->m_https) {
-        return std::shared_ptr<TLSServerContext>(nullptr);
+        return {};
     }
 
     auto context = TLSServerContext::create();
@@ -71,7 +72,7 @@ std::shared_ptr<TLSServerContext> TLSConfiguratorPrivate::configure() const
     return context;
 }
 
-void TLSConfiguratorPrivate::watch(TLSConfigurator::watch_callback_t callback)
+void TLSConfiguratorPrivate::watch(const TLSConfigurator::watch_callback_t& callback)
 {
     if (!callback) {
         this->m_stat_watcher.stop();
@@ -92,7 +93,7 @@ void TLSConfiguratorPrivate::watch(TLSConfigurator::watch_callback_t callback)
 
 void TLSConfiguratorPrivate::on_stat(ev::stat& watcher, int revents)
 {
-    if (revents & ev::ERROR) {
+    if (is_ev_error(revents)) [[unlikely]] {
         std::cerr << std::format("Error: stat watcher error: EV_ERROR\n");
         return;
     }
@@ -101,14 +102,14 @@ void TLSConfiguratorPrivate::on_stat(ev::stat& watcher, int revents)
     this->m_timer.again();
 }
 
-void TLSConfiguratorPrivate::on_timeout(ev::timer& watcher, int revents)
+void TLSConfiguratorPrivate::on_timeout(ev::timer& timer, int revents)
 {
-    watcher.stop();
-    if (revents & ev::ERROR) {
+    if (is_ev_error(revents)) [[unlikely]] {
         std::cerr << std::format("Error: timer watcher error: EV_ERROR\n");
         return;
     }
 
+    timer.stop();
     if (this->m_callback) {
         try {
             auto config = this->configure();

src/tlsconfigurator_p.h

@@ -12,26 +12,25 @@ class TLSConfiguratorPrivate {
 public:
     explicit TLSConfiguratorPrivate(const std::string& env_prefix);
 
-    std::shared_ptr<TLSServerContext> configure() const;
-    void watch(TLSConfigurator::watch_callback_t callback);
+    [[nodiscard]] std::shared_ptr<TLSServerContext> configure() const;
+    void watch(const TLSConfigurator::watch_callback_t& callback);
 
-    static constexpr ev_tstamp RECONFIGURATION_DELAY = 15;
+    static constexpr ev_tstamp RECONFIGURATION_DELAY = 15.0;
 
 private:
-    std::string m_env_prefix;
-    bool m_https = false;
     std::string m_certificate{};
     std::string m_key{};
     std::string m_ca{};
     std::string m_trusted_certificate{};
     std::string m_tls_protocols{};
     std::string m_tls_ciphers{};
     std::string m_tls_curves{};
-    bool m_tls_verify_client = false;
-    bool m_tls_enable_dhe    = false;
     ev::stat m_stat_watcher{};
     ev::timer m_timer{};
     TLSConfigurator::watch_callback_t m_callback = nullptr;
+    bool m_https                                 = false;
+    bool m_tls_verify_client                     = false;
+    bool m_tls_enable_dhe                        = false;
 
     void on_stat(ev::stat& watcher, int revents);
     void on_timeout(ev::timer& timer, int revents);

src/tlsexception.cpp

@@ -0,0 +1,4 @@
+#include "tlsexception.h"
+
+// This is to ensure that the vtable is only emitted once.
+TLSException::~TLSException() = default;