From 59e22bf5ee911fa342573641a3e8396a7deb466e Mon Sep 17 00:00:00 2001
From: Andrew Beltrano <abeltrano@gmail.com>
Date: Tue, 12 Mar 2024 20:01:41 +0000
Subject: [PATCH 01/10] Change bool -> void for IHostapd::Enable.

---
 src/linux/wifi/core/AccessPointControllerLinux.cxx | 4 ++--
 src/linux/wpa-controller/Hostapd.cxx               | 9 ++++++---
 src/linux/wpa-controller/include/Wpa/Hostapd.hxx   | 5 +----
 src/linux/wpa-controller/include/Wpa/IHostapd.hxx  | 5 +----
 tests/unit/linux/wpa-controller/TestHostapd.cxx    | 8 ++++----
 5 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/src/linux/wifi/core/AccessPointControllerLinux.cxx b/src/linux/wifi/core/AccessPointControllerLinux.cxx
index a21670ce..ad137d34 100644
--- a/src/linux/wifi/core/AccessPointControllerLinux.cxx
+++ b/src/linux/wifi/core/AccessPointControllerLinux.cxx
@@ -108,7 +108,7 @@ AccessPointControllerLinux::SetOperationalState(AccessPointOperationalState oper
             status.Code = AccessPointOperationStatusCode::Succeeded;
         } catch (const Wpa::HostapdException& ex) {
             status.Code = AccessPointOperationStatusCode::InternalError;
-            status.Details = "failed to set operational state to 'enabled'";
+            status.Details = std::format("failed to set operational state to 'enabled' ({})", ex.what());
         }
         break;
     }
@@ -118,7 +118,7 @@ AccessPointControllerLinux::SetOperationalState(AccessPointOperationalState oper
             status.Code = AccessPointOperationStatusCode::Succeeded;
         } catch (const Wpa::HostapdException& ex) {
             status.Code = AccessPointOperationStatusCode::InternalError;
-            status.Details = "failed to set operational state to 'disabled'";
+            status.Details = std::format("failed to set operational state to 'disabled' ({})", ex.what());
         }
         break;
     }
diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx
index 705e21f7..40c460dd 100644
--- a/src/linux/wpa-controller/Hostapd.cxx
+++ b/src/linux/wpa-controller/Hostapd.cxx
@@ -120,7 +120,7 @@ Hostapd::SetProperty(std::string_view propertyName, std::string_view propertyVal
     return true;
 }
 
-bool
+void
 Hostapd::Enable()
 {
     static constexpr WpaCommand EnableCommand(ProtocolHostapd::CommandPayloadEnable);
@@ -131,7 +131,8 @@ Hostapd::Enable()
     }
 
     if (response->IsOk()) {
-        return true;
+        LOGV << std::format("Invalid response received when enabling hostapd interface (payload={})", response->Payload());
+        throw HostapdException("Failed to enable hostapd interface (invalid response)");
     }
 
     // The response will indicate failure if the interface is already enabled.
@@ -139,7 +140,9 @@ Hostapd::Enable()
     // This is only done if the 'enable' command fails since the GetStatus()
     // command is fairly heavy-weight in terms of its payload.
     const auto status = GetStatus();
-    return IsHostapdStateOperational(status.State);
+    if (!IsHostapdStateOperational(status.State)) {
+        throw HostapdException(std::format("Failed to enable hostapd interface (invalid state {})", magic_enum::enum_name(status.State)));
+    }
 }
 
 bool
diff --git a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
index 16984a10..6355ee3f 100644
--- a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
@@ -27,11 +27,8 @@ struct Hostapd :
 
     /**
      * @brief Enables the interface for use.
-     *
-     * @return true
-     * @return false
      */
-    bool
+    void
     Enable() override;
 
     /**
diff --git a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
index 295fc67f..9a8dbc14 100644
--- a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
@@ -51,11 +51,8 @@ struct IHostapd
 
     /**
      * @brief Enables the interface for use.
-     *
-     * @return true
-     * @return false
      */
-    virtual bool
+    virtual void
     Enable() = 0;
 
     /**
diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx
index e3e85f76..07776866 100644
--- a/tests/unit/linux/wpa-controller/TestHostapd.cxx
+++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx
@@ -79,7 +79,7 @@ TEST_CASE("Send command: GetStatus() (root)", "[wpa][hostapd][client][remote]")
     using namespace Wpa;
 
     Hostapd hostapd(WpaDaemonManager::InterfaceNameDefault);
-    REQUIRE(hostapd.Enable());
+    REQUIRE_NOTHROW(hostapd.Enable());
 
     SECTION("GetStatus() doesn't throw")
     {
@@ -97,7 +97,7 @@ TEST_CASE("Send command: GetStatus() (root)", "[wpa][hostapd][client][remote]")
         REQUIRE(hostapd.Disable());
         const auto statusInitial = hostapd.GetStatus();
         REQUIRE(statusInitial.State == HostapdInterfaceState::Disabled);
-        REQUIRE(hostapd.Enable());
+        REQUIRE_NOTHROW(hostapd.Enable());
         const auto statusDisabled = hostapd.GetStatus();
         REQUIRE(statusDisabled.State == HostapdInterfaceState::Enabled);
     }
@@ -274,7 +274,7 @@ TEST_CASE("Send control commands: Enable(), Disable() (root)", "[wpa][hostapd][c
         CHECK(hostapd.Disable());
         auto hostapdStatus = hostapd.GetStatus();
         REQUIRE(hostapdStatus.State != HostapdInterfaceState::Enabled);
-        REQUIRE(hostapd.Enable());
+        REQUIRE_NOTHROW(hostapd.Enable());
         hostapdStatus = hostapd.GetStatus();
         REQUIRE(hostapdStatus.State == HostapdInterfaceState::Enabled);
     }
@@ -282,7 +282,7 @@ TEST_CASE("Send control commands: Enable(), Disable() (root)", "[wpa][hostapd][c
     SECTION("Enable() preserves further communication")
     {
         CHECK(hostapd.Disable());
-        CHECK(hostapd.Enable());
+        CHECK_NOTHROW(hostapd.Enable());
         REQUIRE(hostapd.Ping());
     }
 

From 9e1a3e719ae75918ade2947f6a4558c07eb7d028 Mon Sep 17 00:00:00 2001
From: Andrew Beltrano <abeltrano@gmail.com>
Date: Tue, 12 Mar 2024 20:27:53 +0000
Subject: [PATCH 02/10] Change bool -> void for IHostapd::Disable

---
 src/linux/wpa-controller/Hostapd.cxx              | 13 +++++++------
 src/linux/wpa-controller/include/Wpa/Hostapd.hxx  |  5 +----
 src/linux/wpa-controller/include/Wpa/IHostapd.hxx |  5 +----
 tests/unit/linux/wpa-controller/TestHostapd.cxx   | 12 ++++++------
 4 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx
index 40c460dd..6b63ea38 100644
--- a/src/linux/wpa-controller/Hostapd.cxx
+++ b/src/linux/wpa-controller/Hostapd.cxx
@@ -103,7 +103,7 @@ Hostapd::SetProperty(std::string_view propertyName, std::string_view propertyVal
     }
 
     if (!response->IsOk()) {
-        LOGV << std::format("Invalid response received when setting hostapd property '{}' to '{}' (payload={})", propertyName, propertyValue, response->Payload());
+        LOGV << std::format("Invalid response received when setting hostapd property '{}' to '{}'\nResponse payload={}", propertyName, propertyValue, response->Payload());
         throw HostapdException(std::format("Failed to set hostapd property '{}' to '{}'", propertyName, propertyValue));
     }
 
@@ -131,8 +131,7 @@ Hostapd::Enable()
     }
 
     if (response->IsOk()) {
-        LOGV << std::format("Invalid response received when enabling hostapd interface (payload={})", response->Payload());
-        throw HostapdException("Failed to enable hostapd interface (invalid response)");
+        return;
     }
 
     // The response will indicate failure if the interface is already enabled.
@@ -145,7 +144,7 @@ Hostapd::Enable()
     }
 }
 
-bool
+void
 Hostapd::Disable()
 {
     static constexpr WpaCommand DisableCommand(ProtocolHostapd::CommandPayloadDisable);
@@ -156,7 +155,7 @@ Hostapd::Disable()
     }
 
     if (response->IsOk()) {
-        return true;
+        return;
     }
 
     // The response will indicate failure if the interface is already disabled.
@@ -164,7 +163,9 @@ Hostapd::Disable()
     // This is only done if the 'disable' command fails since the GetStatus()
     // command is fairly heavy-weight in terms of its payload.
     const auto status = GetStatus();
-    return !IsHostapdStateOperational(status.State);
+    if (IsHostapdStateOperational(status.State)) {
+        throw HostapdException(std::format("Failed to disable hostapd interface (invalid state {})", magic_enum::enum_name(status.State)));
+    }
 }
 
 bool
diff --git a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
index 6355ee3f..5993d21d 100644
--- a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
@@ -33,11 +33,8 @@ struct Hostapd :
 
     /**
      * @brief Disables the interface for use.
-     *
-     * @return true
-     * @return false
      */
