From f798c91551fef4a7e55689cd84c9099e2b74695d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= <maze@google.com>
Date: Wed, 24 Jan 2024 17:21:38 -0800
Subject: [PATCH] getIfaceNameFromMap - better type safety
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Test: atest libnetworkstats_test NetworkStackIntegrationTests:android.net.NetworkStatsIntegrationTest FrameworksNetTests:android.net.connectivity.com.android.server.net.BpfInterfaceMapUpdaterTest
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I4d60180764f2325025e9903457f8eb56cb9cfa48
---
 .../native/libs/libnetworkstats/BpfNetworkStats.cpp  | 12 ++++++------
 .../libs/libnetworkstats/BpfNetworkStatsTest.cpp     |  2 +-
 .../include/netdbpf/BpfNetworkStats.h                |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp
index 2af6928836..ac5b90eace 100644
--- a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp
+++ b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp
@@ -75,12 +75,12 @@ int bpfGetIfaceStatsInternal(const char* iface, StatsValue* stats,
             [iface, stats, &ifaceNameMap, &unknownIfaceBytesTotal](
                     const uint32_t& key,
                     const BpfMapRO<uint32_t, StatsValue>& ifaceStatsMap) -> Result<void> {
-        char ifname[IFNAMSIZ];
+        IfaceValue ifname;
         if (getIfaceNameFromMap(ifaceNameMap, ifaceStatsMap, key, ifname, key,
                                 &unknownIfaceBytesTotal)) {
             return Result<void>();
         }
-        if (!iface || !strcmp(iface, ifname)) {
+        if (!iface || !strcmp(iface, ifname.name)) {
             Result<StatsValue> statsEntry = ifaceStatsMap.readValue(key);
             if (!statsEntry.ok()) {
                 return statsEntry.error();
@@ -113,9 +113,9 @@ int bpfGetIfIndexStats(int ifindex, StatsValue* stats) {
 }
 
 stats_line populateStatsEntry(const StatsKey& statsKey, const StatsValue& statsEntry,
-                              const char* ifname) {
+                              const IfaceValue& ifname) {
     stats_line newLine;
-    strlcpy(newLine.iface, ifname, sizeof(newLine.iface));
+    strlcpy(newLine.iface, ifname.name, sizeof(newLine.iface));
     newLine.uid = (int32_t)statsKey.uid;
     newLine.set = (int32_t)statsKey.counterSet;
     newLine.tag = (int32_t)statsKey.tag;
@@ -134,7 +134,7 @@ int parseBpfNetworkStatsDetailInternal(std::vector<stats_line>& lines,
             [&lines, &unknownIfaceBytesTotal, &ifaceMap](
                     const StatsKey& key,
                     const BpfMapRO<StatsKey, StatsValue>& statsMap) -> Result<void> {
-        char ifname[IFNAMSIZ];
+        IfaceValue ifname;
         if (getIfaceNameFromMap(ifaceMap, statsMap, key.ifaceIndex, ifname, key,
                                 &unknownIfaceBytesTotal)) {
             return Result<void>();
@@ -224,7 +224,7 @@ int parseBpfNetworkStatsDevInternal(std::vector<stats_line>& lines,
     const auto processDetailIfaceStats = [&lines, &unknownIfaceBytesTotal, &ifaceMap, &statsMap](
                                              const uint32_t& key, const StatsValue& value,
                                              const BpfMapRO<uint32_t, StatsValue>&) {
-        char ifname[IFNAMSIZ];
+        IfaceValue ifname;
         if (getIfaceNameFromMap(ifaceMap, statsMap, key, ifname, key, &unknownIfaceBytesTotal)) {
             return Result<void>();
         }
diff --git a/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp b/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp
index bcc455069c..2c01904235 100644
--- a/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp
+++ b/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp
@@ -352,7 +352,7 @@ TEST_F(BpfNetworkStatsHelperTest, TestUnknownIfaceError) {
             .counterSet = TEST_COUNTERSET0,
             .ifaceIndex = ifaceIndex,
     };
-    char ifname[IFNAMSIZ];
+    IfaceValue ifname;
     int64_t unknownIfaceBytesTotal = 0;
     ASSERT_EQ(-ENODEV, getIfaceNameFromMap(mFakeIfaceIndexNameMap, mFakeStatsMap, ifaceIndex,
                                            ifname, curKey, &unknownIfaceBytesTotal));
diff --git a/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h b/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h
index 8058d0580a..d2efff5eb1 100644
--- a/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h
+++ b/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h
@@ -75,14 +75,14 @@ int cleanStatsMapInternal(const base::unique_fd& cookieTagMap, const base::uniqu
 template <class Key>
 int getIfaceNameFromMap(const BpfMapRO<uint32_t, IfaceValue>& ifaceMap,
                         const BpfMapRO<Key, StatsValue>& statsMap,
-                        uint32_t ifaceIndex, char* ifname,
+                        uint32_t ifaceIndex, IfaceValue& ifname,
                         const Key& curKey, int64_t* unknownIfaceBytesTotal) {
     auto iface = ifaceMap.readValue(ifaceIndex);
     if (!iface.ok()) {
         maybeLogUnknownIface(ifaceIndex, statsMap, curKey, unknownIfaceBytesTotal);
         return -ENODEV;
     }
-    strlcpy(ifname, iface.value().name, sizeof(IfaceValue));
+    ifname = iface.value();
     return 0;
 }
 
-- 
GitLab