feat: simplified ConfigItem class

ConfigItem is now based on std::optional and std::string
reverted condition variable wait since an occasional failure
was occuring during test runs (not seen previously).

Signed-off-by: James Chapman <[email protected]>

lib/staging/tls/tests/tls_connection_test.hpp

@@ -195,14 +195,14 @@ class TlsTest : public testing::Test {
         server_config.service = "8444";
         server_config.ipv6_only = false;
         server_config.verify_client = false;
-        server_config.io_timeout_ms = 500; // no lower than 200ms
+        server_config.io_timeout_ms = 1000; // no lower than 200ms, valgrind need much higher
 
         client_config.cipher_list = "ECDHE-ECDSA-AES128-SHA256";
         // client_config.ciphersuites = "TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384";
         // client_config.certificate_chain_file = "client_chain.pem";
         // client_config.private_key_file = "client_priv.pem";
         client_config.verify_locations_file = "server_root_cert.pem";
-        client_config.io_timeout_ms = 200;
+        client_config.io_timeout_ms = 1000;
         client_config.verify_server = true;
         client_config.status_request = false;
         client_config.status_request_v2 = false;

lib/staging/tls/tests/tls_test.cpp

@@ -52,6 +52,7 @@ TEST(string, use) {
 }
 
 TEST(ConfigItem, test) {
+    // tests reduced with new ConfigStore implementation
     tls::ConfigItem i1;
     tls::ConfigItem i2{nullptr};
     tls::ConfigItem i3{"Hello"};
@@ -64,7 +65,7 @@ TEST(ConfigItem, test) {
     EXPECT_EQ(i5, nullptr);
 
     EXPECT_EQ(i2, i5);
-    EXPECT_EQ(i3, i6);
+    EXPECT_STREQ(i3, i6);
 
     EXPECT_EQ(i1, i2);
     EXPECT_NE(i1, i3);
@@ -75,29 +76,29 @@ TEST(ConfigItem, test) {
 
     auto j1(std::move(i3));
     j2 = std::move(i6);
-    EXPECT_EQ(i6, i3);
-    EXPECT_EQ(j1, j2);
-    EXPECT_EQ(j1, "Hello");
+    EXPECT_STREQ(i6, i3);
+    EXPECT_STREQ(j1, j2);
+    EXPECT_STREQ(j1, "Hello");
     EXPECT_NE(j1, i6);
 
     EXPECT_NE(j1, nullptr);
     EXPECT_NE(j2, nullptr);
 
-    EXPECT_EQ(i3, nullptr);
-    EXPECT_EQ(i6, nullptr);
-    EXPECT_EQ(i6, i3);
+    // EXPECT_EQ(i3, nullptr);
+    // EXPECT_EQ(i6, nullptr);
+    // EXPECT_EQ(i6, i3);
 
     std::vector<tls::ConfigItem> j3 = {"one", "two", nullptr};
-    EXPECT_EQ(j3[0], "one");
-    EXPECT_EQ(j3[1], "two");
+    EXPECT_STREQ(j3[0], "one");
+    EXPECT_STREQ(j3[1], "two");
     EXPECT_EQ(j3[2], nullptr);
 
     const char* p = j1;
     EXPECT_STREQ(p, "Hello");
     j1 = "Goodbye";
     EXPECT_STRNE(j1, "Hello");
     j1 = j2;
-    EXPECT_EQ(j1, j2);
+    EXPECT_STREQ(j1, j2);
 }
 
 using namespace tls::trusted_ca_keys;

lib/staging/tls/tls.cpp

@@ -471,62 +471,10 @@ bool configure_ssl_ctx(SSL_CTX* ctx, const char* ciphersuites, const char* ciphe
     return result;
 }
 
-/**
- * \brief duplicate a string checking for nullptr
- * \param[in] value a pointer to a \0 terminated string or nullptr
- * \return a copy of the string (which must be freed) or nullptr
- */
-constexpr char* dup(const char* value) {
-    char* res = nullptr;
-    if (value != nullptr) {
-        res = strdup(value);
-    }
-    return res;
-}
-
 } // namespace
 
 namespace tls {
 
-ConfigItem::ConfigItem(const char* value) : m_ptr(dup(value)) {
-}
-ConfigItem& ConfigItem::operator=(const char* value) {
-    free(m_ptr);
-    m_ptr = dup(value);
-    return *this;
-}
-ConfigItem::ConfigItem(const ConfigItem& obj) : m_ptr(dup(obj.m_ptr)) {
-}
-ConfigItem& ConfigItem::operator=(const ConfigItem& obj) {
-    free(m_ptr);
-    m_ptr = dup(obj.m_ptr);
-    return *this;
-}
-ConfigItem::ConfigItem(ConfigItem&& obj) noexcept : m_ptr(obj.m_ptr) {
-    obj.m_ptr = nullptr;
-}
-ConfigItem& ConfigItem::operator=(ConfigItem&& obj) noexcept {
-    free(m_ptr);
-    m_ptr = obj.m_ptr;
-    obj.m_ptr = nullptr;
-    return *this;
-}
-ConfigItem::~ConfigItem() {
-    free(m_ptr);
-    m_ptr = nullptr;
-}
-
-bool ConfigItem::operator==(const char* ptr) const {
-    bool result{false};
-    if (m_ptr == ptr) {
-        // both nullptr, or both point to the same string
-        result = true;
-    } else if ((m_ptr != nullptr) && (ptr != nullptr)) {
-        result = strcmp(m_ptr, ptr) == 0;
-    }
-    return result;
-}
-
 using SSL_ptr = std::unique_ptr<SSL>;
 using SSL_CTX_ptr = std::unique_ptr<SSL_CTX>;
 using OCSP_RESPONSE_ptr = std::shared_ptr<OCSP_RESPONSE>;
@@ -1198,16 +1146,12 @@ void Server::stop() {
 
 void Server::wait_running() {
     std::unique_lock lock(m_cv_mutex);
-    while (!m_running) {
-        m_cv.wait(lock);
-    }
+    m_cv.wait(lock, [this]() { return this->m_running; });
 }
 
 void Server::wait_stopped() {
     std::unique_lock lock(m_cv_mutex);
-    while (m_running) {
-        m_cv.wait(lock);
-    }
+    m_cv.wait(lock, [this]() { return !this->m_running; });
 }
 
 // ----------------------------------------------------------------------------

lib/staging/tls/tls.hpp

@@ -15,6 +15,7 @@
 #include <functional>
 #include <memory>
 #include <mutex>
+#include <optional>
 #include <pthread.h>
 #include <string>
 #include <unistd.h>
@@ -41,36 +42,17 @@ struct client_ctx;
  */
 class ConfigItem {
 private:
-    char* m_ptr{nullptr};
+    std::optional<std::string> value{};
 
 public:
     ConfigItem() = default;
-    ConfigItem(const char* value); // must not be explicit
-    ConfigItem& operator=(const char* value);
-    ConfigItem(const ConfigItem& obj);
-    ConfigItem& operator=(const ConfigItem& obj);
-    ConfigItem(ConfigItem&& obj) noexcept;
-    ConfigItem& operator=(ConfigItem&& obj) noexcept;
-
-    ~ConfigItem();
-
-    bool operator==(const char* ptr) const;
-
-    // must not be explicit
-    inline operator const char*() const {
-        return m_ptr;
-    }
-
-    inline bool operator!=(const char* ptr) const {
-        return !(*this == ptr);
-    }
-
-    inline bool operator==(const ConfigItem& obj) const {
-        return *this == obj.m_ptr;
+    ConfigItem(const char* ptr) {
+        if (ptr != nullptr) {
+            value = std::string(ptr);
+        }
     }
-
-    inline bool operator!=(const ConfigItem& obj) const {
-        return !(*this == obj);
+    inline operator const char*() const {
+        return (value) ? value.value().c_str() : nullptr;
     }
 };