Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update IHostapd interface to replace bool returns with exceptions and direct return values #214

Merged
merged 10 commits into from
Mar 12, 2024
20 changes: 9 additions & 11 deletions src/linux/wifi/core/AccessPointControllerLinux.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
Expand All @@ -192,10 +189,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;
}

Expand Down Expand Up @@ -234,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();
Expand Down Expand Up @@ -269,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();
Expand Down
96 changes: 55 additions & 41 deletions src/linux/wpa-controller/Hostapd.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Hostapd::GetInterface()
return m_interface;
}

bool
void
Hostapd::Ping()
{
static constexpr WpaCommand PingCommand(ProtocolHostapd::CommandPayloadPing);
Expand All @@ -43,10 +43,13 @@ 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
void
Hostapd::Reload()
{
static constexpr WpaCommand ReloadCommand(ProtocolHostapd::CommandPayloadReload);
Expand All @@ -56,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
Expand All @@ -72,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);
Expand All @@ -84,14 +90,15 @@ 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
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));
Expand All @@ -103,24 +110,23 @@ 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());
throw HostapdException(std::format("Failed to set hostapd property '{}' to '{}'", propertyName, propertyValue));
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 '{}' (invalid response)", propertyName, propertyValue));
}

if (enforceConfigurationChange == EnforceConfigurationChange::Defer) {
LOGD << std::format("Skipping enforcement of '{}' configuration change (requested)", propertyName);
return true;
return;
}

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;
}

bool
void
Hostapd::Enable()
{
static constexpr WpaCommand EnableCommand(ProtocolHostapd::CommandPayloadEnable);
Expand All @@ -131,18 +137,20 @@ Hostapd::Enable()
}

if (response->IsOk()) {
return true;
return;
}

// The response will indicate failure if the interface is already enabled.
// Check if this is the case by validating the interface status.
// 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
void
Hostapd::Disable()
{
static constexpr WpaCommand DisableCommand(ProtocolHostapd::CommandPayloadDisable);
Expand All @@ -153,18 +161,20 @@ Hostapd::Disable()
}

if (response->IsOk()) {
return true;
return;
}

// The response will indicate failure if the interface is already disabled.
// Check if this is the case by validating the interface status.
// 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
void
Hostapd::Terminate()
{
static constexpr WpaCommand TerminateCommand(ProtocolHostapd::CommandPayloadTerminate);
Expand All @@ -174,17 +184,23 @@ 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
void
Hostapd::SetSsid(std::string_view ssid, EnforceConfigurationChange enforceConfigurationChange)
{
const bool ssidWasSet = SetProperty(ProtocolHostapd::PropertyNameSsid, ssid, enforceConfigurationChange);
return ssidWasSet;
try {
SetProperty(ProtocolHostapd::PropertyNameSsid, ssid, enforceConfigurationChange);
} catch (const HostapdException& e) {
throw HostapdException(std::format("Failed to set hostapd ssid to '{}' ({})", ssid, e.what()));
}
}

bool
void
Hostapd::SetWpaProtocols(std::vector<WpaProtocol> protocols, EnforceConfigurationChange enforceConfigurationChange)
{
if (std::empty(protocols)) {
Expand All @@ -203,15 +219,14 @@ 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()));
}

return true;
}

bool
void
Hostapd::SetKeyManagement(std::vector<WpaKeyManagement> keyManagements, EnforceConfigurationChange enforceConfigurationChange)
{
if (std::empty(keyManagements)) {
Expand All @@ -233,10 +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()));
}

return true;
}
46 changes: 12 additions & 34 deletions src/linux/wpa-controller/include/Wpa/Hostapd.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -27,44 +27,32 @@ struct Hostapd :

/**
* @brief Enables the interface for use.
*
* @return true
* @return false
*/
bool
void
Enable() override;

/**
* @brief Disables the interface for use.
*
* @return true
* @return false
*/
bool
void
Disable() override;

/**
* @brief Terminates the process hosting the daemon.
*
* @return true
* @return false
*/
bool
void
Terminate() override;

/**
* @brief Checks connectivity to the hostapd daemon.
*/
bool
void
Ping() override;

/**
* @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;

/**
Expand All @@ -87,56 +75,46 @@ 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.
*
* @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;

/**
* @brief Set the ssid for the interface.
*
* @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;

/**
* @brief Set the WPA protocol(s) for the interface.
*
* @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;

/**
* @brief Set the Key Management object
*
* @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:
Expand Down
Loading