From 2d1839289eca016893b898dc713cbf1a1df92fc1 Mon Sep 17 00:00:00 2001
From: Nathan Scott <nathans@redhat.com>
Date: Mon, 1 Mar 2021 11:55:15 +1100
Subject: [PATCH] Fix integer sizing issues in the NetworkIO Meter

On Linux kernels the size of the values exported for network
device bytes and packets has used a 64 bit integer for quite
some time (2.6+ IIRC).  Make the procfs value extraction use
correct types and change internal types used to rate convert
these counters (within the NetworkIO Meter) 64 bit integers,
where appropriate.
---
 NetworkIOMeter.c        | 64 ++++++++++++++++++++++-------------------
 NetworkIOMeter.h        |  7 +++++
 darwin/Platform.c       | 10 ++-----
 darwin/Platform.h       |  6 ++--
 dragonflybsd/Platform.c | 10 ++-----
 dragonflybsd/Platform.h |  6 ++--
 freebsd/Platform.c      | 24 +++++-----------
 freebsd/Platform.h      |  6 ++--
 linux/Platform.c        | 31 ++++++++------------
 linux/Platform.h        |  6 ++--
 openbsd/Platform.c      | 10 ++-----
 openbsd/Platform.h      |  6 ++--
 solaris/Platform.c      | 10 ++-----
 solaris/Platform.h      |  6 ++--
 unsupported/Platform.c  | 10 ++-----
 unsupported/Platform.h  |  6 ++--
 16 files changed, 85 insertions(+), 133 deletions(-)

