Re: [Fix] Dir mem leak

Hi,

In message "Re: [Fix] Dir mem leak"
    on 02/09/10, [e-mail] <[e-mail]> writes:

|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 haven't noticed that.  How hard to work on non GC environment.

|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.

The latter should not be too complex.  Here's my modifies.  not
tested.  Maybe it's still incomplete though.

Index: dir.c
===================================================================
RCS file: /src/ruby/dir.c,v
retrieving revision 1.71
diff -u -1 -r1.71 dir.c
--- dir.c	2002/06/15 10:24:38	1.71
+++ dir.c	2002/09/10 16:16:21
@@ -655,3 +655,34 @@
 
-static void
+struct glob_args {
+    void (*func) _((const char*, VALUE));
+    const char *c;
+    VALUE v;
+};
+
+static VALUE
+glob_func_caller(args)
+    struct glob_args *args;
+{
+    (*args->func)(args->c, args->v);
+    return Qnil;
+}
+
+static int
+glob_call_func(func, path, arg)
+    void (*func) _((const char*, VALUE));
+    const char *path;
+    VALUE arg;
+{
+    int status;
+    struct glob_args args;
+
+    args.func = func;
+    args.c = path;
+    args.v = arg;
+
+    rb_protect(glob_func_caller, (VALUE)&args, &status);
+    return status;
+}
+
+static int
 glob_helper(path, sub, flags, func, arg)
@@ -665,2 +696,3 @@
     char *p, *m;
+    int status = 0;
 
@@ -674,3 +706,4 @@
 	if (lstat(path, &st) == 0) {
-	    (*func)(path, arg);
+	    status = glob_call_func(func, path, arg);
+	    if (status) return status;
 	}
@@ -681,6 +714,6 @@
 	}
-	return;
+	return 0;
     }
 
-    while (p) {
+    while (p && !status) {
 	if (*p == '/') p++;
@@ -706,2 +739,3 @@
 	        free(base);
+	        free(magic);
 	        break;
@@ -714,4 +748,5 @@
 		    sprintf(buf, "%s%s", base, *base ? m : m+1);
-		    glob_helper(buf, buf+n, flags, func, arg);
+		    status = glob_helper(buf, buf+n, flags, func, arg);
 		    free(buf);
+		    if (status) goto finalize;
 		}
@@ -721,2 +756,3 @@
 		    free(base);
+		    free(magic);
 		    break;
@@ -726,2 +762,3 @@
 		free(base);
+		free(magic);
 		break;
@@ -749,3 +786,5 @@
 			strcpy(t+3, m);
-			glob_helper(buf, t, flags, func, arg);
+			status = glob_helper(buf, t, flags, func, arg);
+			free(buf);
+			if (status) goto finalize;
 		    }
@@ -758,4 +797,4 @@
 		    if (!m) {
-			(*func)(buf, arg);
-			free(buf);
+			status = glob_call_func(func, path, arg);
+			if (status) goto finalize;
 			continue;
@@ -768,2 +807,3 @@
 	    }
+	  finalize:
 	    closedir(dirp);
@@ -773,15 +813,17 @@
 		while (link) {
-		    if (stat(link->path, &st) == 0) {
-			if (S_ISDIR(st.st_mode)) {
-			    int len = strlen(link->path);
-			    int mlen = strlen(m);
-			    char *t = ALLOC_N(char, len+mlen+1);
-
-			    sprintf(t, "%s%s", link->path, m);
-			    glob_helper(t, t+len, flags, func, arg);
-			    free(t);
+		    if (status == 0) {
+			if (stat(link->path, &st) == 0) {
+			    if (S_ISDIR(st.st_mode)) {
+				int len = strlen(link->path);
+				int mlen = strlen(m);
+				char *t = ALLOC_N(char, len+mlen+1);
+
+				sprintf(t, "%s%s", link->path, m);
+				status = glob_helper(t, t+len, flags, func, arg);
+				free(t);
+			    }
 			}
-		    }
-		    else {
-			rb_sys_warning(link->path);
+			else {
+			    rb_sys_warning(link->path);
+			}
 		    }
@@ -797,2 +839,3 @@
     }
+    return status;
 }
@@ -806,3 +849,4 @@
 {
-    glob_helper(path, 0, flags, func, arg);
+    int status = glob_helper(path, 0, flags, func, arg);
+    if (status) rb_jump_tag(status);
 }