-    bool
+    void
     Disable() override;
 
     /**
diff --git a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
index 9a8dbc14..b2a2b687 100644
--- a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
@@ -57,11 +57,8 @@ struct IHostapd
 
     /**
      * @brief Disables the interface for use.
-     *
-     * @return true
-     * @return false
      */
-    virtual bool
+    virtual void
     Disable() = 0;
 
     /**
diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx
index 07776866..cbf5583a 100644
--- a/tests/unit/linux/wpa-controller/TestHostapd.cxx
+++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx
@@ -94,7 +94,7 @@ TEST_CASE("Send command: GetStatus() (root)", "[wpa][hostapd][client][remote]")
 
     SECTION("GetStatus() reflects changes in interface state (disabled -> not disabled)")
     {
-        REQUIRE(hostapd.Disable());
+        REQUIRE_NOTHROW(hostapd.Disable());
         const auto statusInitial = hostapd.GetStatus();
         REQUIRE(statusInitial.State == HostapdInterfaceState::Disabled);
         REQUIRE_NOTHROW(hostapd.Enable());
@@ -106,7 +106,7 @@ TEST_CASE("Send command: GetStatus() (root)", "[wpa][hostapd][client][remote]")
     {
         const auto statusInitial = hostapd.GetStatus();
         REQUIRE(IsHostapdStateOperational(statusInitial.State));
-        REQUIRE(hostapd.Disable());
+        REQUIRE_NOTHROW(hostapd.Disable());
         const auto statusDisabled = hostapd.GetStatus();
         REQUIRE(!IsHostapdStateOperational(statusDisabled.State));
     }
@@ -271,7 +271,7 @@ TEST_CASE("Send control commands: Enable(), Disable() (root)", "[wpa][hostapd][c
 
     SECTION("Enable() enables the interface")
     {
-        CHECK(hostapd.Disable());
+        CHECK_NOTHROW(hostapd.Disable());
         auto hostapdStatus = hostapd.GetStatus();
         REQUIRE(hostapdStatus.State != HostapdInterfaceState::Enabled);
         REQUIRE_NOTHROW(hostapd.Enable());
@@ -281,7 +281,7 @@ TEST_CASE("Send control commands: Enable(), Disable() (root)", "[wpa][hostapd][c
 
     SECTION("Enable() preserves further communication")
     {
-        CHECK(hostapd.Disable());
+        CHECK_NOTHROW(hostapd.Disable());
         CHECK_NOTHROW(hostapd.Enable());
         REQUIRE(hostapd.Ping());
     }
@@ -293,14 +293,14 @@ TEST_CASE("Send control commands: Enable(), Disable() (root)", "[wpa][hostapd][c
 
     SECTION("Disable() disables the interface")
     {
-        CHECK(hostapd.Disable());
+        CHECK_NOTHROW(hostapd.Disable());
         const auto hostapdStatus = hostapd.GetStatus();
         REQUIRE(hostapdStatus.State == HostapdInterfaceState::Disabled);
     }
 
     SECTION("Disable() doesn't prevent further communication")
     {
-        CHECK(hostapd.Disable());
+        CHECK_NOTHROW(hostapd.Disable());
         REQUIRE(hostapd.Ping());
     }
 }

From 821bf20f2bf034f86946b016ec78d8457d08be34 Mon Sep 17 00:00:00 2001
From: Andrew Beltrano <abeltrano@gmail.com>
Date: Tue, 12 Mar 2024 20:31:39 +0000
Subject: [PATCH 03/10] Change bool -> void for IHostapd::Terminate

---
 src/linux/wpa-controller/Hostapd.cxx              | 7 +++++--
 src/linux/wpa-controller/include/Wpa/Hostapd.hxx  | 5 +----
 src/linux/wpa-controller/include/Wpa/IHostapd.hxx | 5 +----
 tests/unit/linux/wpa-controller/TestHostapd.cxx   | 2 +-
 4 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx
index 6b63ea38..4e2ae6da 100644
--- a/src/linux/wpa-controller/Hostapd.cxx
+++ b/src/linux/wpa-controller/Hostapd.cxx
@@ -168,7 +168,7 @@ Hostapd::Disable()
     }
 }
 
-bool
+void
 Hostapd::Terminate()
 {
     static constexpr WpaCommand TerminateCommand(ProtocolHostapd::CommandPayloadTerminate);
@@ -178,7 +178,10 @@ Hostapd::Terminate()
         throw HostapdException("Failed to send hostapd 'terminate' command");
     }
 
-    return response->IsOk();
+    if (!response->IsOk()) {
+        LOGV << std::format("Invalid response received when terminating hostapd\nResponse payload={}", response->Payload());
+        throw HostapdException("Failed to terminate hostapd process (invalid response)");
+    }
 }
 
 bool
diff --git a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
index 5993d21d..243dc15c 100644
--- a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
@@ -39,11 +39,8 @@ struct Hostapd :
 
     /**
      * @brief Terminates the process hosting the daemon.
-     *
-     * @return true
-     * @return false
      */
-    bool
+    void
     Terminate() override;
 
     /**
diff --git a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
index b2a2b687..2529f7b8 100644
--- a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
@@ -63,11 +63,8 @@ struct IHostapd
 
     /**
      * @brief Terminates the process hosting the daemon.
-     *
-     * @return true
-     * @return false
      */