diff --git a/NetworkIOMeter.c b/NetworkIOMeter.c
index b78ac89a..cd1d46b0 100644
--- a/NetworkIOMeter.c
+++ b/NetworkIOMeter.c
@@ -19,10 +19,10 @@ static const int NetworkIOMeter_attributes[] = {
 
 static bool hasData = false;
 
-static unsigned long int cached_rxb_diff = 0;
-static unsigned long int cached_rxp_diff = 0;
-static unsigned long int cached_txb_diff = 0;
-static unsigned long int cached_txp_diff = 0;
+static unsigned long int cached_rxb_diff;
+static unsigned long int cached_rxp_diff;
+static unsigned long int cached_txb_diff;
+static unsigned long int cached_txp_diff;
 
 static void NetworkIOMeter_updateValues(Meter* this, char* buffer, size_t len) {
    static unsigned long long int cached_last_update = 0;
@@ -34,50 +34,56 @@ static void NetworkIOMeter_updateValues(Meter* this, char* buffer, size_t len) {
 
    /* update only every 500ms */
    if (passedTimeInMs > 500) {
-      static unsigned long int cached_rxb_total = 0;
-      static unsigned long int cached_rxp_total = 0;
-      static unsigned long int cached_txb_total = 0;
-      static unsigned long int cached_txp_total = 0;
+      static unsigned long long int cached_rxb_total;
+      static unsigned long long int cached_rxp_total;
+      static unsigned long long int cached_txb_total;
+      static unsigned long long int cached_txp_total;
+      unsigned long long diff;
 
       cached_last_update = timeInMilliSeconds;
 
-      unsigned long int bytesReceived, packetsReceived, bytesTransmitted, packetsTransmitted;
-
-      hasData = Platform_getNetworkIO(&bytesReceived, &packetsReceived, &bytesTransmitted, &packetsTransmitted);
+      NetworkIOData data;
+      hasData = Platform_getNetworkIO(&data);
       if (!hasData) {
          xSnprintf(buffer, len, "no data");
          return;
       }
 
-      if (bytesReceived > cached_rxb_total) {
-         cached_rxb_diff = (bytesReceived - cached_rxb_total) / 1024; /* Meter_humanUnit() expects unit in kilo */
-         cached_rxb_diff = 1000.0 * cached_rxb_diff / passedTimeInMs; /* convert to per second */
+      if (data.bytesReceived > cached_rxb_total) {
+         diff = data.bytesReceived - cached_rxb_total;
+         diff /= ONE_K; /* Meter_humanUnit() expects unit in kilo */
+         diff = (1000 * diff) / passedTimeInMs; /* convert to per second */
+         cached_rxb_diff = (unsigned long)diff;
       } else {
-         cached_rxb_diff = 0;
+         cached_rxb_diff = 0UL;
       }
-      cached_rxb_total = bytesReceived;
+      cached_rxb_total = data.bytesReceived;
 
-      if (packetsReceived > cached_rxp_total) {
-         cached_rxp_diff = packetsReceived - cached_rxp_total;
+      if (data.packetsReceived > cached_rxp_total) {
+         diff = data.packetsReceived - cached_rxp_total;
+         cached_rxp_diff = (unsigned long)diff;
       } else {
-         cached_rxp_diff = 0;
+         cached_rxp_diff = 0UL;
       }
-      cached_rxp_total = packetsReceived;
+      cached_rxp_total = data.packetsReceived;
 
-      if (bytesTransmitted > cached_txb_total) {
-         cached_txb_diff = (bytesTransmitted - cached_txb_total) / 1024; /* Meter_humanUnit() expects unit in kilo */
-         cached_txb_diff = 1000.0 * cached_txb_diff / passedTimeInMs; /* convert to per second */
+      if (data.bytesTransmitted > cached_txb_total) {
+         diff = data.bytesTransmitted - cached_txb_total;
+         diff /= ONE_K; /* Meter_humanUnit() expects unit in kilo */
+         diff = (1000 * diff) / passedTimeInMs; /* convert to per second */
+         cached_txb_diff = (unsigned long)diff;
       } else {
-         cached_txb_diff = 0;
+         cached_txb_diff = 0UL;
       }
-      cached_txb_total = bytesTransmitted;
+      cached_txb_total = data.bytesTransmitted;
 
-      if (packetsTransmitted > cached_txp_total) {
-         cached_txp_diff = packetsTransmitted - cached_txp_total;
+      if (data.packetsTransmitted > cached_txp_total) {
+         diff = data.packetsTransmitted - cached_txp_total;
+         cached_txp_diff = (unsigned long)diff;
       } else {
-         cached_txp_diff = 0;
+         cached_txp_diff = 0UL;
       }
-      cached_txp_total = packetsTransmitted;
+      cached_txp_total = data.packetsTransmitted;
    }
 
    this->values[0] = cached_rxb_diff;
diff --git a/NetworkIOMeter.h b/NetworkIOMeter.h
index 311b5e64..5b29fa84 100644
--- a/NetworkIOMeter.h
+++ b/NetworkIOMeter.h
@@ -3,6 +3,13 @@
 
 #include "Meter.h"
 
+typedef struct NetworkIOData_ {
+   unsigned long long int bytesReceived;
+   unsigned long long int packetsReceived;
+   unsigned long long int bytesTransmitted;
+   unsigned long long int packetsTransmitted;
+} NetworkIOData;
+
 extern const MeterClass NetworkIOMeter_class;
 
 #endif /* HEADER_NetworkIOMeter */
diff --git a/darwin/Platform.c b/darwin/Platform.c
index d73cdcc7..1d2b0f4f 100644
--- a/darwin/Platform.c
+++ b/darwin/Platform.c
@@ -334,15 +334,9 @@ bool Platform_getDiskIO(DiskIOData* data) {
    return false;
 }
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted) {
+bool Platform_getNetworkIO(NetworkIOData* data) {
    // TODO
-   *bytesReceived = 0;
-   *packetsReceived = 0;
-   *bytesTransmitted = 0;
-   *packetsTransmitted = 0;
+   (void)data;
    return false;
 }
 
diff --git a/darwin/Platform.h b/darwin/Platform.h
index 623063b2..2a97270a 100644
--- a/darwin/Platform.h
+++ b/darwin/Platform.h
@@ -16,6 +16,7 @@ in the source distribution for its full text.
 #include "CPUMeter.h"
 #include "DarwinProcess.h"
 #include "DiskIOMeter.h"
+#include "NetworkIOMeter.h"
 #include "ProcessLocksScreen.h"
 #include "SignalsPanel.h"
 
@@ -62,10 +63,7 @@ FileLocks_ProcessData* Platform_getProcessLocks(pid_t pid);
 
 bool Platform_getDiskIO(DiskIOData* data);
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted);
+bool Platform_getNetworkIO(NetworkIOData* data);
 
 void Platform_getBattery(double *percent, ACPresence *isOnAC);
 
diff --git a/dragonflybsd/Platform.c b/dragonflybsd/Platform.c
index 08132716..3646e33a 100644
--- a/dragonflybsd/Platform.c
+++ b/dragonflybsd/Platform.c
@@ -233,15 +233,9 @@ bool Platform_getDiskIO(DiskIOData* data) {
    return false;
 }
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted) {
+bool Platform_getNetworkIO(NetworkIOData* data) {
    // TODO
-   *bytesReceived = 0;
-   *packetsReceived = 0;
-   *bytesTransmitted = 0;
-   *packetsTransmitted = 0;
+   (void)data;
    return false;
 }
 
diff --git a/dragonflybsd/Platform.h b/dragonflybsd/Platform.h
index 3c5d9cb2..1ae13938 100644
--- a/dragonflybsd/Platform.h
+++ b/dragonflybsd/Platform.h
@@ -14,6 +14,7 @@ in the source distribution for its full text.
 #include "Action.h"
 #include "BatteryMeter.h"
 #include "DiskIOMeter.h"
+#include "NetworkIOMeter.h"
 #include "ProcessLocksScreen.h"
 #include "SignalsPanel.h"
 
@@ -52,10 +53,7 @@ FileLocks_ProcessData* Platform_getProcessLocks(pid_t pid);
 
 bool Platform_getDiskIO(DiskIOData* data);
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted);
+bool Platform_getNetworkIO(NetworkIOData* data);
 
 void Platform_getBattery(double* percent, ACPresence* isOnAC);
 
diff --git a/freebsd/Platform.c b/freebsd/Platform.c
index 9f8c051f..a4fe2354 100644
--- a/freebsd/Platform.c
+++ b/freebsd/Platform.c
@@ -315,24 +315,18 @@ bool Platform_getDiskIO(DiskIOData* data) {
    return true;
 }
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted) {
-   int r;
-
+bool Platform_getNetworkIO(NetworkIOData* data) {
    // get number of interfaces
    int count;
    size_t countLen = sizeof(count);
    const int countMib[] = { CTL_NET, PF_LINK, NETLINK_GENERIC, IFMIB_SYSTEM, IFMIB_IFCOUNT };
 
+   int r;
    r = sysctl(countMib, ARRAYSIZE(countMib), &count, &countLen, NULL, 0);
    if (r < 0)
       return false;
 
-
-   unsigned long int bytesReceivedSum = 0, packetsReceivedSum = 0, bytesTransmittedSum = 0, packetsTransmittedSum = 0;
-
+   memset(data, 0, sizeof(NetworkIOData));
    for (int i = 1; i <= count; i++) {
       struct ifmibdata ifmd;
       size_t ifmdLen = sizeof(ifmd);
@@ -346,16 +340,12 @@ bool Platform_getNetworkIO(unsigned long int* bytesReceived,
       if (ifmd.ifmd_flags & IFF_LOOPBACK)
          continue;
 
-      bytesReceivedSum += ifmd.ifmd_data.ifi_ibytes;
-      packetsReceivedSum += ifmd.ifmd_data.ifi_ipackets;
-      bytesTransmittedSum += ifmd.ifmd_data.ifi_obytes;
-      packetsTransmittedSum += ifmd.ifmd_data.ifi_opackets;
+      data->bytesReceived += ifmd.ifmd_data.ifi_ibytes;
+      data->packetsReceived += ifmd.ifmd_data.ifi_ipackets;
+      data->bytesTransmitted += ifmd.ifmd_data.ifi_obytes;
+      data->packetsTransmitted += ifmd.ifmd_data.ifi_opackets;
    }
 
-   *bytesReceived = bytesReceivedSum;
-   *packetsReceived = packetsReceivedSum;
-   *bytesTransmitted = bytesTransmittedSum;
-   *packetsTransmitted = packetsTransmittedSum;
    return true;
 }
 
diff --git a/freebsd/Platform.h b/freebsd/Platform.h
index 36895b81..8f25d83a 100644
--- a/freebsd/Platform.h
+++ b/freebsd/Platform.h
@@ -14,6 +14,7 @@ in the source distribution for its full text.
 #include "BatteryMeter.h"
 #include "DiskIOMeter.h"
 #include "Meter.h"
+#include "NetworkIOMeter.h"
 #include "Process.h"
 #include "ProcessLocksScreen.h"
 #include "SignalsPanel.h"
@@ -57,10 +58,7 @@ FileLocks_ProcessData* Platform_getProcessLocks(pid_t pid);
 
 bool Platform_getDiskIO(DiskIOData* data);
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted);
+bool Platform_getNetworkIO(NetworkIOData* data);
 
 void Platform_getBattery(double* percent, ACPresence* isOnAC);
 
diff --git a/linux/Platform.c b/linux/Platform.c
index da2ae606..00b8e309 100644
--- a/linux/Platform.c
+++ b/linux/Platform.c
@@ -534,42 +534,35 @@ bool Platform_getDiskIO(DiskIOData* data) {
    return true;
 }
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted) {
+bool Platform_getNetworkIO(NetworkIOData* data) {
    FILE* fd = fopen(PROCDIR "/net/dev", "r");
    if (!fd)
       return false;
 
-   unsigned long int bytesReceivedSum = 0, packetsReceivedSum = 0, bytesTransmittedSum = 0, packetsTransmittedSum = 0;
+   memset(data, 0, sizeof(NetworkIOData));
    char lineBuffer[512];
    while (fgets(lineBuffer, sizeof(lineBuffer), fd)) {
       char interfaceName[32];
-      unsigned long int bytesReceivedParsed, packetsReceivedParsed, bytesTransmittedParsed, packetsTransmittedParsed;
-      if (sscanf(lineBuffer, "%31s %lu %lu %*u %*u %*u %*u %*u %*u %lu %lu",
+      unsigned long long int bytesReceived, packetsReceived, bytesTransmitted, packetsTransmitted;
+      if (sscanf(lineBuffer, "%31s %llu %llu %*u %*u %*u %*u %*u %*u %llu %llu",
                              interfaceName,
-                             &bytesReceivedParsed,
-                             &packetsReceivedParsed,
-                             &bytesTransmittedParsed,
-                             &packetsTransmittedParsed) != 5)
+                             &bytesReceived,
+                             &packetsReceived,
+                             &bytesTransmitted,
+                             &packetsTransmitted) != 5)
          continue;
 
       if (String_eq(interfaceName, "lo:"))
          continue;
 
-      bytesReceivedSum += bytesReceivedParsed;
-      packetsReceivedSum += packetsReceivedParsed;
-      bytesTransmittedSum += bytesTransmittedParsed;
-      packetsTransmittedSum += packetsTransmittedParsed;
+      data->bytesReceived += bytesReceived;
+      data->packetsReceived += packetsReceived;
+      data->bytesTransmitted += bytesTransmitted;
+      data->packetsTransmitted += packetsTransmitted;
    }
 
    fclose(fd);
 
-   *bytesReceived = bytesReceivedSum;
-   *packetsReceived = packetsReceivedSum;
-   *bytesTransmitted = bytesTransmittedSum;
-   *packetsTransmitted = packetsTransmittedSum;
    return true;
 }
 
diff --git a/linux/Platform.h b/linux/Platform.h
index d87ef55a..f932c583 100644
--- a/linux/Platform.h
+++ b/linux/Platform.h
@@ -15,6 +15,7 @@ in the source distribution for its full text.
 #include "BatteryMeter.h"
 #include "DiskIOMeter.h"
 #include "Meter.h"
+#include "NetworkIOMeter.h"
 #include "Process.h"
 #include "ProcessLocksScreen.h"
 #include "SignalsPanel.h"
@@ -67,10 +68,7 @@ void Platform_getPressureStall(const char *file, bool some, double* ten, double*
 
 bool Platform_getDiskIO(DiskIOData* data);
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted);
+bool Platform_getNetworkIO(NetworkIOData* data);
 
 void Platform_getBattery(double *percent, ACPresence *isOnAC);
 
diff --git a/openbsd/Platform.c b/openbsd/Platform.c
index a50aa345..606869a5 100644
--- a/openbsd/Platform.c
+++ b/openbsd/Platform.c
@@ -283,15 +283,9 @@ bool Platform_getDiskIO(DiskIOData* data) {
    return false;
 }
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted) {
+bool Platform_getNetworkIO(NetworkIOData* data) {
    // TODO
-   *bytesReceived = 0;
-   *packetsReceived = 0;
-   *bytesTransmitted = 0;
-   *packetsTransmitted = 0;
+   (void)data;
    return false;
 }
 
diff --git a/openbsd/Platform.h b/openbsd/Platform.h
index e7a59662..66b804de 100644
--- a/openbsd/Platform.h
+++ b/openbsd/Platform.h
@@ -15,6 +15,7 @@ in the source distribution for its full text.
 #include "BatteryMeter.h"
 #include "DiskIOMeter.h"
 #include "Meter.h"
+#include "NetworkIOMeter.h"
 #include "Process.h"
 #include "ProcessLocksScreen.h"
 #include "SignalsPanel.h"
@@ -55,10 +56,7 @@ FileLocks_ProcessData* Platform_getProcessLocks(pid_t pid);
 
 bool Platform_getDiskIO(DiskIOData* data);
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted);
+bool Platform_getNetworkIO(NetworkIOData* data);
 
 void Platform_getBattery(double* percent, ACPresence* isOnAC);
 
diff --git a/solaris/Platform.c b/solaris/Platform.c
index 5f7f0edb..6a2ab312 100644
--- a/solaris/Platform.c
+++ b/solaris/Platform.c
@@ -296,15 +296,9 @@ bool Platform_getDiskIO(DiskIOData* data) {
    return false;
 }
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted) {
+bool Platform_getNetworkIO(NetworkIOData* data) {
    // TODO
-   *bytesReceived = 0;
-   *packetsReceived = 0;
-   *bytesTransmitted = 0;
-   *packetsTransmitted = 0;
+   (void)data;
    return false;
 }
 
diff --git a/solaris/Platform.h b/solaris/Platform.h
index de2b2c92..08636ef9 100644
--- a/solaris/Platform.h
+++ b/solaris/Platform.h
@@ -19,6 +19,7 @@ in the source distribution for its full text.
 #include "Action.h"
 #include "BatteryMeter.h"
 #include "DiskIOMeter.h"
+#include "NetworkIOMeter.h"
 #include "ProcessLocksScreen.h"
 #include "SignalsPanel.h"
 
@@ -74,10 +75,7 @@ FileLocks_ProcessData* Platform_getProcessLocks(pid_t pid);
 
 bool Platform_getDiskIO(DiskIOData* data);
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted);
+bool Platform_getNetworkIO(NetworkIOData* data);
 
 void Platform_getBattery(double* percent, ACPresence* isOnAC);
 
diff --git a/unsupported/Platform.c b/unsupported/Platform.c
index f32b7660..5791acca 100644
--- a/unsupported/Platform.c
+++ b/unsupported/Platform.c
@@ -137,14 +137,8 @@ bool Platform_getDiskIO(DiskIOData* data) {
    return false;
 }
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted) {
-   *bytesReceived = 0;
-   *packetsReceived = 0;
-   *bytesTransmitted = 0;
-   *packetsTransmitted = 0;
+bool Platform_getNetworkIO(NetworkIOData* data) {
+   (void)data;
    return false;
 }
 
diff --git a/unsupported/Platform.h b/unsupported/Platform.h
index 8f57e833..9b98fd97 100644
--- a/unsupported/Platform.h
+++ b/unsupported/Platform.h
@@ -11,6 +11,7 @@ in the source distribution for its full text.
 #include "Action.h"
 #include "BatteryMeter.h"
 #include "DiskIOMeter.h"
+#include "NetworkIOMeter.h"
 #include "ProcessLocksScreen.h"
 #include "SignalsPanel.h"
 #include "UnsupportedProcess.h"
@@ -52,10 +53,7 @@ FileLocks_ProcessData* Platform_getProcessLocks(pid_t pid);
 
 bool Platform_getDiskIO(DiskIOData* data);
 
-bool Platform_getNetworkIO(unsigned long int* bytesReceived,
-                           unsigned long int* packetsReceived,
-                           unsigned long int* bytesTransmitted,
-                           unsigned long int* packetsTransmitted);
+bool Platform_getNetworkIO(NetworkIOData* data);
 
 void Platform_getBattery(double *percent, ACPresence *isOnAC);