net: NetworkMonitorImplOsx: various minor cleanups

Change-Id: Ie56bb6897018d93cb3f431c98cf8df8c97706e51
Refs: #3817
diff --git a/src/net/detail/network-monitor-impl-osx.cpp b/src/net/detail/network-monitor-impl-osx.cpp
index 34d53ed..ebf90b0 100644
--- a/src/net/detail/network-monitor-impl-osx.cpp
+++ b/src/net/detail/network-monitor-impl-osx.cpp
@@ -50,12 +50,11 @@
  *   POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include "ndn-cxx-config.hpp"
-
 #include "network-monitor-impl-osx.hpp"
+
+#include "../network-address.hpp"
 #include "../../name.hpp"
 #include "../../util/logger.hpp"
-#include "../network-address.hpp"
 
 #include <ifaddrs.h>       // for getifaddrs()
 #include <arpa/inet.h>     // for inet_ntop()
@@ -109,8 +108,7 @@
   , m_scStore(SCDynamicStoreCreate(nullptr, CFSTR("net.named-data.ndn-cxx.NetworkMonitor"),
                                    &NetworkMonitorImplOsx::onConfigChanged, &m_context))
   , m_loopSource(SCDynamicStoreCreateRunLoopSource(nullptr, m_scStore.get(), 0))
-  , m_nullUdpSocket(io, boost::asio::ip::udp::v4())
-
+  , m_ioctlSocket(io, boost::asio::ip::udp::v4())
 {
   scheduleCfLoop();
 
@@ -132,17 +130,18 @@
   // State:/Network/Interface/.*/Link
   // State:/Network/Interface/.*/IPv4
   // State:/Network/Interface/.*/IPv6
-  // State:/Network/Global/DNS
-  // State:/Network/Global/IPv4
+  // State:/Network/Interface/.*/AirPort (not used)
+  //
+  // https://developer.apple.com/library/content/documentation/Networking/Conceptual/SystemConfigFrameworks/SC_UnderstandSchema/SC_UnderstandSchema.html
   //
   auto patterns = CFArrayCreateMutable(nullptr, 0, &kCFTypeArrayCallBacks);
   CFArrayAppendValue(patterns, CFSTR("State:/Network/Interface/.*/Link"));
   CFArrayAppendValue(patterns, CFSTR("State:/Network/Interface/.*/IPv4"));
   CFArrayAppendValue(patterns, CFSTR("State:/Network/Interface/.*/IPv6"));
-  // CFArrayAppendValue(patterns, CFSTR("State:/Network/Global/DNS"));
-  // CFArrayAppendValue(patterns, CFSTR("State:/Network/Global/IPv4"));
 
-  SCDynamicStoreSetNotificationKeys(m_scStore.get(), nullptr, patterns);
+  if (!SCDynamicStoreSetNotificationKeys(m_scStore.get(), nullptr, patterns)) {
+    BOOST_THROW_EXCEPTION(Error("SCDynamicStoreSetNotificationKeys failed"));
+  }
 
   io.post([this] { enumerateInterfaces(); });
 }
