Skip to content

Commit

Permalink
Move updates.
Browse files Browse the repository at this point in the history
  • Loading branch information
abeltrano committed Feb 9, 2024
1 parent 988d76b commit 7876615
Show file tree
Hide file tree
Showing 18 changed files with 52 additions and 39 deletions.
2 changes: 1 addition & 1 deletion src/common/server/NetRemoteServerConfiguration.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ ParseCliAppOptions(bool throwOnParseError, Args&&... args)

/* static */
NetRemoteServerConfiguration
NetRemoteServerConfiguration::FromCommandLineArguments(int argc, char* argv[], bool throwOnParseError)
NetRemoteServerConfiguration::FromCommandLineArguments(int argc, char* argv[], bool throwOnParseError) // NOLINT(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays)
{
return details::ParseCliAppOptions(throwOnParseError, argc, argv);
}
Expand Down
2 changes: 1 addition & 1 deletion src/common/shared/logging/LogUtils.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ logging::GetLogName(std::string_view componentName)

std::stringstream ss;

ss << std::put_time(std::localtime(&t_c), "%Y%m%d")
ss << std::put_time(std::localtime(&t_c), "%Y%m%d") // NOLINT(concurrency-mt-unsafe)
<< "-LogNetRemote-"
<< componentName
<< ".txt";
Expand Down
12 changes: 6 additions & 6 deletions src/common/tools/cli/NetRemoteCli.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
using namespace Microsoft::Net::Remote;

NetRemoteCli::NetRemoteCli(std::shared_ptr<NetRemoteCliData> cliData, std::shared_ptr<NetRemoteCliHandler> cliHandler) :
m_cliData{ cliData },
m_cliHandler{ cliHandler },
m_cliData{ std::move(cliData) },
m_cliHandler{ std::move(cliHandler) },
m_cliApp{ CreateParser() }
{
}
Expand Down Expand Up @@ -46,7 +46,7 @@ NetRemoteCli::GetData() const
}

