From: "H.Yamamoto" <ocean@...2.ccsnet.ne.jp>
Date: 2004-09-10T06:03:28+09:00
Subject: Re: [PATCH] dir.c --- Dir.chdir error handling

Sorry for late posting. Typhoon striked me.....

nobu.nokada@softhome.net wrote:
(2004/09/07 14:07)

>Well, I meant just about preventing a work object from GC;
>making the argument `data2' of rb_ensure() volatile.

I don't know well, but

  VALUE v = ......

  /* GC happened!! before assigned to volatile */

  rb_ensure(......, v); /* v is volatile VALUE */

is safe?

/////////////////////////////////////////////////////////////

Here is another issue.

  Dir.chdir {|path| .... } # temporary move to HOME or LOGDIR

This code passes nil to 'path' now, but document says

  If a block is given, it is passed the name of the new current
  directory

How about this patch?

Index: dir.c
===================================================================
RCS file: /var/cvs/src/ruby/dir.c,v
retrieving revision 1.125
diff -u -w -b -p -r1.125 dir.c
--- dir.c	19 Aug 2004 07:33:15 -0000	1.125
+++ dir.c	9 Sep 2004 13:03:11 -0000
@@ -678,10 +678,10 @@ dir_close(dir)
 
 static void
 dir_chdir(path)
-    const char *path;
+    VALUE path;
 {
-    if (chdir(path) < 0)
-	rb_sys_fail(path);
+    if (chdir(RSTRING(path)->ptr) < 0)
+	rb_sys_fail(RSTRING(path)->ptr);
 }
 
 static int chdir_blocking = 0;
@@ -689,13 +689,12 @@ static VALUE chdir_thread = Qnil;
 
 static VALUE
 chdir_restore(path)
-    char *path;
+    VALUE path;
 {
     chdir_blocking--;
     if (chdir_blocking == 0)
 	chdir_thread = Qnil;
     dir_chdir(path);
-    free(path);
     return Qnil;
 }
 
@@ -744,20 +743,19 @@ dir_s_chdir(argc, argv, obj)
     VALUE *argv;
     VALUE obj;
 {
-    VALUE path = Qnil;
-    char *dist = "";
+    volatile VALUE path;
 
     rb_secure(2);
     if (rb_scan_args(argc, argv, "01", &path) == 1) {
 	FilePathValue(path);
-	dist = RSTRING(path)->ptr;
     }
     else {
-	dist = getenv("HOME");
+	char *dist = getenv("HOME");
 	if (!dist) {
 	    dist = getenv("LOGDIR");
 	    if (!dist) rb_raise(rb_eArgError, "HOME/LOGDIR not set");
 	}
+	path = rb_str_new2(dist);
     }
 
     if (chdir_blocking > 0) {
@@ -766,14 +764,16 @@ dir_s_chdir(argc, argv, obj)
     }
 
     if (rb_block_given_p()) {
-	char *cwd = my_getcwd();
+	char *tmp = my_getcwd();
+	volatile VALUE cwd = rb_str_new2(tmp); free(tmp);
+
+	dir_chdir(path);
 	chdir_blocking++;
 	if (chdir_thread == Qnil)
 	    chdir_thread = rb_thread_current();
-	dir_chdir(dist);
-	return rb_ensure(rb_yield, path, chdir_restore, (VALUE)cwd);
+	return rb_ensure(rb_yield, path, chdir_restore, cwd);
     }
-    dir_chdir(dist);
+    dir_chdir(path);
 
     return INT2FIX(0);
 }