From: Henrik Gustafsson (openbsdfnord.se)
Date: Fri Sep 01 2006 - 11:41:01 CDT

• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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]

• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]