int
NetRemoteCli::Parse(int argc, char* argv[]) noexcept
NetRemoteCli::Parse(int argc, char* argv[]) noexcept // NOLINT(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays)
{
try {
m_cliApp->parse(argc, argv);
Expand All @@ -66,7 +66,7 @@ NetRemoteCli::CreateParser() noexcept

app->require_subcommand();

auto optionServer = app->add_option_function<std::string>("-s,--server", [this](const std::string& serverAddress) {
auto* optionServer = app->add_option_function<std::string>("-s,--server", [this](const std::string& serverAddress) {
OnServerAddressChanged(serverAddress);
});
optionServer->description("The address of the netremote server to connect to, with format '<hostname>[:port]");
Expand All @@ -93,7 +93,7 @@ NetRemoteCli::AddSubcommandWifi(CLI::App* parent)
CLI::App*
NetRemoteCli::AddSubcommandWifiEnumerateAccessPoints(CLI::App* parent)
{
auto cliAppWifiEnumerateAccessPoints = parent->add_subcommand("enumerate-access-points", "Enumerate available Wi-Fi access points");
auto* cliAppWifiEnumerateAccessPoints = parent->add_subcommand("enumerate-access-points", "Enumerate available Wi-Fi access points");
cliAppWifiEnumerateAccessPoints->alias("enumaps");
cliAppWifiEnumerateAccessPoints->callback([this] {
OnWifiEnumerateAccessPoints();
Expand All @@ -109,7 +109,7 @@ NetRemoteCli::OnServerAddressChanged(const std::string& serverAddressArg)

// Append the default port if not specified in command-line argument.
auto serverAddress = serverAddressArg;
if (serverAddress.find(':') == serverAddress.npos) {
if (serverAddress.find(':') == std::string::npos) {
serverAddress += std::format("{}{}", NetRemoteProtocol::PortSeparator, NetRemoteProtocol::PortDefault);
}

Expand Down
4 changes: 2 additions & 2 deletions src/common/tools/cli/NetRemoteCliHandler.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ NetRemoteCliHandler::NetRemoteCliHandler(std::unique_ptr<INetRemoteCliHandlerOpe
void
NetRemoteCliHandler::SetParent(std::weak_ptr<NetRemoteCli> parent)
{
m_parent = parent;
m_parent = std::move(parent);
}

void
NetRemoteCliHandler::SetConnection(std::shared_ptr<NetRemoteServerConnection> connection)
{
m_connection = connection;
m_connection = std::move(connection);
m_operations = m_operationsFactory->Create(m_connection);
}

Expand Down
2 changes: 1 addition & 1 deletion src/common/tools/cli/NetRemoteCliHandlerOperations.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ NetRemoteAccessPointCapabilitiesToString(const Microsoft::Net::Wifi::Dot11Access
void
NetRemoteCliHandlerOperations::WifiEnumerateAccessPoints()
{
WifiEnumerateAccessPointsRequest request{};
const WifiEnumerateAccessPointsRequest request{};
WifiEnumerateAccessPointsResult result{};
grpc::ClientContext clientContext{};

Expand Down
4 changes: 2 additions & 2 deletions src/common/wifi/apmanager/AccessPointDiscoveryAgent.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ AccessPointDiscoveryAgent::GetInstance() noexcept
void
AccessPointDiscoveryAgent::RegisterDiscoveryEventCallback(AccessPointPresenceEventCallback onDevicePresenceChanged)
{
std::unique_lock<std::shared_mutex> onDevicePresenceChangedLock{ m_onDevicePresenceChangedGate };
const std::unique_lock<std::shared_mutex> onDevicePresenceChangedLock{ m_onDevicePresenceChangedGate };
m_onDevicePresenceChanged = std::move(onDevicePresenceChanged);
}

void
AccessPointDiscoveryAgent::DevicePresenceChanged(AccessPointPresenceEvent presence, std::shared_ptr<IAccessPoint> accessPoint) const noexcept
{
std::shared_lock<std::shared_mutex> onDevicePresenceChangedLock{ m_onDevicePresenceChangedGate };
const std::shared_lock<std::shared_mutex> onDevicePresenceChangedLock{ m_onDevicePresenceChangedGate };
if (m_onDevicePresenceChanged) {
LOGI << std::format("Access Point Discovery Event: Interface {} {}", accessPoint->GetInterfaceName(), magic_enum::enum_name(presence));
m_onDevicePresenceChanged(presence, std::move(accessPoint));
Expand Down
4 changes: 2 additions & 2 deletions src/common/wifi/apmanager/AccessPointManager.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ AccessPointManager::AddDiscoveryAgent(std::shared_ptr<AccessPointDiscoveryAgent>

// Add the agent.
{
std::unique_lock<std::shared_mutex> discoveryAgentLock{ m_discoveryAgentsGate };
const std::unique_lock<std::shared_mutex> discoveryAgentLock{ m_discoveryAgentsGate };
m_discoveryAgents.push_back(std::move(discoveryAgent));
}

Expand All @@ -134,7 +134,7 @@ AccessPointManager::AddDiscoveryAgent(std::shared_ptr<AccessPointDiscoveryAgent>
// If the operation completed, get the results and add those access points.
if (waitResult == std::future_status::ready) {
auto existingAccessPoints = existingAccessPointsProbe.get();
for (const auto& existingAccessPoint : existingAccessPoints) {
for (auto& existingAccessPoint : existingAccessPoints) {
AddAccessPoint(std::move(existingAccessPoint));
}
} else {
Expand Down
10 changes: 3 additions & 7 deletions src/linux/libnl-helpers/Netlink80211Interface.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@

using namespace Microsoft::Net::Netlink::Nl80211;

using Microsoft::Net::Netlink::NetlinkMessage;
using Microsoft::Net::Netlink::NetlinkSocket;

Nl80211Interface::Nl80211Interface(std::string_view name, nl80211_iftype type, uint32_t index, uint32_t wiphyIndex) noexcept :
Name(name),
Type(type),
Expand Down Expand Up @@ -75,7 +72,7 @@ Nl80211Interface::Parse(struct nl_msg *nl80211Message) noexcept
}

// Tease out parameters to populate the Nl80211Interface instance.
auto *interfaceName = static_cast<const char *>(nla_data(newInterfaceMessageAttributes[NL80211_ATTR_IFNAME]));
const auto *interfaceName = static_cast<const char *>(nla_data(newInterfaceMessageAttributes[NL80211_ATTR_IFNAME]));
auto interfaceType = static_cast<nl80211_iftype>(nla_get_u32(newInterfaceMessageAttributes[NL80211_ATTR_IFTYPE]));
auto interfaceIndex = static_cast<uint32_t>(nla_get_u32(newInterfaceMessageAttributes[NL80211_ATTR_IFINDEX]));
auto wiphyIndex = static_cast<uint32_t>(nla_get_u32(newInterfaceMessageAttributes[NL80211_ATTR_WIPHY]));
Expand Down Expand Up @@ -108,10 +105,9 @@ HandleNl80211InterfaceDumpResponse(struct nl_msg *nl80211Message, void *context)
if (!nl80211Interface.has_value()) {
LOGW << "Failed to parse nl80211 interface dump response";
return NL_SKIP;
} else {
LOGD << std::format("Parsed nl80211 interface dump response: {}", nl80211Interface->ToString());
}


LOGD << std::format("Parsed nl80211 interface dump response: {}", nl80211Interface->ToString());
nl80211Interfaces.push_back(std::move(nl80211Interface.value()));

return NL_OK;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#ifndef NETLINK_82011_HXX
#define NETLINK_82011_HXX

#include <cstdint>
#include <optional>
#include <string_view>
#include <unordered_map>
Expand Down Expand Up @@ -54,7 +55,7 @@ Nl80211InterfaceTypeToString(nl80211_iftype type) noexcept;
* @return std::string_view
*/
std::string_view
Nl80211CipherSuiteToString(uint32_t cipherType) noexcept;
Nl80211CipherSuiteToString(uint32_t cipherSuite) noexcept;

/**
* @brief Create a netlink socket for use with Nl80211.
Expand Down
2 changes: 2 additions & 0 deletions src/linux/wpa-controller/ProtocolHostapd.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ using namespace Wpa;
HostapdInterfaceState
Wpa::HostapdInterfaceStateFromString(std::string_view state) noexcept
{
// NOLINTBEGIN(readability-else-after-return)
// Implementation uses starts_with() instead of equals() to accommodate
// unparsed payloads from command responses.
if (state.starts_with(ProtocolHostapd::ResponsePayloadStatusUninitialized)) {
Expand All @@ -27,6 +28,7 @@ Wpa::HostapdInterfaceStateFromString(std::string_view state) noexcept
} else if (state.starts_with(ProtocolHostapd::ResponsePayloadStatusNoIr)) {
return HostapdInterfaceState::NoIr;
}
// NOLINTEND(readability-else-after-return)

return HostapdInterfaceState::Unknown;
}
Expand Down
1 change: 0 additions & 1 deletion src/linux/wpa-controller/WpaCommandSet.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using namespace Wpa;

WpaCommandSet::WpaCommandSet(std::string_view propertyName, std::string_view propertyValue) :
WpaCommand(),
PropertyPayload(std::format("{} {} {}", ProtocolWpa::CommandPayloadSet, propertyName, propertyValue)),
PropertyName(propertyName),
PropertyValue(propertyValue)
Expand Down
4 changes: 2 additions & 2 deletions src/linux/wpa-controller/WpaController.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ WpaController::GetCommandControlSocket()
{
// Check if a socket connection is already established.
{
std::shared_lock lockShared{ m_controlSocketCommandGate };
const std::shared_lock lockShared{ m_controlSocketCommandGate };
if (m_controlSocketCommand != nullptr) {
return m_controlSocketCommand;
}
}

// Check if socket was updated between releasing the shared lock and acquiring the exclusive lock.
std::scoped_lock lockExclusive{ m_controlSocketCommandGate };
const std::scoped_lock lockExclusive{ m_controlSocketCommandGate };
if (m_controlSocketCommand != nullptr) {
return m_controlSocketCommand;
}
Expand Down
17 changes: 11 additions & 6 deletions tests/unit/TestNetRemoteServer.cxx
Original file line number Diff line number Diff line change
@@ -1,27 +1,32 @@

#include <chrono>
#include <cstdlib>
#include <memory>
#include <optional>
#include <ranges>

#include <catch2/catch_test_macros.hpp>
#include <catch2/generators/catch_generators_range.hpp>
#include <grpc/impl/codegen/connectivity_state.h>
#include <grpcpp/create_channel.h>
#include <grpcpp/security/credentials.h>
#include <microsoft/net/remote/NetRemoteServer.hxx>
#include <microsoft/net/remote/NetRemoteServerConfiguration.hxx>
#include <microsoft/net/remote/protocol/NetRemoteService.grpc.pb.h>
#include <microsoft/net/wifi/test/AccessPointTest.hxx>
#include <microsoft/net/wifi/AccessPointManager.hxx>

#include "TestNetRemoteCommon.hxx"

using Microsoft::Net::Remote::Test::EstablishClientConnections;
using Microsoft::Net::Remote::Test::RemoteServiceAddressHttp;
using Microsoft::Net::Remote::Test::RemoteServiceConnectionTimeout;
using Microsoft::Net::Wifi::Test::AccessPointFactoryTest;

TEST_CASE("Create a NetRemoteServer instance", "[basic][rpc][remote]")
{
using namespace Microsoft::Net::Remote;
using namespace Microsoft::Net::Wifi;

NetRemoteServerConfiguration Configuration{
const NetRemoteServerConfiguration Configuration{
.ServerAddress = RemoteServiceAddressHttp,
.AccessPointManager = AccessPointManager::Create(),
};
Expand Down Expand Up @@ -69,7 +74,7 @@ TEST_CASE("NetRemoteServer can be reached", "[basic][rpc][remote]")
using namespace Microsoft::Net::Remote::Service;
using namespace Microsoft::Net::Wifi;

NetRemoteServerConfiguration Configuration{
const NetRemoteServerConfiguration Configuration{
.ServerAddress = RemoteServiceAddressHttp,
.AccessPointManager = AccessPointManager::Create(),
};
Expand All @@ -92,7 +97,7 @@ TEST_CASE("NetRemoteServer shuts down correctly", "[basic][rpc][remote]")
using namespace Microsoft::Net::Remote::Service;
using namespace Microsoft::Net::Wifi;

NetRemoteServerConfiguration Configuration{
const NetRemoteServerConfiguration Configuration{
.ServerAddress = RemoteServiceAddressHttp,
.AccessPointManager = AccessPointManager::Create(),
};
Expand Down Expand Up @@ -147,7 +152,7 @@ TEST_CASE("NetRemoteServer can be cycled through run/stop states", "[basic][rpc]
using namespace Microsoft::Net::Remote::Service;
using namespace Microsoft::Net::Wifi;

NetRemoteServerConfiguration Configuration{
const NetRemoteServerConfiguration Configuration{
.ServerAddress = RemoteServiceAddressHttp,
.AccessPointManager = AccessPointManager::Create(),
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@

#include <catch2/catch_test_macros.hpp>
#include <microsoft/net/netlink/nl80211/Netlink80211Interface.hxx>
#include <netlink/attr.h>

TEST_CASE("Nl80211Interface instance creation", "[linux][libnl-helpers]")
{
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/linux/wpa-controller/Main.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#include <plog/Appenders/ColorConsoleAppender.h>
#include <plog/Formatters/MessageOnlyFormatter.h>
#include <plog/Init.h>
#include <plog/Log.h>
#include <plog/Severity.h>

int main(int argc, char* argv[])
{
Expand Down
6 changes: 5 additions & 1 deletion tests/unit/linux/wpa-controller/TestHostapd.cxx
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@

#include <chrono>
#include <optional>
#include <string>
#include <string_view>
#include <thread>

#include <Wpa/Hostapd.hxx>
#include <Wpa/IHostapd.hxx>
#include <Wpa/ProtocolHostapd.hxx>
#include <catch2/catch_test_macros.hpp>

Expand All @@ -13,7 +17,7 @@ namespace Wpa::Test
std::string_view
GetPropertyEnablementValue(int valueToSet)
{
return !!valueToSet ? ProtocolHostapd::PropertyEnabled : ProtocolHostapd::PropertyDisabled;
return (valueToSet != 0) ? ProtocolHostapd::PropertyEnabled : ProtocolHostapd::PropertyDisabled;
}
} // namespace Wpa::Test

Expand Down
13 changes: 9 additions & 4 deletions tests/unit/linux/wpa-controller/TestWpaController.cxx
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@

#include <filesystem>
#include <memory>
#include <optional>
#include <string_view>

#include <Wpa/WpaCommand.hxx>
#include <Wpa/WpaResponse.hxx>
#include <Wpa/WpaController.hxx>
#include <Wpa/WpaCore.hxx>
#include <catch2/catch_test_macros.hpp>
#include <magic_enum.hpp>

#include "detail/WpaDaemonManager.hxx"
#include <Wpa/WpaController.hxx>

namespace TestDetail
{
Expand All @@ -25,7 +30,7 @@ TEST_CASE("Send/receive WpaController request/response (root)", "[wpa][hostapd][
for (const auto& wpaType : TestDetail::WpaTypesSupported) {
WpaController wpaController(WpaDaemonManager::InterfaceNameDefault, wpaType);

WpaCommand wpaCommand("PING");
const WpaCommand wpaCommand("PING");
std::shared_ptr<WpaResponse> wpaResponse;
REQUIRE_NOTHROW(wpaResponse = wpaController.SendCommand(wpaCommand));
REQUIRE(wpaResponse != nullptr);
Expand Down Expand Up @@ -54,7 +59,7 @@ TEST_CASE("Create a WpaController", "[wpa][client][local]")
SECTION("Create for each daemon type reflects correct type")
{
for (const auto& wpaType : magic_enum::enum_values<WpaType>()) {
WpaController wpaController(WpaDaemonManager::InterfaceNameDefault, wpaType);
const WpaController wpaController(WpaDaemonManager::InterfaceNameDefault, wpaType);
REQUIRE(wpaController.Type() == wpaType);
}
}
Expand Down Expand Up @@ -87,7 +92,7 @@ TEST_CASE("Create a WpaController", "[wpa][client][local]")
{
for (const auto& wpaType : magic_enum::enum_values<WpaType>()) {
// Create with control socket path encoded as std::filesystem::path.
WpaController wpaController(WpaDaemonManager::InterfaceNameDefault, wpaType, ControlSocketPath);
const WpaController wpaController(WpaDaemonManager::InterfaceNameDefault, wpaType, ControlSocketPath);
REQUIRE(wpaController.ControlSocketPath() == ControlSocketPath);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ WifiVirtualDeviceManager::CreateInterfacesForDriver(uint32_t numberOfInterfaces,

// Load the specified kernel module, which should create the simulated wireless interface(s).
const auto driverLoadCommand = std::format("sudo modprobe {} {}", driverName, driverArguments);
int ret = std::system(driverLoadCommand.c_str()); // NOLINT(cert-env33-c, cert-err34-c)
int ret = std::system(driverLoadCommand.c_str()); // NOLINT(cert-env33-c, cert-err34-c, concurrency-mt-unsafe)
if (ret == -1) {
ret = WEXITSTATUS(ret);
std::cerr << std::format("Failed to load {} driver, load command '{}' failed (ret={})\n", driverName, driverLoadCommand, ret);
Expand Down

0 comments on commit 7876615

Please sign in to comment.