Theoretical 'top' memory leak

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Theoretical 'top' memory leak

Henrik Gustafsson
While looking for a good example on how to get statistics on swap use  
in top I noticed the following in machine.c:swapmode()

---
nswap = swapctl(SWAP_NSWAP, 0, 0);
if (nswap == 0)
        return 0;
---
According to the man-page swapctl returns -1 on error. I assume the  
== 0 is for detecting a system with no swap devices, so it is a valid  
value to check for, but I propose <= 0 in order to catch failed  
syscalls as well (of course, in the case of a failure malloc a few  
lines further down will probably refuse to allocate the requested  
amount of memory and things will work anyway).

Right after the call to malloc another swapctl call is made:

---
rnswap = swapctl(SWAP_STATS, swdev, nswap);
if (rnswap == -1)
         return 0;

/* if rnswap != nswap, then what? */
---

Now, if this call fails the function exits without freeing the  
allocated memory. AFAICS top continues to run even if this function  
returns 0, thus we have a leak. I think adding a free() to the error  
path might be prudent.

As for the comment, I'd like it more if top reported a 0 sized swap  
instead of a partial (if swap space has been added since SWAP_NSWAP)  
or erroneous summation (in the case of removal as the uninitialized  
entries might be calculated if SWF_ENABLE happens to be set).

I can't see that this would actually happen very often in this case,  
but still... :)

Possible patch attached.

// Henrik

[demime 1.01d removed an attachment of type application/octet-stream which had a name of machine.c.diff]

Reply | Threaded
Open this post in threaded view
|

Re: Theoretical 'top' memory leak

Henrik Gustafsson
I blame the email client! :)

Let's hope this works better.

// Henrik


Index: usr.bin/top/machine.c
===================================================================
RCS file: /cvs/src/usr.bin/top/machine.c,v
retrieving revision 1.52
diff -u -r1.52 machine.c
--- usr.bin/top/machine.c 29 Apr 2006 14:40:44 -0000 1.52
+++ usr.bin/top/machine.c 1 Sep 2006 15:43:11 -0000
@@ -717,7 +717,7 @@
        int nswap, rnswap, i;
        nswap = swapctl(SWAP_NSWAP, 0, 0);
- if (nswap == 0)
+ if (nswap <= 0)
                return 0;
        swdev = malloc(nswap * sizeof(*swdev));
@@ -725,10 +725,10 @@
                return 0;
        rnswap = swapctl(SWAP_STATS, swdev, nswap);
- if (rnswap == -1)
+ if (rnswap == -1 || rnswap != nswap) {
+ free(swdev);
                return 0;
-
- /* if rnswap != nswap, then what? */
+ }
        /* Total things up */
        *total = *used = 0;