Re: [Fix] Dir mem leak
From:
nobu.nokada@...
Date:
2002-09-10 11:41:25 UTC
List:
ruby-core #476
Hi,
At Mon, 9 Sep 2002 18:07:07 +0900,
Michal Rokos wrote:
> magic is allocated as well, but sometimes it's not freed.
Unfortunately, it's not enough.
glob_helper() calls arbitrary functions, even ruby blocks
passed to Dir.glob, so this function must be exception safe.
Also, closedir() must be ensured.
I thought 2 approaches, 1) wrap resources by VALUE, 2) use
rb_protect() all *func and recursive call, but feel the latter
is too complex.
Index: dir.c
===================================================================
RCS file: /cvs/ruby/src/ruby/dir.c,v
retrieving revision 1.71
diff -u -2 -p -r1.71 dir.c
--- dir.c 15 Jun 2002 10:24:38 -0000 1.71
+++ dir.c 10 Sep 2002 11:30:03 -0000
@@ -598,14 +598,11 @@ has_magic(s, send, flags)
}
-static char*
+static VALUE
extract_path(p, pend)
- char *p, *pend;
+ const char *p, *pend;
{
- char *alloc;
int len;
len = pend - p;
- alloc = ALLOC_N(char, len+1);
- memcpy(alloc, p, len);
if (len > 1 && pend[-1] == '/'
#if defined DOSISH
@@ -613,18 +610,15 @@ extract_path(p, pend)
#endif
) {
- alloc[len-1] = 0;
- }
- else {
- alloc[len] = 0;
+ --len;
}
- return alloc;
+ return rb_str_new(p, len);
}
-static char*
+static VALUE
extract_elem(path)
- char *path;
+ const char *path;
{
- char *pend;
+ const char *pend;
pend = strchr(path, '/');
@@ -654,4 +648,25 @@ remove_backslashes(p)
#endif
+struct d_link {
+ char *path;
+ struct d_link *next;
+};
+
+static void d_link_free _((void *));
+static void
+d_link_free(p)
+ void *p;
+{
+ struct d_link *tmp, *link = p;
+ while (link) {
+ tmp = link;
+ link = link->next;
+ free(tmp->path);
+ free(tmp);
+ }
+}
+
+#define DISPOSE(s) (free(RSTRING(s)->ptr), rb_gc_force_recycle(s))
+
static void
glob_helper(path, sub, flags, func, arg)
@@ -687,50 +702,54 @@ glob_helper(path, sub, flags, func, arg)
m = strchr(p, '/');
if (has_magic(p, m, flags)) {
- char *dir, *base, *magic, *buf;
+ const char *dir;
DIR *dirp;
struct dirent *dp;
int recursive = 0;
-
- struct d_link {
- char *path;
- struct d_link *next;
- } *tmp, *link = 0;
+ VALUE dirv, base, magic, buf, link;
+ struct d_link *tmp, **tail;
base = extract_path(path, p);
if (path == p) dir = ".";
- else dir = base;
+ else dir = RSTRING(base)->ptr;
magic = extract_elem(p);
if (stat(dir, &st) < 0) {
if (errno != ENOENT) rb_sys_warning(dir);
- free(base);
+ DISPOSE(base);
break;
}
if (S_ISDIR(st.st_mode)) {
- if (m && strcmp(magic, "**") == 0) {
- int n = strlen(base);
+ if (m && strcmp(RSTRING(magic)->ptr, "**") == 0) {
+ int n = RSTRING(base)->len;
recursive = 1;
- buf = ALLOC_N(char, n+strlen(m)+3);
- sprintf(buf, "%s%s", base, *base ? m : m+1);
- glob_helper(buf, buf+n, flags, func, arg);
- free(buf);
+ buf = rb_str_new(0, n+strlen(m)+2);
+ sprintf(RSTRING(buf)->ptr, "%s%s", RSTRING(base)->ptr, RSTRING(base)->len ? m : m+1);
+ glob_helper(RSTRING(buf)->ptr, RSTRING(buf)->ptr+n, flags, func, arg);
+ DISPOSE(buf);
}
dirp = opendir(dir);
if (dirp == NULL) {
rb_sys_warning(dir);
- free(base);
+ DISPOSE(base);
+ DISPOSE(magic);
break;
}
+ dirv = Data_Wrap_Struct(rb_cData, 0, (RUBY_DATA_FUNC)closedir, dirp);
}
else {
- free(base);
+ DISPOSE(base);
+ DISPOSE(magic);
break;
}
-
+
+ link = Data_Wrap_Struct(rb_cData, 0, d_link_free, 0);
+ tail = (struct d_link **)&DATA_PTR(link);
+ dir = RSTRING(base)->ptr;
#if defined DOSISH
-#define BASE (*base && !((isdirsep(*base) && !base[1]) || (base[1] == ':' && isdirsep(base[2]) && !base[3])))
+#define BASE (*dir && !((isdirsep(*dir) && !dir[1]) || (dir[1] == ':' && isdirsep(dir[2]) && !dir[3])))
#else
-#define BASE (*base && !(*base == '/' && !base[1]))
+#define BASE (*dir && !(*dir == '/' && !dir[1]))
#endif
+ dir = (BASE) ? "/" : "";
for (dp = readdir(dirp); dp != NULL; dp = readdir(dirp)) {
@@ -738,59 +757,67 @@ glob_helper(path, sub, flags, func, arg)
if (strcmp(".", dp->d_name) == 0 || strcmp("..", dp->d_name) == 0)
continue;
- buf = ALLOC_N(char, strlen(base)+NAMLEN(dp)+strlen(m)+6);
- sprintf(buf, "%s%s%s", base, (BASE) ? "/" : "", dp->d_name);
- if (lstat(buf, &st) < 0) {
- if (errno != ENOENT) rb_sys_warning(buf);
+ buf = rb_str_new(0, RSTRING(base)->len + NAMLEN(dp) + strlen(m) + 5);
+ sprintf(RSTRING(buf)->ptr, "%s%s%s", RSTRING(base)->ptr, dir, dp->d_name);
+ if (lstat(RSTRING(buf)->ptr, &st) < 0) {
+ if (errno != ENOENT) rb_sys_warning(RSTRING(buf)->ptr);
continue;
}
if (S_ISDIR(st.st_mode)) {
- char *t = buf+strlen(buf);
+ char *t = RSTRING(buf)->ptr + strlen(RSTRING(buf)->ptr);
strcpy(t, "/**");
strcpy(t+3, m);
- glob_helper(buf, t, flags, func, arg);
+ glob_helper(RSTRING(buf)->ptr, t, flags, func, arg);
}
- free(buf);
+ DISPOSE(buf);
continue;
}
- if (fnmatch(magic, dp->d_name, flags) == 0) {
- buf = ALLOC_N(char, strlen(base)+NAMLEN(dp)+2);
- sprintf(buf, "%s%s%s", base, (BASE) ? "/" : "", dp->d_name);
+ if (fnmatch(RSTRING(magic)->ptr, dp->d_name, flags) == 0) {
+ buf = rb_str_new(0, RSTRING(base)->len + NAMLEN(dp) + 1);
+ sprintf(RSTRING(buf)->ptr, "%s%s%s", RSTRING(base)->ptr, dir, dp->d_name);
if (!m) {
- (*func)(buf, arg);
- free(buf);
+ (*func)(RSTRING(buf)->ptr, arg);
+ DISPOSE(buf);
continue;
}
tmp = ALLOC(struct d_link);
- tmp->path = buf;
- tmp->next = link;
- link = tmp;
+ tmp->path = RSTRING(buf)->ptr;
+ tmp->next = 0;
+ *tail = tmp;
+ tail = &tmp->next;
+ RSTRING(buf)->ptr = 0;
+ RSTRING(buf)->len = 0;
+ rb_gc_force_recycle(buf);
}
}
closedir(dirp);
- free(base);
- free(magic);
- if (link) {
- while (link) {
- if (stat(link->path, &st) == 0) {
+ DATA_PTR(dirv) = 0;
+ rb_gc_force_recycle(dirv);
+ DISPOSE(base);
+ DISPOSE(magic);
+ if ((tmp = DATA_PTR(link)) != 0) {
+ do {
+ if (stat(tmp->path, &st) == 0) {
if (S_ISDIR(st.st_mode)) {
- int len = strlen(link->path);
+ int len = strlen(tmp->path);
int mlen = strlen(m);
- char *t = ALLOC_N(char, len+mlen+1);
+ VALUE t = rb_str_new(0, len+mlen);
- sprintf(t, "%s%s", link->path, m);
- glob_helper(t, t+len, flags, func, arg);
- free(t);
+ sprintf(RSTRING(t)->ptr, "%s%s", tmp->path, m);
+ glob_helper(RSTRING(t)->ptr, RSTRING(t)->ptr+len, flags, func, arg);
+ DISPOSE(t);
}
}
else {
- rb_sys_warning(link->path);
+ rb_sys_warning(tmp->path);
}
- tmp = link;
- link = link->next;
+ DATA_PTR(link) = tmp->next;
free(tmp->path);
free(tmp);
- }
+ } while ((tmp = DATA_PTR(link)) != 0);
+ rb_gc_force_recycle(link);
break;
}
+ d_link_free(DATA_PTR(link));
+ rb_gc_force_recycle(link);
}
p = m;
@@ -856,5 +883,5 @@ push_braces(ary, s, flags)
int flags;
{
- char *buf;
+ VALUE buf;
char *p, *t, *b;
char *lbrace, *rbrace;
@@ -880,8 +907,7 @@ push_braces(ary, s, flags)
if (lbrace) {
- int len = strlen(s);
- buf = xmalloc(len + 1);
- memcpy(buf, s, lbrace-s);
- b = buf + (lbrace-s);
+ buf = rb_str_new(0, strlen(s));
+ memcpy(RSTRING(buf)->ptr, s, lbrace-s);
+ b = RSTRING(buf)->ptr + (lbrace-s);
p = lbrace;
while (*p != '}') {
@@ -893,7 +919,8 @@ push_braces(ary, s, flags)
memcpy(b, t, p-t);
strcpy(b+(p-t), rbrace+1);
- push_braces(ary, buf, flags);
+ push_braces(ary, RSTRING(buf)->ptr, flags);
}
- free(buf);
+ free(RSTRING(buf)->ptr);
+ rb_gc_force_recycle(buf);
}
else {
@@ -910,9 +937,9 @@ rb_push_glob(str, flags)
{
char *p, *pend;
- char *buf;
char *t;
int nest, maxnest;
int noescape = flags & FNM_NOESCAPE;
VALUE ary;
+ VALUE buf;
if (rb_block_given_p())
@@ -922,5 +949,5 @@ rb_push_glob(str, flags)
SafeStringValue(str);
- buf = xmalloc(RSTRING(str)->len + 1);
+ buf = rb_str_new(0, RSTRING(str)->len);
p = RSTRING(str)->ptr;
@@ -928,5 +955,5 @@ rb_push_glob(str, flags)
while (p < pend) {
- t = buf;
+ t = RSTRING(buf)->ptr;
nest = maxnest = 0;
while (p < pend && isdelim(*p)) p++;
@@ -942,12 +969,13 @@ rb_push_glob(str, flags)
*t = '\0';
if (maxnest == 0) {
- push_globs(ary, buf, flags);
+ push_globs(ary, RSTRING(buf)->ptr, flags);
}
else if (nest == 0) {
- push_braces(ary, buf, flags);
+ push_braces(ary, RSTRING(buf)->ptr, flags);
}
/* else unmatched braces */
}
- free(buf);
+ free(RSTRING(buf)->ptr);
+ rb_gc_force_recycle(buf);
return ary;
--
Nobu Nakada