Fix memory leak, simplify memory management

No need to keep around the ifaddrs list returned by getifaddrs, instead
always free it as soon as we have the data we want.

Leak reported by Ed Hynan who also provided an alternative patch.
Testing hints and ok sthen@
This commit is contained in:
jca 2018-12-19 19:45:26 +00:00
parent fdc2a91593
commit 672a87dec2
2 changed files with 47 additions and 34 deletions

View File

@ -1,9 +1,9 @@
# $OpenBSD: Makefile,v 1.13 2015/12/07 14:58:54 jasper Exp $
# $OpenBSD: Makefile,v 1.14 2018/12/19 19:45:26 jca Exp $
COMMENT= wm-dockapp; simple network interface monitoring tool
DISTNAME= wmnetload-1.3
REVISION= 3
REVISION= 4
CATEGORIES= net x11 x11/windowmaker
HOMEPAGE= http://freshmeat.net/projects/wmnetload

View File

@ -1,10 +1,11 @@
$OpenBSD: patch-src_ifstat_openbsd_c,v 1.2 2015/12/07 14:58:54 jasper Exp $
$OpenBSD: patch-src_ifstat_openbsd_c,v 1.3 2018/12/19 19:45:26 jca Exp $
use getifaddrs(3) instead of libkvm.
--- src/ifstat_openbsd.c.orig Tue Jan 29 09:09:18 2002
+++ src/ifstat_openbsd.c Mon Dec 7 14:31:38 2015
@@ -27,19 +27,14 @@
Index: src/ifstat_openbsd.c
--- src/ifstat_openbsd.c.orig
+++ src/ifstat_openbsd.c
@@ -27,10 +27,7 @@
#include <sys/types.h>
#include <sys/socket.h>
#include <net/if.h>
@ -14,19 +15,24 @@ use getifaddrs(3) instead of libkvm.
-#include <nlist.h>
+#include <ifaddrs.h>
#include <stdlib.h>
-#include <string.h>
#include <string.h>
#include "ifstat.h"
@@ -38,21 +35,16 @@
#include "utils.h"
struct ifstatstate {
- void *ifnet_head;
- kvm_t *kd;
+ struct ifaddrs *ifap;
+ int unused;
};
/*
@@ -51,8 +46,6 @@ ifstatstate_t *
- * Do one-time setup stuff for accessing the interface statistics and store
- * the gathered information in an interface statistics state structure.
- * Return the state structure.
+ * Return a dummy state structure.
*/
ifstatstate_t *
if_statinit(void)
{
ifstatstate_t *statep;
@ -35,7 +41,7 @@ use getifaddrs(3) instead of libkvm.
statep = malloc(sizeof (ifstatstate_t));
if (statep == NULL) {
@@ -60,35 +53,8 @@ if_statinit(void)
@@ -60,63 +52,44 @@ if_statinit(void)
return (NULL);
}
@ -66,60 +72,67 @@ use getifaddrs(3) instead of libkvm.
- }
-
return (statep);
fail:
-fail:
- (void) kvm_close(statep->kd);
free(statep);
return (NULL);
- free(statep);
- return (NULL);
}
@@ -100,22 +66,33 @@ fail:
/*
- * Optionally using state stored in `statep', retrieve stats on interface
- * `ifname', and store the statistics in `ifstatsp'.
+ * Retrieve stats on interface `ifname', and store the statistics in `ifstatsp'.
*/
int
if_stats(const char *ifname, ifstatstate_t *statep, ifstats_t *ifstatsp)
{
- void *ifnet_addr = statep->ifnet_head;
- struct ifnet ifnet;
+ struct ifaddrs *ifa;
+ struct ifaddrs *ifa0, *ifa;
+ int ret = 0;
- for (; ifnet_addr != NULL; ifnet_addr = TAILQ_NEXT(&ifnet, if_list)) {
+ if (getifaddrs(&statep->ifap) != 0) {
+ warn("failed to get interface addresses");
+ return (0);
+ }
+ (void)statep;
- if (kvm_read(statep->kd, (unsigned long)ifnet_addr, &ifnet,
- sizeof (struct ifnet)) != sizeof (struct ifnet))
- return (0);
+ for (ifa = statep->ifap; ifa != NULL; ifa = ifa->ifa_next) {
+ if (strcmp(ifname, ifa->ifa_name)) {
+ continue;
+ }
+ if (getifaddrs(&ifa0) != 0) {
+ warn("failed to get interface addresses");
+ return (0);
+ }
- if (strcmp(ifnet.if_xname, ifname) == 0) {
- ifstatsp->rxbytes = ifnet.if_ibytes;
- ifstatsp->txbytes = ifnet.if_obytes;
- return (1);
+ if (ifa->ifa_addr->sa_family == AF_LINK) {
+ struct sockaddr_dl *dl = (struct sockaddr_dl *)ifa->ifa_addr;
+ struct if_data *ifd = NULL;
+ for (ifa = ifa0; ifa != NULL; ifa = ifa->ifa_next) {
+ if (strcmp(ifname, ifa->ifa_name)) {
+ continue;
}
+
+ ifd = ifa->ifa_data;
+ if (ifa->ifa_addr->sa_family == AF_LINK) {
+ struct if_data *ifd = ifa->ifa_data;
+
+ if (ifd != NULL) {
+ ifstatsp->rxbytes = ifd->ifi_ibytes;
+ ifstatsp->txbytes = ifd->ifi_obytes;
+ return 1;
+ ret = 1;
+ break;
+ }
}
+ }
}
+ freeifaddrs(statep->ifap);
return (0);
- return (0);
+ freeifaddrs(ifa0);
+ return (ret);
}
@@ -125,6 +102,6 @@ if_stats(const char *ifname, ifstatstate_t *statep, if
/*
@@ -125,6 +98,5 @@ if_stats(const char *ifname, ifstatstate_t *statep, if
void
if_statfini(ifstatstate_t *statep)
{
- (void) kvm_close(statep->kd);
+ freeifaddrs(statep->ifap);
free(statep);
}