Date: Fri, 7 Aug 1998 10:03:20 -0700 (PDT) From: Archie Cobbs <archie@whistle.com> To: jb@cimlogic.com.au (John Birrell) Cc: archie@whistle.com, freebsd-current@FreeBSD.ORG, bde@zeta.org.au, wollman@khavrinen.lcs.mit.edu, dg@root.com Subject: Re: memory leaks in libc Message-ID: <199808071703.KAA29656@bubba.whistle.com> In-Reply-To: <199808070247.MAA03023@cimlogic.com.au> from John Birrell at "Aug 7, 98 12:47:23 pm"
next in thread | previous in thread | raw e-mail | index | archive | help
John Birrell writes: > Archie Cobbs wrote: > > OK, just to prove I'm not lazy... try this. > > And a thread safe version? Well, setenv() and unsetenv() are not thread safe as they currently exist. How does this look? -Archie ___________________________________________________________________________ Archie Cobbs * Whistle Communications, Inc. * http://www.whistle.com Index: getenv.c =================================================================== RCS file: /cvs/freebsd/src/lib/libc/stdlib/getenv.c,v retrieving revision 1.2 diff -c -u -r1.2 getenv.c --- getenv.c 1995/10/17 21:37:41 1.2 +++ getenv.c 1998/08/07 17:03:04 @@ -39,6 +39,12 @@ #include <stddef.h> #include <string.h> +#include "libc_private.h" +#include "spinlock.h" +static spinlock_t thread_lock = _SPINLOCK_INITIALIZER; +#define THREAD_LOCK() if (__isthreaded) _SPINLOCK(&thread_lock); +#define THREAD_UNLOCK() if (__isthreaded) _SPINUNLOCK(&thread_lock); + inline char *__findenv __P((const char *, int *)); /* @@ -78,6 +84,28 @@ } /* + * __env_lock() + * + * Obtain mutex lock on the environment. Should be static. + */ +void +__env_lock(void) +{ + THREAD_LOCK(); +} + +/* + * __env_unlock() + * + * Release mutex lock on the environment. Should be static. + */ +void +__env_unlock(void) +{ + THREAD_UNLOCK(); +} + +/* * getenv -- * Returns ptr to value associated with name, if any, else NULL. */ @@ -86,6 +114,11 @@ const char *name; { int offset; + char *rtn; - return (__findenv(name, &offset)); + __env_lock(); + rtn = __findenv(name, &offset); + __env_unlock(); + return rtn; } + 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 17:03:04 @@ -40,7 +40,23 @@ #include <string.h> char *__findenv __P((const char *, int *)); +void __env_lock __P((void)); +void __env_unlock __P((void)); +/* 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,16 +71,21 @@ extern char **environ; static int alloced; /* if allocated space before */ register char *c; + char *newptr; int l_value, offset; if (*value == '=') /* no `=' in value */ ++value; l_value = strlen(value); + __env_lock(); if ((c = __findenv(name, &offset))) { /* find if already exists */ - if (!rewrite) + if (!rewrite) { + __env_unlock(); return (0); + } if (strlen(c) >= l_value) { /* old larger; copy over */ while ( (*c++ = *value++) ); + __env_unlock(); return (0); } } else { /* create new slot */ @@ -75,14 +96,18 @@ if (alloced) { /* just increase size */ environ = (char **)realloc((char *)environ, (size_t)(sizeof(char *) * (cnt + 2))); - if (!environ) + if (!environ) { + __env_unlock(); return (-1); + } } else { /* get new space */ alloced = 1; /* copy old entries into it */ p = malloc((size_t)(sizeof(char *) * (cnt + 2))); - if (!p) + if (!p) { + __env_unlock(); return (-1); + } bcopy(environ, p, cnt * sizeof(char *)); environ = p; } @@ -90,11 +115,17 @@ offset = cnt; } for (c = (char *)name; *c && *c != '='; ++c); /* no `=' in name */ - if (!(environ[offset] = /* name + `=' + value */ - malloc((size_t)((int)(c - name) + l_value + 2)))) + if (!(newptr = /* name + `=' + value */ + malloc((size_t)((int)(c - name) + l_value + 2)))) { + __env_unlock(); 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++); ); + __env_unlock(); return (0); } @@ -110,8 +141,64 @@ register char **p; int offset; - while (__findenv(name, &offset)) /* if set multiple times */ + __env_lock(); + 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; + } + __env_unlock(); +} + +/* + * 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; + } + + /* 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) /* race condition here is ok */ + return; + for (sp = &table[HASH(ptr)]; (s = *sp) != NULL; sp = &s->next) { + if (s->ptr == ptr) { + *sp = s->next; + free(ptr); + free(s); + break; + } + } +} + 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?199808071703.KAA29656>