-    virtual bool
+    virtual void
     Terminate() = 0;
 
     /**
diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx
index cbf5583a..36f2e465 100644
--- a/tests/unit/linux/wpa-controller/TestHostapd.cxx
+++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx
@@ -327,7 +327,7 @@ TEST_CASE("Send command: Terminate() ping failure (root)", "[wpa][hostapd][clien
 
     Hostapd hostapd(WpaDaemonManager::InterfaceNameDefault);
     REQUIRE(hostapd.Ping());
-    REQUIRE(hostapd.Terminate());
+    REQUIRE_NOTHROW(hostapd.Terminate());
 
     // The terminate command merely requests hostapd to shut down. The daemon's
     // polling loop must enter its next cycle before the termination process is

From edcad4d4ccaec60f8d6629cc143816edb568ccca Mon Sep 17 00:00:00 2001
From: Andrew Beltrano <abeltrano@gmail.com>
Date: Tue, 12 Mar 2024 20:35:14 +0000
Subject: [PATCH 04/10] Change bool -> void for IHostapd::Ping

---
 src/linux/wpa-controller/Hostapd.cxx              |  7 +++++--
 src/linux/wpa-controller/include/Wpa/Hostapd.hxx  |  2 +-
 src/linux/wpa-controller/include/Wpa/IHostapd.hxx |  2 +-
 tests/unit/linux/wpa-controller/TestHostapd.cxx   | 10 +++++-----
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx
index 4e2ae6da..55bd2162 100644
--- a/src/linux/wpa-controller/Hostapd.cxx
+++ b/src/linux/wpa-controller/Hostapd.cxx
@@ -33,7 +33,7 @@ Hostapd::GetInterface()
     return m_interface;
 }
 
-bool
+void
 Hostapd::Ping()
 {
     static constexpr WpaCommand PingCommand(ProtocolHostapd::CommandPayloadPing);
@@ -43,7 +43,10 @@ Hostapd::Ping()
         throw HostapdException("Failed to ping hostapd");
     }
 
-    return response->Payload().starts_with(ProtocolHostapd::ResponsePayloadPing);
+    if (!response->Payload().starts_with(ProtocolHostapd::ResponsePayloadPing)) {
+        LOGV << std::format("Invalid response received when sending hostapd ping\nResponse payload={}", response->Payload());
+        throw HostapdException("Invalid response received when pinging hostapd");
+    }
 }
 
 bool
diff --git a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
index 243dc15c..c6fc3cdb 100644
--- a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
@@ -46,7 +46,7 @@ struct Hostapd :
     /**
      * @brief Checks connectivity to the hostapd daemon.
      */
-    bool
+    void
     Ping() override;
 
     /**
diff --git a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
index 2529f7b8..a4517814 100644
--- a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
@@ -70,7 +70,7 @@ struct IHostapd
     /**
      * @brief Checks connectivity to the hostapd daemon.
      */
-    virtual bool
+    virtual void
     Ping() = 0;
 
     /**
diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx
index 36f2e465..e93a2b98 100644
--- a/tests/unit/linux/wpa-controller/TestHostapd.cxx
+++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx
@@ -64,13 +64,13 @@ TEST_CASE("Send Ping() command (root)", "[wpa][hostapd][client][remote]")
 
     SECTION("Send Ping() command")
     {
-        REQUIRE(hostapd.Ping());
+        REQUIRE_NOTHROW(hostapd.Ping());
     }
 
     SECTION("Send Ping() command on second instance")
     {
         Hostapd hostapd2(WpaDaemonManager::InterfaceNameDefault);
-        REQUIRE(hostapd2.Ping());
+        REQUIRE_NOTHROW(hostapd2.Ping());
     }
 }
 
@@ -283,7 +283,7 @@ TEST_CASE("Send control commands: Enable(), Disable() (root)", "[wpa][hostapd][c
     {
         CHECK_NOTHROW(hostapd.Disable());
         CHECK_NOTHROW(hostapd.Enable());
-        REQUIRE(hostapd.Ping());
+        REQUIRE_NOTHROW(hostapd.Ping());
     }
 
     SECTION("Disable() doesn't throw")
@@ -301,7 +301,7 @@ TEST_CASE("Send control commands: Enable(), Disable() (root)", "[wpa][hostapd][c
     SECTION("Disable() doesn't prevent further communication")
     {
         CHECK_NOTHROW(hostapd.Disable());
-        REQUIRE(hostapd.Ping());
+        REQUIRE_NOTHROW(hostapd.Ping());
     }
 }
 
@@ -326,7 +326,7 @@ TEST_CASE("Send command: Terminate() ping failure (root)", "[wpa][hostapd][clien
     static constexpr auto TerminationWaitTime{ 2s }; // NOLINT
 
     Hostapd hostapd(WpaDaemonManager::InterfaceNameDefault);
-    REQUIRE(hostapd.Ping());
+    REQUIRE_NOTHROW(hostapd.Ping());
     REQUIRE_NOTHROW(hostapd.Terminate());
 
     // The terminate command merely requests hostapd to shut down. The daemon's

From 6a33887b7036ee006cac8303eccb9cc26b028f1d Mon Sep 17 00:00:00 2001
From: Andrew Beltrano <abeltrano@gmail.com>
Date: Tue, 12 Mar 2024 20:42:42 +0000
Subject: [PATCH 05/10] Change bool -> void for IHostapd::Reload

---
 src/linux/wifi/core/AccessPointControllerLinux.cxx |  7 ++++---
 src/linux/wpa-controller/Hostapd.cxx               | 14 +++++++++-----
 src/linux/wpa-controller/include/Wpa/Hostapd.hxx   |  5 +----
 src/linux/wpa-controller/include/Wpa/IHostapd.hxx  |  5 +----
 4 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/linux/wifi/core/AccessPointControllerLinux.cxx b/src/linux/wifi/core/AccessPointControllerLinux.cxx
index ad137d34..74c8c4ce 100644
--- a/src/linux/wifi/core/AccessPointControllerLinux.cxx
+++ b/src/linux/wifi/core/AccessPointControllerLinux.cxx
@@ -192,10 +192,11 @@ AccessPointControllerLinux::SetProtocol(Ieee80211Protocol ieeeProtocol) noexcept
     }
 
     // Reload the hostapd configuration to pick up the changes.
-    hostapdOperationSucceeded = m_hostapd.Reload();
-    if (!hostapdOperationSucceeded) {
+    try {
+        m_hostapd.Reload();
+    } catch (const Wpa::HostapdException& ex) {
         status.Code = AccessPointOperationStatusCode::InternalError;
-        status.Details = "failed to reload hostapd configuration";
+        status.Details = std::format("failed to reload hostapd configuration - {}", ex.what());
         return status;
     }
 
diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx
index 55bd2162..374a6e73 100644
--- a/src/linux/wpa-controller/Hostapd.cxx
+++ b/src/linux/wpa-controller/Hostapd.cxx
@@ -49,7 +49,7 @@ Hostapd::Ping()
     }
 }
 
-bool
+void
 Hostapd::Reload()
 {
     static constexpr WpaCommand ReloadCommand(ProtocolHostapd::CommandPayloadReload);
@@ -59,7 +59,10 @@ Hostapd::Reload()
         throw HostapdException("Failed to send hostapd 'reload' command");
     }
 
-    return response->IsOk();
+    if (!response->IsOk()) {
+        LOGV << std::format("Invalid response received when reloading hostapd configuration\nResponse payload={}", response->Payload());
+        throw HostapdException("Invalid response received when reloading hostapd configuration");
+    }
 }
 
 HostapdStatus
@@ -115,9 +118,10 @@ Hostapd::SetProperty(std::string_view propertyName, std::string_view propertyVal
         return true;
     }
 
-    const bool configurationReloadSucceeded = Reload();
-    if (!configurationReloadSucceeded) {
-        throw HostapdException(std::format("Failed to reload hostapd configuration after '{}' property change", propertyName));
+    try {
+        Reload();
+    } catch (HostapdException& e) {
+        throw HostapdException(std::format("Failed to reload hostapd configuration after '{}' property change: {}", propertyName, e.what()));
     }
 
     return true;
diff --git a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
index c6fc3cdb..72663ce0 100644
--- a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
@@ -51,11 +51,8 @@ struct Hostapd :
 
     /**
      * @brief Reloads the interface. This will cause any settings that have been changed to take effect.
-     *
-     * @return true If the configuration was reloaded successfully.
-     * @return false If the configuration was not reloaded successfully.
      */
