Merge pull request #160 from microsoft/fixnits

Some some possible issues identified by clang-tidy

src/common/tools/cli/NetRemoteCli.cxx

@@ -25,7 +25,7 @@ NetRemoteCli::NetRemoteCli(std::shared_ptr<NetRemoteCliData> cliData, std::share
 
 /* static */
 std::shared_ptr<NetRemoteCli>
-NetRemoteCli::Create(std::shared_ptr<NetRemoteCliData> cliData, std::shared_ptr<NetRemoteCliHandler> cliHandler)
+NetRemoteCli::Create(std::shared_ptr<NetRemoteCliData> cliData, const std::shared_ptr<NetRemoteCliHandler>& cliHandler)
 {
     auto instance = std::make_shared<notstd::enable_make_protected<NetRemoteCli>>(std::move(cliData), cliHandler);
     cliHandler->SetParent(instance->weak_from_this());

src/common/tools/cli/include/microsoft/net/remote/NetRemoteCli.hxx

@@ -28,7 +28,7 @@ struct NetRemoteCli :
      * @return std::shared_ptr<NetRemoteClie>
      */
     static std::shared_ptr<NetRemoteCli>
-    Create(std::shared_ptr<NetRemoteCliData> cliData, std::shared_ptr<NetRemoteCliHandler> cliHandler);
+    Create(std::shared_ptr<NetRemoteCliData> cliData, const std::shared_ptr<NetRemoteCliHandler>& cliHandler);
 
     /**
      * @brief Get a reference to the parser. The parser will be configured with all common command-line interface

src/linux/libnl-helpers/Netlink80211Interface.cxx

@@ -29,6 +29,8 @@
 
 using namespace Microsoft::Net::Netlink::Nl80211;
 
+// NOLINTBEGIN(concurrency-mt-unsafe)
+
 Nl80211Interface::Nl80211Interface(std::string_view name, nl80211_iftype type, uint32_t index, uint32_t wiphyIndex) noexcept :
     Name(name),
     Type(type),
@@ -67,7 +69,7 @@ Nl80211Interface::Parse(struct nl_msg *nl80211Message) noexcept
     std::array<struct nlattr *, NL80211_ATTR_MAX + 1> newInterfaceMessageAttributes{};
     int ret = nla_parse(std::data(newInterfaceMessageAttributes), std::size(newInterfaceMessageAttributes), genlmsg_attrdata(genl80211MessageHeader, 0), genlmsg_attrlen(genl80211MessageHeader, 0), nullptr);
     if (ret < 0) {
-        LOG_ERROR << std::format("Failed to parse netlink message attributes with error {} ({})", ret, strerror(-ret));
+        LOGE << std::format("Failed to parse netlink message attributes with error {} ({})", ret, strerror(-ret));
         return std::nullopt;
     }
 
@@ -177,3 +179,5 @@ Nl80211Interface::IsAccessPoint() const noexcept
 {
     return (Type == nl80211_iftype::NL80211_IFTYPE_AP);
 }
+
+// NOLINTEND(concurrency-mt-unsafe)

src/linux/libnl-helpers/Netlink80211ProtocolState.cxx

@@ -23,25 +23,25 @@ Nl80211ProtocolState::Nl80211ProtocolState()
     auto netlinkSocket{ NetlinkSocket::Allocate() };
 
     // Connect the socket to the generic netlink family.
-    int ret = genl_connect(netlinkSocket);
+    const int ret = genl_connect(netlinkSocket);
     if (ret < 0) {
         const auto err = errno;
-        LOG_ERROR << std::format("Failed to connect netlink socket for nl control with error {} ({})", err, strerror(err));
+        LOGE << std::format("Failed to connect netlink socket for nl control with error {} ({})", err, strerror(err)); // NOLINT(concurrency-mt-unsafe)
         throw err;
     }
 
     // Look up the nl80211 driver id.
     DriverId = genl_ctrl_resolve(netlinkSocket, NL80211_GENL_NAME);
     if (DriverId < 0) {
-        LOG_ERROR << std::format("Failed to resolve nl80211 netlink id with error {} ({})", DriverId, nl_geterror(DriverId));
+        LOGE << std::format("Failed to resolve nl80211 netlink id with error {} ({})", DriverId, nl_geterror(DriverId));
         throw DriverId;
     }
 
     // Lookup the ids for the nl80211 multicast groups.
     for (const auto& [multicastGroup, multicastGroupName] : Nl80211MulticastGroupNames) {
         int multicastGroupId = genl_ctrl_resolve_grp(netlinkSocket, NL80211_GENL_NAME, std::data(multicastGroupName));
         if (multicastGroupId < 0) {
-            LOG_ERROR << std::format("Failed to resolve nl80211 {} multicast group id with error {} ({})", multicastGroupName, multicastGroupId, nl_geterror(multicastGroupId));
+            LOGE << std::format("Failed to resolve nl80211 {} multicast group id with error {} ({})", multicastGroupName, multicastGroupId, nl_geterror(multicastGroupId));
             throw multicastGroupId;
         }
         MulticastGroupId[multicastGroup] = multicastGroupId;

src/linux/libnl-helpers/Netlink80211Wiphy.cxx

@@ -76,7 +76,7 @@ HandleNl80211GetWiphyResponse(struct nl_msg *nl80211Message, void *context) noex
 
 /* static */
 std::optional<Nl80211Wiphy>
-Nl80211Wiphy::FromId(std::function<void(Microsoft::Net::Netlink::NetlinkMessage &)> addWiphyIdentifier)
+Nl80211Wiphy::FromId(const std::function<void(Microsoft::Net::Netlink::NetlinkMessage &)> &addWiphyIdentifier)
 {
     // Allocate a new netlink socket.
     auto nl80211SocketOpt{ CreateNl80211Socket() };
@@ -178,7 +178,7 @@ Nl80211Wiphy::Parse(struct nl_msg *nl80211Message) noexcept
     std::array<struct nlattr *, NL80211_ATTR_MAX + 1> wiphyAttributes{};
     int ret = nla_parse(std::data(wiphyAttributes), std::size(wiphyAttributes), genlmsg_attrdata(genl80211MessageHeader, 0), genlmsg_attrlen(genl80211MessageHeader, 0), nullptr);
     if (ret < 0) {
-        LOG_ERROR << std::format("Failed to parse netlink message attributes with error {} ({})", ret, strerror(-ret));
+        LOGE << std::format("Failed to parse netlink message attributes with error {} ({})", ret, strerror(-ret)); // NOLINT(concurrency-mt-unsafe)
         return std::nullopt;
     }
 
@@ -189,14 +189,14 @@ Nl80211Wiphy::Parse(struct nl_msg *nl80211Message) noexcept
     }
 
     auto wiphyIndex = static_cast<uint32_t>(nla_get_u32(wiphyAttributes[NL80211_ATTR_WIPHY]));
-    auto wiphyName = static_cast<const char *>(nla_data(wiphyAttributes[NL80211_ATTR_WIPHY_NAME]));
+    const auto *wiphyName = static_cast<const char *>(nla_data(wiphyAttributes[NL80211_ATTR_WIPHY_NAME]));
 
     // Process bands.
-    auto wiphyBands = wiphyAttributes[NL80211_ATTR_WIPHY_BANDS];
+    auto *wiphyBands = wiphyAttributes[NL80211_ATTR_WIPHY_BANDS];
     std::unordered_map<nl80211_band, Nl80211WiphyBand> wiphyBandMap{};
     if (wiphyBands != nullptr) {
-        int remainingBands;
-        struct nlattr *wiphyBand;
+        int remainingBands = 0;
+        struct nlattr *wiphyBand = nullptr;
 
         nla_for_each_nested(wiphyBand, wiphyBands, remainingBands)
         {
@@ -214,13 +214,13 @@ Nl80211Wiphy::Parse(struct nl_msg *nl80211Message) noexcept
 
     // Process AKM suites.
     // Note: NL80211_ATTR_AKM_SUITES describes the AKMs supported by the PHY (wiphy) and is not specific to an interface.
-    uint32_t *wiphyAkmSuites;
+    uint32_t *wiphyAkmSuites = nullptr;
     std::size_t wiphyNumAkmSuites = 0;
     std::vector<uint32_t> akmSuites{};
     if (wiphyAttributes[NL80211_ATTR_AKM_SUITES] != nullptr) {
         wiphyAkmSuites = static_cast<uint32_t *>(nla_data(wiphyAttributes[NL80211_ATTR_AKM_SUITES]));
         wiphyNumAkmSuites = static_cast<std::size_t>(nla_len(wiphyAttributes[NL80211_ATTR_AKM_SUITES])) / sizeof(*wiphyAkmSuites);
-        akmSuites = std::vector<uint32_t>(wiphyAkmSuites, wiphyAkmSuites + wiphyNumAkmSuites);
+        akmSuites = std::vector<uint32_t>(wiphyAkmSuites, wiphyAkmSuites + wiphyNumAkmSuites); // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic)
     } else {
         // Per nl80211 documentation, if this attribute is not present, userspace should assume all AKMs are supported.
         akmSuites = Microsoft::Net::Wifi::AllIeee80211Akms;
@@ -230,17 +230,17 @@ Nl80211Wiphy::Parse(struct nl_msg *nl80211Message) noexcept
     std::size_t wiphyNumCipherSuites = 0;
     std::vector<uint32_t> cipherSuites{};
     if (wiphyAttributes[NL80211_ATTR_CIPHER_SUITES] != nullptr) {
-        uint32_t *wiphyCipherSuites;
+        uint32_t *wiphyCipherSuites = nullptr;
         wiphyNumCipherSuites = static_cast<std::size_t>(nla_len(wiphyAttributes[NL80211_ATTR_CIPHER_SUITES])) / sizeof(*wiphyCipherSuites);
         wiphyCipherSuites = static_cast<uint32_t *>(nla_data(wiphyAttributes[NL80211_ATTR_CIPHER_SUITES]));
-        cipherSuites = std::vector<uint32_t>(wiphyCipherSuites, wiphyCipherSuites + wiphyNumCipherSuites);
+        cipherSuites = std::vector<uint32_t>(wiphyCipherSuites, wiphyCipherSuites + wiphyNumCipherSuites); // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic)
     }
 
     // Process supported interface types.
     std::vector<nl80211_iftype> supportedInterfaceTypes{};
     if (wiphyAttributes[NL80211_ATTR_SUPPORTED_IFTYPES] != nullptr) {
-        int remainingSupportedInterfaceTypes;
-        struct nlattr *supportedInterfaceType;
+        int remainingSupportedInterfaceTypes = 0;
+        struct nlattr *supportedInterfaceType = nullptr;
         nla_for_each_nested(supportedInterfaceType, wiphyAttributes[NL80211_ATTR_SUPPORTED_IFTYPES], remainingSupportedInterfaceTypes)
         {
             auto interfaceType = static_cast<nl80211_iftype>(supportedInterfaceType->nla_type);

src/linux/libnl-helpers/include/microsoft/net/netlink/nl80211/Netlink80211Wiphy.hxx

@@ -92,7 +92,7 @@ private:
      * @return std::optional<Nl80211Wiphy>
      */
     static std::optional<Nl80211Wiphy>
-    FromId(std::function<void(Microsoft::Net::Netlink::NetlinkMessage&)> addWiphyIdentifier);
+    FromId(const std::function<void(Microsoft::Net::Netlink::NetlinkMessage&)>& addWiphyIdentifier);
 };
 
 } // namespace Microsoft::Net::Netlink::Nl80211

src/linux/tools/apmonitor/Main.cxx

@@ -1,5 +1,9 @@
 
-#include <csignal> // NOLINT(misc-include-cleaner)
+// NOLINTBEGIN(misc-include-cleaner, concurrency-mt-unsafe)
+// clang-tidy can't seem to figure out that sig/SIG* stuff is indirectly included by <csignal>. While we could include
+// the actual headers directly, this goes against the std library's convention of including the top-level header.
+
+#include <csignal>
 #include <format>
 #include <memory>
 #include <utility>
@@ -33,27 +37,30 @@ main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
         LOGI << std::format("{} -> {}", accessPointChanged->GetInterfaceName(), magic_enum::enum_name(presence));
     });
 
-    LOG_INFO << "starting access point discovery agent";
+    LOGI << "starting access point discovery agent";
     accessPointDiscoveryAgent->Start();
 
     // Mask SIGTERM and SIGINT so they can be explicitly waited on from the main thread.
-    int signal;
+    int signal = 0;
     sigset_t mask;
     sigemptyset(&mask);
     sigaddset(&mask, SIGTERM);
     sigaddset(&mask, SIGINT);
 
     if (sigprocmask(SIG_BLOCK, &mask, nullptr) < 0) {
-        LOG_ERROR << "failed to block terminate signals";
+        LOGE << "failed to block terminate signals";
         return -1;
     }
 
     // Wait for the process to be signaled to exit.
-    while (sigwait(&mask, &signal) != 0)
+    while (sigwait(&mask, &signal) != 0) {
         ;
+    }
 
     // Received interrupt or terminate signal, so shut down.
     accessPointDiscoveryAgent->Stop();
 
     return 0;
 }
+
+// NOLINTEND(misc-include-cleaner, concurrency-mt-unsafe)