On Wed, Dec 19 2018, Ed Hynan <eh_l...@optonline.net> wrote: > net/wmnetload has a memory leak [patch] > > wmnetload (6.4 release) repeatedly calls getifaddrs(3) > without freeifaddrs.
Indeed. > The patch here fixes and would replace > net/wmnetload/patches/patch-src_ifstat_openbsd_c > not apply over it. Thanks. If you want to make it easier for people to pick up your patches, please try to submit patches using make update-patches and cvs diff. For more information on how to use update-patches, see item 11: https://www.openbsd.org/faq/ports/guide.html Since the fix changes what ends up in the package, it should include a REVISION bump. > Also fixes a compile warning from strcmp() w/o <string.h> Yep. Here's a reworked version, not directly based on your diff. There's no need to keep around what getifaddrs(3) returns, we can always free it. This simplifies the memory management and appears to fix the memory leak in the current package. I have no idea if wmnetload actually works, it seems to just draw an empty black window under my window manager. Could you please test this and report back? Index: Makefile =================================================================== RCS file: /cvs/ports/net/wmnetload/Makefile,v retrieving revision 1.13 diff -u -p -r1.13 Makefile --- Makefile 7 Dec 2015 14:58:54 -0000 1.13 +++ Makefile 19 Dec 2018 18:29:20 -0000 @@ -3,7 +3,7 @@ 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 Index: patches/patch-src_ifstat_openbsd_c =================================================================== RCS file: /cvs/ports/net/wmnetload/patches/patch-src_ifstat_openbsd_c,v retrieving revision 1.2 diff -u -p -r1.2 patch-src_ifstat_openbsd_c --- patches/patch-src_ifstat_openbsd_c 7 Dec 2015 14:58:54 -0000 1.2 +++ patches/patch-src_ifstat_openbsd_c 19 Dec 2018 18:29:20 -0000 @@ -2,9 +2,10 @@ $OpenBSD: patch-src_ifstat_openbsd_c,v 1 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); } -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