-    bool
+    void
     Reload() override;
 
     /**
diff --git a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
index a4517814..1338619b 100644
--- a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
@@ -75,11 +75,8 @@ struct IHostapd
 
     /**
      * @brief Reloads the interface. This will cause any settings that have been changed to take effect.
-     *
-     * @return true If the configuration was reloaded successfully.
-     * @return false If the configuration was not reloaded successfully.
      */
-    virtual bool
+    virtual void
     Reload() = 0;
 
     /**

From ea59e6f3248da92c3a64d5ff557f3b0673bf6453 Mon Sep 17 00:00:00 2001
From: Andrew Beltrano <abeltrano@gmail.com>
Date: Tue, 12 Mar 2024 20:56:06 +0000
Subject: [PATCH 06/10] Change bool -> std::string for IHostapd::GetProperty

---
 src/linux/wpa-controller/Hostapd.cxx          | 13 +++++-----
 .../wpa-controller/include/Wpa/Hostapd.hxx    |  8 +++---
 .../wpa-controller/include/Wpa/IHostapd.hxx   |  8 +++---
 .../unit/linux/wpa-controller/TestHostapd.cxx | 26 +++++++------------
 4 files changed, 22 insertions(+), 33 deletions(-)

diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx
index 374a6e73..8971a8c0 100644
--- a/src/linux/wpa-controller/Hostapd.cxx
+++ b/src/linux/wpa-controller/Hostapd.cxx
@@ -78,8 +78,8 @@ Hostapd::GetStatus()
     return response->Status;
 }
 
-bool
-Hostapd::GetProperty(std::string_view propertyName, std::string& propertyValue)
+std::string
+Hostapd::GetProperty(std::string_view propertyName)
 {
     const WpaCommandGet getCommand(propertyName);
     const auto response = m_controller.SendCommand(getCommand);
@@ -90,11 +90,12 @@ Hostapd::GetProperty(std::string_view propertyName, std::string& propertyValue)
     // Check Failed() instead of IsOk() since the response will indicate failure
     // with "FAIL", otherwise, the payload is the property value (not "OK").
     if (response->Failed()) {
-        return false;
+        LOGV << std::format("Invalid response received when getting hostapd property '{}'\nResponse payload={}", propertyName, response->Payload());
+        throw HostapdException(std::format("Failed to get hostapd property '{}' (invalid response)", propertyName));
     }
 
-    propertyValue = response->Payload();
-    return true;
+    std::string propertyValue{ response->Payload() };
+    return propertyValue;
 }
 
 bool
@@ -110,7 +111,7 @@ Hostapd::SetProperty(std::string_view propertyName, std::string_view propertyVal
 
     if (!response->IsOk()) {
         LOGV << std::format("Invalid response received when setting hostapd property '{}' to '{}'\nResponse payload={}", propertyName, propertyValue, response->Payload());
-        throw HostapdException(std::format("Failed to set hostapd property '{}' to '{}'", propertyName, propertyValue));
+        throw HostapdException(std::format("Failed to set hostapd property '{}' to '{}' (invalid response)", propertyName, propertyValue));
     }
 
     if (enforceConfigurationChange == EnforceConfigurationChange::Defer) {
diff --git a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
index 72663ce0..281f81a7 100644
--- a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
@@ -75,12 +75,10 @@ struct Hostapd :
      * @brief Get a property value for the interface.
      *
      * @param propertyName The name of the property to retrieve.
-     * @param propertyValue The string to store the property value in.
-     * @return true If the property value was obtained and its value is in 'propertyValue'.
-     * @return false If t he property value could not be obtained due to an error.
+     * @return std::string The property string value.
      */
-    bool
-    GetProperty(std::string_view propertyName, std::string& propertyValue) override;
+    std::string
+    GetProperty(std::string_view propertyName) override;
 
     /**
      * @brief Set a property on the interface.
diff --git a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
index 1338619b..e85f5f5a 100644
--- a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
@@ -99,12 +99,10 @@ struct IHostapd
      * @brief Get a property value for the interface.
      *
      * @param propertyName The name of the property to retrieve.
-     * @param propertyValue The string to store the property value in.
-     * @return true If the property value was obtained and its value is in 'propertyValue'.
-     * @return false If t he property value could not be obtained due to an error.
+     * @return std::string The property string value.
      */
-    virtual bool
-    GetProperty(std::string_view propertyName, std::string& propertyValue) = 0;
+    virtual std::string
+    GetProperty(std::string_view propertyName) = 0;
 
     /**
      * @brief Set a property on the interface.
diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx
index e93a2b98..0fee6a87 100644
--- a/tests/unit/linux/wpa-controller/TestHostapd.cxx
+++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx
@@ -169,35 +169,27 @@ TEST_CASE("Send GetProperty() command (root)", "[wpa][hostapd][client][remote]")
 
     Hostapd hostapd(WpaDaemonManager::InterfaceNameDefault);
 
-    SECTION("GetProperty() doesn't throw")
+    SECTION("doesn't throw for valid property")
     {
-        std::string whateverValue;
-        REQUIRE_NOTHROW(hostapd.GetProperty("whatever", whateverValue));
+        REQUIRE_NOTHROW(hostapd.GetProperty(ProtocolHostapd::PropertyNameVersion));
     }
 
-    SECTION("GetProperty() returns false for invalid property")
-    {
-        std::string whateverValue;
-        REQUIRE_FALSE(hostapd.GetProperty("whatever", whateverValue));
-    }
-
-    SECTION("GetProperty() returns true for valid property")
+    SECTION("doesn't throw for valid property with non-empty value")
     {
         std::string versionValue;
-        REQUIRE(hostapd.GetProperty(ProtocolHostapd::PropertyNameVersion, versionValue));
+        REQUIRE_NOTHROW(versionValue = hostapd.GetProperty(ProtocolHostapd::PropertyNameVersion));
+        REQUIRE_FALSE(versionValue.empty());
     }
 
-    SECTION("GetProperty() returns true for valid property with non-empty value")
+    SECTION("throws for invalid property")
     {
-        std::string versionValue;
-        REQUIRE(hostapd.GetProperty(ProtocolHostapd::PropertyNameVersion, versionValue));
-        REQUIRE_FALSE(versionValue.empty());
+        REQUIRE_THROWS_AS(hostapd.GetProperty("whatever"), HostapdException);
     }
 
-    SECTION("GetProperty() returns correct value for valid property")
+    SECTION("returns correct value for valid property")
     {
         std::string versionValue;
-        CHECK(hostapd.GetProperty(ProtocolHostapd::PropertyNameVersion, versionValue));
+        CHECK_NOTHROW(versionValue = hostapd.GetProperty(ProtocolHostapd::PropertyNameVersion));
         REQUIRE(versionValue == ProtocolHostapd::PropertyVersionValue);
     }
 }

From 5847b37b460c2ab3d207b6f2b616791f22c84de6 Mon Sep 17 00:00:00 2001
From: Andrew Beltrano <abeltrano@gmail.com>
Date: Tue, 12 Mar 2024 21:01:43 +0000
Subject: [PATCH 07/10] Change bool -> void for IHostapd::SetSsid

---
 src/linux/wpa-controller/Hostapd.cxx              | 6 ++++--
 src/linux/wpa-controller/include/Wpa/Hostapd.hxx  | 4 +---
 src/linux/wpa-controller/include/Wpa/IHostapd.hxx | 5 ++---
 tests/unit/linux/wpa-controller/TestHostapd.cxx   | 9 ++-------
 4 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx
index 8971a8c0..7d94e96b 100644
--- a/src/linux/wpa-controller/Hostapd.cxx
+++ b/src/linux/wpa-controller/Hostapd.cxx
@@ -192,11 +192,13 @@ Hostapd::Terminate()
     }
 }
 
-bool
+void
 Hostapd::SetSsid(std::string_view ssid, EnforceConfigurationChange enforceConfigurationChange)
 {
     const bool ssidWasSet = SetProperty(ProtocolHostapd::PropertyNameSsid, ssid, enforceConfigurationChange);
-    return ssidWasSet;
+    if (!ssidWasSet) {
+        throw HostapdException(std::format("Failed to set hostapd 'ssid' property to '{}'", ssid));
+    }
 }
 
 bool
diff --git a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
index 281f81a7..d0814f99 100644
--- a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
@@ -97,10 +97,8 @@ struct Hostapd :
      *
      * @param ssid The ssid to set.
      * @param enforceConfigurationChange When the enforce the configuration change. A value of 'Now' will trigger a configuration reload.
-     * @return true If the ssid was set successfully.
-     * @return false If the ssid was not set successfully.
      */
