Date: Thu, 6 Aug 1998 19:08:48 -0700 (PDT) From: Archie Cobbs <archie@whistle.com> To: freebsd-current@FreeBSD.ORG Cc: bde@zeta.org.au, wollman@khavrinen.lcs.mit.edu, dg@root.com Subject: Re: memory leaks in libc Message-ID: <199808070208.TAA26936@bubba.whistle.com> In-Reply-To: <199808070123.SAA26723@bubba.whistle.com> from Archie Cobbs at "Aug 6, 98 06:23:27 pm"
next in thread | previous in thread | raw e-mail | index | archive | help
I wrote: > The idea of keeping a hash table of pointers that were gotten via > malloc() is simple enough. It would solve the problem for programs > than need it (a real world example of which exists at Whistle). > > For programs that don't do a lot of putenv()/setenv(), which is > most programs, there would be no difference. In fact, we can optimize > for this common case, which is NO calls to putenv()/setenv(), by > not creating the hash table at all. > > Now all we need is some enterprising soul to come up with the patch... OK, just to prove I'm not lazy... try this. -Archie ___________________________________________________________________________ Archie Cobbs * Whistle Communications, Inc. * http://www.whistle.com Index: setenv.c =================================================================== RCS file: /cvs/freebsd/src/lib/libc/stdlib/setenv.c,v retrieving revision 1.3 diff -c -u -r1.3 setenv.c --- setenv.c 1996/07/12 18:55:21 1.3 +++ setenv.c 1998/08/07 02:05:15 @@ -41,6 +41,20 @@ char *__findenv __P((const char *, int *)); +/* We keep track of pointers gotten via malloc() using a hash table */ +#define NUMBUCKETS 64 +#define HASH(p) ((((int)(p) >> 24) ^ ((int)(p) >> 16) ^ \ + ((int)(p) >> 8) ^ ((int)(p))) % NUMBUCKETS) + +struct hashp { + void *ptr; + struct hashp *next; +}; +static struct hashp **table; + +static void addhash(void *ptr); +static void rmhash(void *ptr); + /* * setenv -- * Set the value of the environmental variable "name" to be @@ -55,6 +69,7 @@ extern char **environ; static int alloced; /* if allocated space before */ register char *c; + char *newptr; int l_value, offset; if (*value == '=') /* no `=' in value */ @@ -90,10 +105,13 @@ offset = cnt; } for (c = (char *)name; *c && *c != '='; ++c); /* no `=' in name */ - if (!(environ[offset] = /* name + `=' + value */ + if (!(newptr = /* name + `=' + value */ malloc((size_t)((int)(c - name) + l_value + 2)))) return (-1); - for (c = environ[offset]; (*c = *name++) && *c != '='; ++c); + rmhash(environ[offset]); /* free old value if malloc'd */ + environ[offset] = newptr; /* replace with new region */ + addhash(newptr); /* remember we malloc'd it */ + for (c = newptr; (*c = *name++) && *c != '='; ++c); for (*c++ = '='; (*c++ = *value++); ); return (0); } @@ -110,8 +128,63 @@ register char **p; int offset; - while (__findenv(name, &offset)) /* if set multiple times */ + while (__findenv(name, &offset)) { /* if set multiple times */ + rmhash(environ[offset]); /* free memory if malloc'd */ for (p = &environ[offset];; ++p) if (!(*p = *(p + 1))) break; + } } + +/* + * addhash(ptr) + * + * Add a pointer that we obtained via malloc() to our secret internal + * hash table, so when this variable is deleted or changed, we know to + * free the memory. + */ +static void +addhash(void *ptr) +{ + struct hashp *s; + int bucket; + + /* Create hash table if it doesn't already exist */ + if (table == NULL) { + if ((table = malloc(NUMBUCKETS * sizeof(*table))) == NULL) + return; + memset(table, 0, NUMBUCKETS * sizeof(*table)); + } + + /* Create new struct holding pointer and add it to the hash bucket */ + if ((s = malloc(sizeof(*s))) == NULL) + return; + s->ptr = ptr; + bucket = HASH(ptr); + s->next = table[bucket]; + table[bucket] = s; +} + +/* + * rmhash(ptr) + * + * Remove a pointer from the hash table and free() it. If the pointer + * was not obtained via malloc(), then it won't be found and we do nothing. + */ +static void +rmhash(void *ptr) +{ + struct hashp **sp, *s; + + if (table == NULL) + return; + for (sp = &table[HASH(ptr)]; (s = *sp) != NULL; sp = &s->next) { + if (s->ptr == ptr) { + *sp = s->next; + free(ptr); + free(s); + return; + } + } +} + To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199808070208.TAA26936>