@@ -230,7 +229,14 @@
 {
   CFReleaser<CFDictionaryRef> dict =
     (CFDictionaryRef)SCDynamicStoreCopyValue(m_scStore.get(), CFSTR("State:/Network/Interface"));
+  if (dict.get() == nullptr) {
+    return {};
+  }
+
   CFArrayRef interfaces = (CFArrayRef)CFDictionaryGetValue(dict.get(), CFSTR("Interfaces"));
+  if (interfaces == nullptr) {
+    return {};
+  }
 
   std::set<std::string> ifNames;
   size_t count = CFArrayGetCount(interfaces);
@@ -246,7 +252,7 @@
 {
   shared_ptr<NetworkInterface> interface = makeNetworkInterface();
   interface->setName(ifName);
-  interface->setState(getInterfaceState(interface->getName()));
+  interface->setState(getInterfaceState(*interface));
   updateInterfaceInfo(*interface, ifaList);
 
   if (interface->getType() == InterfaceType::UNKNOWN) {
@@ -260,13 +266,15 @@
 }
 
 InterfaceState
-NetworkMonitorImplOsx::getInterfaceState(const std::string& ifName) const
+NetworkMonitorImplOsx::getInterfaceState(const NetworkInterface& netif) const
 {
   CFReleaser<CFStringRef> linkName =
-    CFStringCreateWithCString(nullptr, ("State:/Network/Interface/" + ifName + "/Link").data(),
+    CFStringCreateWithCString(kCFAllocatorDefault,
+                              ("State:/Network/Interface/" + netif.getName() + "/Link").data(),
                               kCFStringEncodingASCII);
 
-  CFReleaser<CFDictionaryRef> dict = (CFDictionaryRef)SCDynamicStoreCopyValue(m_scStore.get(), linkName.get());
+  CFReleaser<CFDictionaryRef> dict =
+    (CFDictionaryRef)SCDynamicStoreCopyValue(m_scStore.get(), linkName.get());
   if (dict.get() == nullptr) {
     return InterfaceState::UNKNOWN;
   }
@@ -280,16 +288,16 @@
 }
 
 size_t
-NetworkMonitorImplOsx::getInterfaceMtu(const std::string& ifName)
+NetworkMonitorImplOsx::getInterfaceMtu(const NetworkInterface& netif)
 {
   ifreq ifr{};
-  std::strncpy(ifr.ifr_name, ifName.data(), sizeof(ifr.ifr_name) - 1);
+  std::strncpy(ifr.ifr_name, netif.getName().data(), sizeof(ifr.ifr_name) - 1);
 
-  if (::ioctl(m_nullUdpSocket.native_handle(), SIOCGIFMTU, &ifr) == 0) {
+  if (::ioctl(m_ioctlSocket.native_handle(), SIOCGIFMTU, &ifr) == 0) {
     return static_cast<size_t>(ifr.ifr_mtu);
   }
 
-  NDN_LOG_WARN("failed to get MTU of " << ifName << ": " << std::strerror(errno));
+  NDN_LOG_WARN("failed to get MTU of " << netif.getName() << ": " << std::strerror(errno));
   return ethernet::MAX_DATA_LEN;
 }
 
@@ -310,12 +318,15 @@
 void
 NetworkMonitorImplOsx::updateInterfaceInfo(NetworkInterface& netif, const IfAddrs& ifaList)
 {
+  BOOST_ASSERT(!netif.getName().empty());
+
+  netif.setMtu(getInterfaceMtu(netif));
+
   for (ifaddrs* ifa = ifaList.get(); ifa != nullptr; ifa = ifa->ifa_next) {
     if (ifa->ifa_name != netif.getName())
       continue;
 
     netif.setFlags(ifa->ifa_flags);
-    netif.setMtu(getInterfaceMtu(netif.getName()));
 
     if (ifa->ifa_addr == nullptr)
       continue;
@@ -384,6 +395,9 @@
       }
     }
 
+    if (addrFamily == AddressFamily::UNSPECIFIED)
+      continue;
+
     AddressScope scope = AddressScope::GLOBAL;
     if (ipAddr.is_loopback()) {
       scope = AddressScope::HOST;
@@ -410,8 +424,7 @@
 
   size_t count = CFArrayGetCount(changedKeys);
   for (size_t i = 0; i != count; ++i) {
-    std::string keyName = convertToStdString((CFStringRef)CFArrayGetValueAtIndex(changedKeys, i));
-    Name key(keyName);
+    Name key(convertToStdString((CFStringRef)CFArrayGetValueAtIndex(changedKeys, i)));
     std::string ifName = key.at(-2).toUri();
 
     auto ifIt = m_interfaces.find(ifName);
@@ -420,17 +433,17 @@
       return;
     }
 
-    NetworkInterface& netif = *ifIt->second;
-
     auto removeInterface = [&] {
       NDN_LOG_DEBUG("removing interface " << ifName);
-      shared_ptr<NetworkInterface> removedInterface = ifIt->second;
+      shared_ptr<NetworkInterface> removedNetif = ifIt->second;
       m_interfaces.erase(ifIt);
-      this->emitSignal(onInterfaceRemoved, removedInterface);
+      this->emitSignal(onInterfaceRemoved, removedNetif);
     };
 
-    if (key.at(-1).toUri() == "Link") {
-      auto newState = getInterfaceState(ifName);
+    NetworkInterface& netif = *ifIt->second;
+    std::string changedItem = key.at(-1).toUri();
+    if (changedItem == "Link") {
+      auto newState = getInterfaceState(netif);
       if (newState == InterfaceState::UNKNOWN) {
         // check if it is really unknown or interface removed
         if (getInterfaceNames().count(ifName) == 0) {
@@ -441,20 +454,18 @@
       NDN_LOG_TRACE(ifName << " status changed from " << netif.getState() << " to " << newState);
       netif.setState(newState);
     }
-
-    if (key.at(-1).toUri() == "IPv4" || key.at(-1).toUri() == "IPv6") {
-      shared_ptr<NetworkInterface> updatedInterface = makeNetworkInterface();
-      updatedInterface->setName(ifName);
-      updateInterfaceInfo(*updatedInterface, ifaList);
-      if (updatedInterface->getType() == InterfaceType::UNKNOWN) {
+    else if (changedItem == "IPv4" || changedItem == "IPv6") {
+      auto updatedNetif = makeNetworkInterface();
+      updatedNetif->setName(ifName);
+      updateInterfaceInfo(*updatedNetif, ifaList);
+      if (updatedNetif->getType() == InterfaceType::UNKNOWN) {
         NDN_LOG_DEBUG(ifName << " type changed to unknown");
         removeInterface();
         return;
       }
 
-      const auto& newAddrs = updatedInterface->getNetworkAddresses();
+      const auto& newAddrs = updatedNetif->getNetworkAddresses();
       const auto& oldAddrs = netif.getNetworkAddresses();
-
       std::set<NetworkAddress> added;
       std::set<NetworkAddress> removed;
       std::set_difference(newAddrs.begin(), newAddrs.end(),
diff --git a/src/net/detail/network-monitor-impl-osx.hpp b/src/net/detail/network-monitor-impl-osx.hpp
index 7339153..809c638 100644
--- a/src/net/detail/network-monitor-impl-osx.hpp
+++ b/src/net/detail/network-monitor-impl-osx.hpp
@@ -86,10 +86,10 @@
   addNewInterface(const std::string& ifName, const IfAddrs& ifaList);
 
   InterfaceState
-  getInterfaceState(const std::string& ifName) const;
+  getInterfaceState(const NetworkInterface& netif) const;
 
   size_t
-  getInterfaceMtu(const std::string& ifName);
+  getInterfaceMtu(const NetworkInterface& netif);
 
   void
   updateInterfaceInfo(NetworkInterface& netif, const IfAddrs& ifaList);
@@ -110,7 +110,7 @@
   util::CFReleaser<SCDynamicStoreRef> m_scStore;
   util::CFReleaser<CFRunLoopSourceRef> m_loopSource;
 
-  boost::asio::ip::udp::socket m_nullUdpSocket;
+  boost::asio::ip::udp::socket m_ioctlSocket;
 };
 
 } // namespace net