-    bool
+    void
     SetSsid(std::string_view ssid, EnforceConfigurationChange enforceConfigurationChange = EnforceConfigurationChange::Now) override;
 
     /**
diff --git a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
index e85f5f5a..ec3c00bd 100644
--- a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
@@ -120,10 +120,9 @@ struct IHostapd
      * @brief Set the ssid for the interface.
      *
      * @param ssid The ssid to set.
-     * @return true If the ssid was set successfully.
-     * @return false If the ssid was not set successfully.
+     * @param enforceConfigurationChange When the enforce the configuration change. A value of 'Now' will trigger a configuration reload.
      */
-    virtual bool
+    virtual void
     SetSsid(std::string_view ssid, EnforceConfigurationChange enforceConfigurationChange) = 0;
 
     /**
diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx
index 0fee6a87..179dc7fc 100644
--- a/tests/unit/linux/wpa-controller/TestHostapd.cxx
+++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx
@@ -338,16 +338,11 @@ TEST_CASE("Send SetSsid() command (root)", "[wpa][hostapd][client][remote]")
 
     Hostapd hostapd(WpaDaemonManager::InterfaceNameDefault);
 
-    SECTION("SetSsid() doesn't throw")
+    SECTION("doesn't throw for valid SSID")
     {
         REQUIRE_NOTHROW(hostapd.SetSsid(SsidValid));
     }
 
-    SECTION("SetSsid() returns true for valid SSID")
-    {
-        REQUIRE(hostapd.SetSsid(SsidValid));
-    }
-
     SECTION("SetSsid() throws for empty SSID")
     {
         REQUIRE_THROWS_AS(hostapd.SetSsid(""), HostapdException);
@@ -361,7 +356,7 @@ TEST_CASE("Send SetSsid() command (root)", "[wpa][hostapd][client][remote]")
     SECTION("SetSsid() changes the SSID for valid input")
     {
         constexpr auto SsidToSet{ SsidValid };
-        REQUIRE(hostapd.SetSsid(SsidToSet));
+        REQUIRE_NOTHROW(hostapd.SetSsid(SsidToSet));
         const auto status = hostapd.GetStatus();
         REQUIRE(!std::empty(status.Bss));
         REQUIRE(status.Bss[0].Ssid == SsidToSet);

From 0f64637f968f2ffeda9ada84dd80530ff2fc6fe6 Mon Sep 17 00:00:00 2001
From: Andrew Beltrano <abeltrano@gmail.com>
Date: Tue, 12 Mar 2024 21:05:09 +0000
Subject: [PATCH 08/10] Change bool -> void for IHostapd::SetWpaProtocols

---
 src/linux/wpa-controller/Hostapd.cxx             |  4 +---
 src/linux/wpa-controller/include/Wpa/Hostapd.hxx |  4 +---
 .../wpa-controller/include/Wpa/IHostapd.hxx      |  4 +---
 tests/unit/linux/wpa-controller/TestHostapd.cxx  | 16 ++++++++--------
 4 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx
index 7d94e96b..d7de7b3e 100644
--- a/src/linux/wpa-controller/Hostapd.cxx
+++ b/src/linux/wpa-controller/Hostapd.cxx
@@ -201,7 +201,7 @@ Hostapd::SetSsid(std::string_view ssid, EnforceConfigurationChange enforceConfig
     }
 }
 
-bool
+void
 Hostapd::SetWpaProtocols(std::vector<WpaProtocol> protocols, EnforceConfigurationChange enforceConfigurationChange)
 {
     if (std::empty(protocols)) {
@@ -224,8 +224,6 @@ Hostapd::SetWpaProtocols(std::vector<WpaProtocol> protocols, EnforceConfiguratio
     if (!protocolsWereSet) {
         throw HostapdException(std::format("Failed to set hostapd 'wpa' property to '{}'", protocolsValue));
     }
-
-    return true;
 }
 
 bool
diff --git a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
index d0814f99..572d444d 100644
--- a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
@@ -106,10 +106,8 @@ struct Hostapd :
      *
      * @param protocols The protocols to set.
      * @param enforceConfigurationChange When the enforce the configuration change. A value of 'Now' will trigger a configuration reload.
-     * @return true If the protocols were set successfully.
-     * @return false If the protocols were not set successfully.
      */
-    bool
+    void
     SetWpaProtocols(std::vector<WpaProtocol> protocols, EnforceConfigurationChange enforceConfigurationChange = EnforceConfigurationChange::Now) override;
 
     /**
diff --git a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
index ec3c00bd..58ffecf7 100644
--- a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
@@ -130,10 +130,8 @@ struct IHostapd
      *
      * @param protocols The protocols to set.
      * @param enforceConfigurationChange When the enforce the configuration change. A value of 'Now' will trigger a configuration reload.
-     * @return true If the protocols were set successfully.
-     * @return false If the protocols were not set successfully.
      */
