argus 3.0.0 rc.28 memory leaks
Gabriel L. Somlo
somlo at cmu.edu
Fri Sep 15 14:27:40 EDT 2006
On Mon, Sep 11, 2006 at 12:40:45PM -0400, Carter Bullard wrote:
> Hey Gabriel,
> So, ...., I've added the concepts from your patch to argus, however,
> there are a number of things that you are suggesting that are not
> workable. So, we'll need to test the new rc.29, which I should have
> up on the server later today, to see if it solves your memory
> problem.
Carter,
Thanks for making those changes -- there are no more memory leaks that
I can see (although I still end up swapping under certain traffic
conditions -- need to look into that :) )
Meanwhile, couple more things:
1. there's some leftover files under argus-3.0.0.rc.29/argus/ :
.argus.c.swp .ArgusOutput.c.swp .gdb_history
2. ArgusDeleteList() deletes both a WfileStruct and a RfileStruct
the same way, which generates memory errors (RfileStruct
has no member 'filter', so looking for one generates an
invalid memory read operation).
This is fixed in the first attachment (argus-3.0.0.rc.29-mem1.patch)
3. ArgusMalloc(), ArgusCalloc(), and ArgusFree(); The second
attachment replaces the existing alignment magic with calls to
posix_memalign() which does the work for you in a standard way,
and simplifies the code a lot (I also got rid of the
#ifdef ARGUSMEMDEBUG ... #endif code - it wasn't used
anyway, and there are pretty good tools to debug that
should it be needed in the future).
Let me know what you think.
Thanks,
Gabriel
-------------- next part --------------
diff -NarU5 argus-3.0.0.rc.29.orig/argus/argus.c argus-3.0.0.rc.29/argus/argus.c
--- argus-3.0.0.rc.29.orig/argus/argus.c 2006-09-11 11:20:05.000000000 -0400
+++ argus-3.0.0.rc.29/argus/argus.c 2006-09-13 16:41:37.000000000 -0400
@@ -1049,11 +1049,11 @@
void
clearArgusWfile(void)
{
- ArgusDeleteList (ArgusOutputTask->ArgusWfileList, ARGUS_FILE_LIST);
+ ArgusDeleteList (ArgusOutputTask->ArgusWfileList, ARGUS_WFILE_LIST);
ArgusOutputTask->ArgusWfileList = NULL;
}
void
setArgusWfile(char *file, char *filter)
diff -NarU5 argus-3.0.0.rc.29.orig/argus/ArgusOutput.c argus-3.0.0.rc.29/argus/ArgusOutput.c
--- argus-3.0.0.rc.29.orig/argus/ArgusOutput.c 2006-09-11 10:46:34.000000000 -0400
+++ argus-3.0.0.rc.29/argus/ArgusOutput.c 2006-09-13 16:41:04.000000000 -0400
@@ -141,11 +141,11 @@
output->clients[ind].ArgusClientStart++;
ArgusPushBackList (output->ArgusWfileList, (struct ArgusListRecord *) wfile, ARGUS_NOLOCK);
}
} while (output->ArgusWfileList->start->obj != sfile);
- ArgusDeleteList(output->ArgusWfileList, ARGUS_FILE_LIST);
+ ArgusDeleteList(output->ArgusWfileList, ARGUS_WFILE_LIST);
output->ArgusWfileList = NULL;
}
output->ArgusLfd = -1;
diff -NarU5 argus-3.0.0.rc.29.orig/argus/ArgusSource.c argus-3.0.0.rc.29/argus/ArgusSource.c
--- argus-3.0.0.rc.29.orig/argus/ArgusSource.c 2006-09-11 10:12:50.000000000 -0400
+++ argus-3.0.0.rc.29/argus/ArgusSource.c 2006-09-13 16:41:21.000000000 -0400
@@ -205,11 +205,11 @@
if (src->ArgusDeviceList)
ArgusDeleteList(src->ArgusDeviceList, ARGUS_DEVICE_LIST);
if (src->ArgusRfileList != NULL)
- ArgusDeleteList (src->ArgusRfileList, ARGUS_FILE_LIST);
+ ArgusDeleteList (src->ArgusRfileList, ARGUS_RFILE_LIST);
}
#ifdef ARGUSDEBUG
ArgusDebug (1, "ArgusCloseSource(0x%x) deleting source\n", src);
#endif
diff -NarU5 argus-3.0.0.rc.29.orig/argus/ArgusUtil.c argus-3.0.0.rc.29/argus/ArgusUtil.c
--- argus-3.0.0.rc.29.orig/argus/ArgusUtil.c 2006-09-11 11:58:24.000000000 -0400
+++ argus-3.0.0.rc.29/argus/ArgusUtil.c 2006-09-13 17:03:32.000000000 -0400
@@ -107,20 +107,30 @@
{
if (list) {
while (list->start) {
struct ArgusListRecord *retn = ArgusPopFrontList(list, ARGUS_LOCK);
switch (type) {
- case ARGUS_FILE_LIST: {
+ case ARGUS_WFILE_LIST: {
struct ArgusWfileStruct *wfile = (struct ArgusWfileStruct *) retn;
if (wfile->filename != NULL)
free(wfile->filename);
if (wfile->filter != NULL)
free(wfile->filter);
ArgusFree(retn);
break;
}
+ /* ArgusRfileStruct doesn't have a filter field,
+ so accessing it would be an error -- GLS */
+ case ARGUS_RFILE_LIST: {
+ struct ArgusRfileStruct *rfile = (struct ArgusRfileStruct *) retn;
+ if (rfile->name != NULL)
+ free(rfile->name);
+ ArgusFree(retn);
+ break;
+ }
+
case ARGUS_DEVICE_LIST: {
struct ArgusDeviceStruct *device = (struct ArgusDeviceStruct *) retn;
if (device->name != NULL)
free(device->name);
ArgusFree(retn);
diff -NarU5 argus-3.0.0.rc.29.orig/argus/ArgusUtil.h argus-3.0.0.rc.29/argus/ArgusUtil.h
--- argus-3.0.0.rc.29.orig/argus/ArgusUtil.h 2006-09-08 12:57:16.000000000 -0400
+++ argus-3.0.0.rc.29/argus/ArgusUtil.h 2006-09-13 16:40:39.000000000 -0400
@@ -71,13 +71,14 @@
#define ARGUS_NOLOCK 0
#define ARGUS_LOCK 1
-#define ARGUS_FILE_LIST 1
-#define ARGUS_DEVICE_LIST 2
-#define ARGUS_OUTPUT_LIST 3
+#define ARGUS_WFILE_LIST 1
+#define ARGUS_RFILE_LIST 2
+#define ARGUS_DEVICE_LIST 3
+#define ARGUS_OUTPUT_LIST 4
struct ArgusListObjectStruct {
struct ArgusListObjectStruct *nxt;
void *obj;
};
-------------- next part --------------
diff -NarU5 argus-3.0.0.rc.29.orig/common/argus_util.c argus-3.0.0.rc.29/common/argus_util.c
--- argus-3.0.0.rc.29.orig/common/argus_util.c 2006-09-08 15:08:08.000000000 -0400
+++ argus-3.0.0.rc.29/common/argus_util.c 2006-09-15 14:08:18.000000000 -0400
@@ -1011,12 +1011,13 @@
fflush(stdout);
}
+#define _XOPEN_SOURCE 600
#include <stdlib.h>
-#define ARGUS_ALIGN_128 1
+#define ARGUS_ALIGN 128
struct ArgusMemoryHeader {
struct ArgusMemoryHeader *nxt, *prv;
#if defined(__GNUC__)
void *frame[4];
@@ -1033,182 +1034,67 @@
int ArgusAllocTotal = 0;
int ArgusAllocBytes = 0;
int ArgusFreeTotal = 0;
-struct ArgusMemoryList memory = {NULL, 0};
-
-#define ARGUS_ALLOC 0x45672381
void *
ArgusMalloc (int bytes)
{
- void *retn = NULL;
- int offset = 0;
+ void *retn;
- if (bytes) {
+ if (bytes > 0) {
ArgusAllocTotal++;
ArgusAllocBytes += bytes;
-#if defined(ARGUS_ALIGN_128)
- offset = 128;
-#endif
-
-#if defined(ARGUSMEMDEBUG)
- if ((retn = (u_int *) malloc (bytes + sizeof(struct ArgusMemoryHeader) + offset)) != NULL) {
- struct ArgusMemoryHeader *mem = (struct ArgusMemoryHeader *)retn;
- mem->tag = ARGUS_ALLOC;
- mem->len = bytes;
- mem->offset = offset;
-#if defined(__GNUC__)
- mem->frame[0] = __builtin_return_address(0);
- mem->frame[1] = __builtin_return_address(1);
- mem->frame[2] = __builtin_return_address(2);
-#endif
- if (memory.start) {
- mem->nxt = memory.start;
- mem->prv = memory.start->prv;
- mem->prv->nxt = mem;
- mem->nxt->prv = mem;
- memory.end = mem;
- } else {
- memory.start = mem;
- memory.end = mem;
- mem->nxt = mem;
- mem->prv = mem;
- }
- memory.count++;
- memory.total++;
- retn = (void *)(mem + 1);
- }
-#else
- retn = (void *) malloc (bytes + offset);
-#endif
-
- if (retn != NULL) {
-#if defined(ARGUS_ALIGN_128)
- unsigned short toff;
- toff = ((unsigned long)retn & (offset - 1));
- toff = offset - toff;
- retn = (void *)((char *)retn + toff);
- ((unsigned short *)retn)[-1] = toff;
-#endif
- }
+ if (posix_memalign(&retn, ARGUS_ALIGN, bytes) != 0)
+ retn = NULL;
}
+
#ifdef ARGUSDEBUG
ArgusDebug (6, "ArgusMalloc (%d) returning 0x%x\n", bytes, retn);
#endif
+
return (retn);
}
void *
ArgusCalloc (int nitems, int bytes)
{
- int offset = 0, total = nitems * bytes;
- void *retn = NULL;
+ int total = nitems * bytes;
+ void *retn;
if (total) {
ArgusAllocTotal++;
ArgusAllocBytes += total;
-#if defined(ARGUS_ALIGN_128)
- offset = 128;
-#endif
-
-#if defined(ARGUSMEMDEBUG)
- if ((retn = calloc (1, total + sizeof(struct ArgusMemoryHeader) + offset)) != NULL) {
- struct ArgusMemoryHeader *mem = retn;
- mem->tag = ARGUS_ALLOC;
- mem->len = total;
- mem->offset = offset;
-#if defined(__GNUC__)
- mem->frame[0] = __builtin_return_address(0);
- mem->frame[1] = __builtin_return_address(1);
- mem->frame[2] = __builtin_return_address(2);
-#endif
- if (memory.start) {
- mem->nxt = memory.start;
- mem->prv = memory.start->prv;
- mem->prv->nxt = mem;
- mem->nxt->prv = mem;
- memory.end = mem;
- } else {
- memory.start = mem;
- memory.end = mem;
- mem->nxt = mem;
- mem->prv = mem;
- }
- memory.total++;
- memory.count++;
- retn = (void *)(mem + 1);
- }
-#else
- retn = calloc (1, total + offset);
-#endif
-
-#if defined(ARGUS_ALIGN_128)
- if (retn != NULL) {
- unsigned short toff;
- toff = ((unsigned long)retn & (offset - 1));
- toff = offset - toff;
- retn = (void *)((char *)retn + toff);
- ((unsigned short *)retn)[-1] = toff;
+ if (posix_memalign(&retn, ARGUS_ALIGN, total) != 0) {
+ retn = NULL;
+ } else {
+ /* we're a "calloc" replacement, so zero out the block: */
+ memset(retn, 0, total);
}
-#endif
}
#ifdef ARGUSDEBUG
ArgusDebug (6, "ArgusCalloc (%d, %d) returning 0x%x\n", nitems, bytes, retn);
#endif
+
return (retn);
}
void
ArgusFree (void *buf)
{
- void *ptr = buf;
-
- if (ptr) {
+ if (buf) {
ArgusFreeTotal++;
-#if defined(ARGUSMEMDEBUG)
- {
- struct ArgusMemoryHeader *mem = ptr;
-#if defined(ARGUS_ALIGN_128)
- unsigned short offset = ((unsigned short *)mem)[-1];
- mem = (void *)((char *)mem - offset);
-#endif
- mem--;
- if (mem->tag != ARGUS_ALLOC)
- ArgusLog (LOG_ERR, "ArgusFree: buffer error 0x%x", ptr);
- if (memory.count == 1) {
- memory.start = NULL;
- memory.end = NULL;
- } else {
- mem->prv->nxt = mem->nxt;
- mem->nxt->prv = mem->prv;
- if (mem == memory.start) {
- memory.start = mem->nxt;
- } else if (mem == memory.end) {
- memory.end = mem->prv;
- }
- }
- ArgusAllocBytes -= mem->len;
- memory.count--;
- ptr = mem;
- }
-#else
-#if defined(ARGUS_ALIGN_128)
- {
- unsigned short offset;
- if ((offset = ((unsigned short *)ptr)[-1]) > 0)
- ptr = (void *)((char *)ptr - offset);
- }
-#endif
-#endif
- free (ptr);
+ /* FIXME: we should decrement ArgusAllocBytes here,
+ but can't know for sure by how much -- GLS */
+ free (buf);
}
+
#ifdef ARGUSDEBUG
ArgusDebug (6, "ArgusFree (0x%x)\n", buf);
#endif
}
More information about the argus
mailing list