-    virtual bool
+    virtual void
     SetWpaProtocols(std::vector<WpaProtocol> protocols, EnforceConfigurationChange enforceConfigurationChange) = 0;
 
     /**
diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx
index 179dc7fc..c4fc37f3 100644
--- a/tests/unit/linux/wpa-controller/TestHostapd.cxx
+++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx
@@ -413,23 +413,23 @@ TEST_CASE("Send SetWpaProtocols() command (root)", "[wpa][hostapd][client][remot
 
     SECTION("Succeeds with valid, single input")
     {
-        REQUIRE(hostapd.SetWpaProtocols({ WpaProtocol::Wpa }, EnforceConfigurationChange::Now));
-        REQUIRE(hostapd.SetWpaProtocols({ WpaProtocol::Wpa }, EnforceConfigurationChange::Defer));
+        REQUIRE_NOTHROW(hostapd.SetWpaProtocols({ WpaProtocol::Wpa }, EnforceConfigurationChange::Now));
+        REQUIRE_NOTHROW(hostapd.SetWpaProtocols({ WpaProtocol::Wpa }, EnforceConfigurationChange::Defer));
     }
 
     SECTION("Succeeds with valid, multiple inputs")
     {
-        REQUIRE(hostapd.SetWpaProtocols({ WpaProtocol::Wpa, WpaProtocol::Wpa2 }, EnforceConfigurationChange::Now));
-        REQUIRE(hostapd.SetWpaProtocols({ WpaProtocol::Wpa, WpaProtocol::Wpa2 }, EnforceConfigurationChange::Defer));
+        REQUIRE_NOTHROW(hostapd.SetWpaProtocols({ WpaProtocol::Wpa, WpaProtocol::Wpa2 }, EnforceConfigurationChange::Now));
+        REQUIRE_NOTHROW(hostapd.SetWpaProtocols({ WpaProtocol::Wpa, WpaProtocol::Wpa2 }, EnforceConfigurationChange::Defer));
     }
 
     SECTION("Succeeds with valid, duplicate inputs")
     {
-        REQUIRE(hostapd.SetWpaProtocols({ WpaProtocol::Wpa, WpaProtocol::Wpa }, EnforceConfigurationChange::Now));
-        REQUIRE(hostapd.SetWpaProtocols({ WpaProtocol::Wpa, WpaProtocol::Wpa }, EnforceConfigurationChange::Defer));
+        REQUIRE_NOTHROW(hostapd.SetWpaProtocols({ WpaProtocol::Wpa, WpaProtocol::Wpa }, EnforceConfigurationChange::Now));
+        REQUIRE_NOTHROW(hostapd.SetWpaProtocols({ WpaProtocol::Wpa, WpaProtocol::Wpa }, EnforceConfigurationChange::Defer));
 
-        REQUIRE(hostapd.SetWpaProtocols({ WpaProtocol::Wpa2, WpaProtocol::Wpa2 }, EnforceConfigurationChange::Now));
-        REQUIRE(hostapd.SetWpaProtocols({ WpaProtocol::Wpa2, WpaProtocol::Wpa2 }, EnforceConfigurationChange::Defer));
+        REQUIRE_NOTHROW(hostapd.SetWpaProtocols({ WpaProtocol::Wpa2, WpaProtocol::Wpa2 }, EnforceConfigurationChange::Now));
+        REQUIRE_NOTHROW(hostapd.SetWpaProtocols({ WpaProtocol::Wpa2, WpaProtocol::Wpa2 }, EnforceConfigurationChange::Defer));
     }
 }
 

From 90dbe0d12574a66338f8ffd5a1fef4a848283357 Mon Sep 17 00:00:00 2001
From: Andrew Beltrano <abeltrano@gmail.com>
Date: Tue, 12 Mar 2024 21:08:26 +0000
Subject: [PATCH 09/10] Change bool -> void for IHostapd::SetKeyManagement

---
 src/linux/wpa-controller/Hostapd.cxx              |  4 +---
 src/linux/wpa-controller/include/Wpa/Hostapd.hxx  |  4 +---
 src/linux/wpa-controller/include/Wpa/IHostapd.hxx |  4 +---
 tests/unit/linux/wpa-controller/TestHostapd.cxx   | 12 ++++++------
 4 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx
index d7de7b3e..810e95e0 100644
--- a/src/linux/wpa-controller/Hostapd.cxx
+++ b/src/linux/wpa-controller/Hostapd.cxx
@@ -226,7 +226,7 @@ Hostapd::SetWpaProtocols(std::vector<WpaProtocol> protocols, EnforceConfiguratio
     }
 }
 
-bool
+void
 Hostapd::SetKeyManagement(std::vector<WpaKeyManagement> keyManagements, EnforceConfigurationChange enforceConfigurationChange)
 {
     if (std::empty(keyManagements)) {
@@ -252,6 +252,4 @@ Hostapd::SetKeyManagement(std::vector<WpaKeyManagement> keyManagements, EnforceC
     if (!keyManagementWasSet) {
         throw HostapdException(std::format("Failed to set hostapd 'wpa_key_mgmt' property to '{}'", keyManagementPropertyValue));
     }
-
-    return true;
 }
diff --git a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
index 572d444d..e0e821c5 100644
--- a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
@@ -115,10 +115,8 @@ struct Hostapd :
      *
      * @param keyManagements The key management value(s) to set.
      * @param enforceConfigurationChange When the enforce the configuration change. A value of 'Now' will trigger a configuration reload.
-     * @return true The key management value(s) were set successfully.
-     * @return false The key management value(s) were not set successfully.
      */
-    bool
+    void
     SetKeyManagement(std::vector<WpaKeyManagement> keyManagements, EnforceConfigurationChange enforceConfigurationChange = EnforceConfigurationChange::Now) override;
 
 private:
diff --git a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
index 58ffecf7..7a503c2a 100644
--- a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
@@ -139,10 +139,8 @@ struct IHostapd
      *
      * @param keyManagements The key management value(s) to set.
      * @param enforceConfigurationChange When the enforce the configuration change. A value of 'Now' will trigger a configuration reload.
-     * @return true The key management value(s) were set successfully.
-     * @return false The key management value(s) were not set successfully.
      */
-    virtual bool
+    virtual void
     SetKeyManagement(std::vector<WpaKeyManagement> keyManagements, EnforceConfigurationChange enforceConfigurationChange) = 0;
 };
 
diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx
index c4fc37f3..0916d698 100644
--- a/tests/unit/linux/wpa-controller/TestHostapd.cxx
+++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx
@@ -499,8 +499,8 @@ TEST_CASE("Send SetKeyManagement() command (root)", "[wpa][hostapd][client][remo
     SECTION("Succeeds with all valid, single inputs")
     {
         for (const auto keyManagementValidValue : KeyManagementValidValues) {
-            REQUIRE(hostapd.SetKeyManagement({ keyManagementValidValue }, EnforceConfigurationChange::Now));
-            REQUIRE(hostapd.SetKeyManagement({ keyManagementValidValue }, EnforceConfigurationChange::Defer));
+            REQUIRE_NOTHROW(hostapd.SetKeyManagement({ keyManagementValidValue }, EnforceConfigurationChange::Now));
+            REQUIRE_NOTHROW(hostapd.SetKeyManagement({ keyManagementValidValue }, EnforceConfigurationChange::Defer));
         }
     }
 
@@ -509,16 +509,16 @@ TEST_CASE("Send SetKeyManagement() command (root)", "[wpa][hostapd][client][remo
         std::vector<WpaKeyManagement> keyManagementValidValues{};
         for (const auto keyManagementValidValue : KeyManagementValidValues) {
             keyManagementValidValues.push_back(keyManagementValidValue);
-            REQUIRE(hostapd.SetKeyManagement(keyManagementValidValues, EnforceConfigurationChange::Now));
-            REQUIRE(hostapd.SetKeyManagement(keyManagementValidValues, EnforceConfigurationChange::Defer));
+            REQUIRE_NOTHROW(hostapd.SetKeyManagement(keyManagementValidValues, EnforceConfigurationChange::Now));
+            REQUIRE_NOTHROW(hostapd.SetKeyManagement(keyManagementValidValues, EnforceConfigurationChange::Defer));
         }
     }
 
     SECTION("Succeeds with valid, duplicate inputs")
     {
         for (const auto keyManagementValidValue : KeyManagementValidValues) {
-            REQUIRE(hostapd.SetKeyManagement({ keyManagementValidValue, keyManagementValidValue }, EnforceConfigurationChange::Now));
-            REQUIRE(hostapd.SetKeyManagement({ keyManagementValidValue, keyManagementValidValue }, EnforceConfigurationChange::Defer));
+            REQUIRE_NOTHROW(hostapd.SetKeyManagement({ keyManagementValidValue, keyManagementValidValue }, EnforceConfigurationChange::Now));
+            REQUIRE_NOTHROW(hostapd.SetKeyManagement({ keyManagementValidValue, keyManagementValidValue }, EnforceConfigurationChange::Defer));
         }
     }
 }

From acd831761c7400be725cbdc7aeb4937d47837da7 Mon Sep 17 00:00:00 2001
From: Andrew Beltrano <abeltrano@gmail.com>
Date: Tue, 12 Mar 2024 21:22:24 +0000
Subject: [PATCH 10/10] Change bool -> void for IHostapd::SetProperty

---
 .../wifi/core/AccessPointControllerLinux.cxx  |  9 ++---
 src/linux/wpa-controller/Hostapd.cxx          | 27 +++++++------
 .../wpa-controller/include/Wpa/Hostapd.hxx    |  4 +-
 .../wpa-controller/include/Wpa/IHostapd.hxx   |  4 +-
 .../unit/linux/wpa-controller/TestHostapd.cxx | 40 +++++++++----------
 5 files changed, 39 insertions(+), 45 deletions(-)

diff --git a/src/linux/wifi/core/AccessPointControllerLinux.cxx b/src/linux/wifi/core/AccessPointControllerLinux.cxx
index 74c8c4ce..58b69c7c 100644
--- a/src/linux/wifi/core/AccessPointControllerLinux.cxx
+++ b/src/linux/wifi/core/AccessPointControllerLinux.cxx
@@ -175,10 +175,7 @@ AccessPointControllerLinux::SetProtocol(Ieee80211Protocol ieeeProtocol) noexcept
     try {
         for (auto& propertyToSet : propertiesToSet) {
             std::tie(propertyKeyToSet, propertyValueToSet) = std::move(propertyToSet);
-            hostapdOperationSucceeded = m_hostapd.SetProperty(propertyKeyToSet, propertyValueToSet, EnforceConfigurationChange::Defer);
-            if (!hostapdOperationSucceeded) {
-                break;
-            }
+            m_hostapd.SetProperty(propertyKeyToSet, propertyValueToSet, EnforceConfigurationChange::Defer);
         }
     } catch (const Wpa::HostapdException& ex) {
         hostapdOperationSucceeded = false;
@@ -235,7 +232,7 @@ AccessPointControllerLinux::SetFrequencyBands(std::vector<Ieee80211FrequencyBand
 
     // Set the hostapd "setband" property.
     try {
-        hostapdOperationSucceeded = m_hostapd.SetProperty(propertyKeyToSet, propertyValueToSet, EnforceConfigurationChange::Now);
+        m_hostapd.SetProperty(propertyKeyToSet, propertyValueToSet, EnforceConfigurationChange::Now);
     } catch (const Wpa::HostapdException& ex) {
         hostapdOperationSucceeded = false;
         errorDetails = ex.what();
@@ -270,7 +267,7 @@ AccessPointControllerLinux::SetSsid(std::string_view ssid) noexcept
 
     // Attempt to set the SSID.
     try {
-        hostapdOperationSucceeded = m_hostapd.SetProperty(Wpa::ProtocolHostapd::PropertyNameSsid, ssid, EnforceConfigurationChange::Now);
+        m_hostapd.SetProperty(Wpa::ProtocolHostapd::PropertyNameSsid, ssid, EnforceConfigurationChange::Now);
     } catch (Wpa::HostapdException& ex) {
         hostapdOperationSucceeded = false;
         errorDetails = ex.what();
diff --git a/src/linux/wpa-controller/Hostapd.cxx b/src/linux/wpa-controller/Hostapd.cxx
index 810e95e0..3e0430d4 100644
--- a/src/linux/wpa-controller/Hostapd.cxx
+++ b/src/linux/wpa-controller/Hostapd.cxx
@@ -98,7 +98,7 @@ Hostapd::GetProperty(std::string_view propertyName)
     return propertyValue;
 }
 
-bool
+void
 Hostapd::SetProperty(std::string_view propertyName, std::string_view propertyValue, EnforceConfigurationChange enforceConfigurationChange)
 {
     LOGD << std::format("Attempting to set hostapd property '{}' (size={}) to '{}' (size={})", propertyName, std::size(propertyName), propertyValue, std::size(propertyValue));
@@ -116,7 +116,7 @@ Hostapd::SetProperty(std::string_view propertyName, std::string_view propertyVal
 
     if (enforceConfigurationChange == EnforceConfigurationChange::Defer) {
         LOGD << std::format("Skipping enforcement of '{}' configuration change (requested)", propertyName);
-        return true;
+        return;
     }
 
     try {
@@ -124,8 +124,6 @@ Hostapd::SetProperty(std::string_view propertyName, std::string_view propertyVal
     } catch (HostapdException& e) {
         throw HostapdException(std::format("Failed to reload hostapd configuration after '{}' property change: {}", propertyName, e.what()));
     }
-
-    return true;
 }
 
 void
@@ -195,9 +193,10 @@ Hostapd::Terminate()
 void
 Hostapd::SetSsid(std::string_view ssid, EnforceConfigurationChange enforceConfigurationChange)
 {
-    const bool ssidWasSet = SetProperty(ProtocolHostapd::PropertyNameSsid, ssid, enforceConfigurationChange);
-    if (!ssidWasSet) {
-        throw HostapdException(std::format("Failed to set hostapd 'ssid' property to '{}'", ssid));
+    try {
+        SetProperty(ProtocolHostapd::PropertyNameSsid, ssid, enforceConfigurationChange);
+    } catch (const HostapdException& e) {
+        throw HostapdException(std::format("Failed to set hostapd ssid to '{}' ({})", ssid, e.what()));
     }
 }
 
@@ -220,9 +219,10 @@ Hostapd::SetWpaProtocols(std::vector<WpaProtocol> protocols, EnforceConfiguratio
     protocolsToSet &= WpaProtocolMask;
 
     const auto protocolsValue = std::format("{}", protocolsToSet);
-    const bool protocolsWereSet = SetProperty(ProtocolHostapd::PropertyNameWpaProtocol, protocolsValue, enforceConfigurationChange);
-    if (!protocolsWereSet) {
-        throw HostapdException(std::format("Failed to set hostapd 'wpa' property to '{}'", protocolsValue));
+    try {
+        SetProperty(ProtocolHostapd::PropertyNameWpaProtocol, protocolsValue, enforceConfigurationChange);
+    } catch (HostapdException& e) {
+        throw HostapdException(std::format("Failed to set hostapd 'wpa' property to '{}' ({})", protocolsValue, e.what()));
     }
 }
 
@@ -248,8 +248,9 @@ Hostapd::SetKeyManagement(std::vector<WpaKeyManagement> keyManagements, EnforceC
         keyManagementPropertyValue = keyManagementPropertyValueBuilder.str();
     }
 
-    const auto keyManagementWasSet = SetProperty(ProtocolHostapd::PropertyNameWpaKeyManagement, keyManagementPropertyValue, enforceConfigurationChange);
-    if (!keyManagementWasSet) {
-        throw HostapdException(std::format("Failed to set hostapd 'wpa_key_mgmt' property to '{}'", keyManagementPropertyValue));
+    try {
+        SetProperty(ProtocolHostapd::PropertyNameWpaKeyManagement, keyManagementPropertyValue, enforceConfigurationChange);
+    } catch (const HostapdException& e) {
+        throw HostapdException(std::format("Failed to set hostapd 'wpa_key_mgmt' property to '{}' ({})", keyManagementPropertyValue, e.what()));
     }
 }
diff --git a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
index e0e821c5..b815fe0d 100644
--- a/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/Hostapd.hxx
@@ -86,10 +86,8 @@ struct Hostapd :
      * @param propertyName The name of the property to set.
      * @param propertyValue The value of the property to set.
      * @param enforceConfigurationChange When the enforce the configuration change. A value of 'Now' will trigger a configuration reload.
-     * @return true The property was set successfully.
-     * @return false The property was not set successfully.
      */
-    bool
+    void
     SetProperty(std::string_view propertyName, std::string_view propertyValue, EnforceConfigurationChange enforceConfigurationChange = EnforceConfigurationChange::Now) override;
 
     /**
diff --git a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
index 7a503c2a..f20974f2 100644
--- a/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
+++ b/src/linux/wpa-controller/include/Wpa/IHostapd.hxx
@@ -110,10 +110,8 @@ struct IHostapd
      * @param propertyName The name of the property to set.
      * @param propertyValue The value of the property to set.
      * @param enforceConfigurationChange When the enforce the configuration change. A value of 'Now' will trigger a configuration reload.
-     * @return true The property was set successfully.
-     * @return false The property was not set successfully.
      */
-    virtual bool
+    virtual void
     SetProperty(std::string_view propertyName, std::string_view propertyValue, EnforceConfigurationChange enforceConfigurationChange) = 0;
 
     /**
diff --git a/tests/unit/linux/wpa-controller/TestHostapd.cxx b/tests/unit/linux/wpa-controller/TestHostapd.cxx
index 0916d698..646aa762 100644
--- a/tests/unit/linux/wpa-controller/TestHostapd.cxx
+++ b/tests/unit/linux/wpa-controller/TestHostapd.cxx
@@ -118,12 +118,12 @@ TEST_CASE("Send command: GetStatus() (root)", "[wpa][hostapd][client][remote]")
         const auto ieee80211nInitial = hostapd.GetStatus().Ieee80211n;
 
         auto ieee80211nValueExpected = static_cast<bool>(ieee80211nInitial);
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211N, GetPropertyEnablementValue(ieee80211nValueExpected)));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211N, GetPropertyEnablementValue(ieee80211nValueExpected)));
         auto ieee80211nValueUpdated = hostapd.GetStatus().Ieee80211n;
         REQUIRE(ieee80211nValueUpdated == ieee80211nValueExpected);
 
         ieee80211nValueExpected = static_cast<bool>(ieee80211nValueUpdated);
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211N, GetPropertyEnablementValue(ieee80211nValueExpected)));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211N, GetPropertyEnablementValue(ieee80211nValueExpected)));
         ieee80211nValueUpdated = hostapd.GetStatus().Ieee80211n;
         REQUIRE(ieee80211nValueUpdated == ieee80211nValueExpected);
     }
@@ -135,12 +135,12 @@ TEST_CASE("Send command: GetStatus() (root)", "[wpa][hostapd][client][remote]")
         const auto ieee80211acInitial = hostapd.GetStatus().Ieee80211ac;
 
         auto ieee80211acValueExpected = static_cast<bool>(ieee80211acInitial);
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AC, GetPropertyEnablementValue(ieee80211acValueExpected)));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AC, GetPropertyEnablementValue(ieee80211acValueExpected)));
         auto ieee80211acValueUpdated = hostapd.GetStatus().Ieee80211ac;
         REQUIRE(ieee80211acValueUpdated == ieee80211acValueExpected);
 
         ieee80211acValueExpected = static_cast<bool>(ieee80211acValueUpdated);
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AC, GetPropertyEnablementValue(ieee80211acValueExpected)));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AC, GetPropertyEnablementValue(ieee80211acValueExpected)));
         ieee80211acValueUpdated = hostapd.GetStatus().Ieee80211ac;
         REQUIRE(ieee80211acValueUpdated == ieee80211acValueExpected);
     }
@@ -152,12 +152,12 @@ TEST_CASE("Send command: GetStatus() (root)", "[wpa][hostapd][client][remote]")
         const auto ieee80211axInitial = hostapd.GetStatus().Ieee80211ax;
 
         auto ieee80211axValueExpected = static_cast<bool>(ieee80211axInitial);
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AX, GetPropertyEnablementValue(ieee80211axValueExpected)));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AX, GetPropertyEnablementValue(ieee80211axValueExpected)));
         auto ieee80211axValueUpdated = hostapd.GetStatus().Ieee80211ax;
         REQUIRE(ieee80211axValueUpdated == ieee80211axValueExpected);
 
         ieee80211axValueExpected = static_cast<bool>(ieee80211axValueUpdated);
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AX, GetPropertyEnablementValue(ieee80211axValueExpected)));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameIeee80211AX, GetPropertyEnablementValue(ieee80211axValueExpected)));
         ieee80211axValueUpdated = hostapd.GetStatus().Ieee80211ax;
         REQUIRE(ieee80211axValueUpdated == ieee80211axValueExpected);
     }
@@ -217,34 +217,34 @@ TEST_CASE("Send SetProperty() command (root)", "[wpa][hostapd][client][remote]")
 
     SECTION("SetProperty() returns true for valid property")
     {
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Now));
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Defer));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Now));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Defer));
     }
 
     SECTION("SetProperty() allows setting a property to the same value")
     {
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Now));
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Now));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Now));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Now));
 
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Defer));
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Defer));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Defer));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Defer));
     }
 
     SECTION("SetProperty allows setting a property to a different value")
     {
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Now));
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue2G, EnforceConfigurationChange::Now));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Now));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue2G, EnforceConfigurationChange::Now));
 
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue5G, EnforceConfigurationChange::Defer));
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue6G, EnforceConfigurationChange::Defer));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue5G, EnforceConfigurationChange::Defer));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue6G, EnforceConfigurationChange::Defer));
     }
 
     SECTION("SetProperty allows interleaving enforcement of configuration changes")
     {
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Now));
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue2G, EnforceConfigurationChange::Defer));
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue5G, EnforceConfigurationChange::Now));
-        REQUIRE(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue6G, EnforceConfigurationChange::Defer));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValueAuto, EnforceConfigurationChange::Now));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue2G, EnforceConfigurationChange::Defer));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue5G, EnforceConfigurationChange::Now));
+        REQUIRE_NOTHROW(hostapd.SetProperty(ProtocolHostapd::PropertyNameSetBand, ProtocolHostapd::PropertySetBandValue6G, EnforceConfigurationChange::Defer));
     }
 
     // TODO: validate that the property was actually set. Need to find a property whose value is retrievable.