Re: [ ruby-Bugs-3155 ] Dir#seek segfaults on Windows XP

Hi.

>But I think SEGV is not good even if so. I'll fix it.
>
># And I found my own bug on ruby 1.9 while survey.
># seekdir in win32/win32.c must update `bitpos`. :-(

This is patch. This is still hot, so I'll commit after my brain
cool down.

/////////////////////////////////////////
// ruby 1.9

Index: dir.h
===================================================================
RCS file: /src/ruby/win32/dir.h,v
retrieving revision 1.9
diff -u -w -b -p -r1.9 dir.h
--- dir.h	19 Jan 2004 12:28:14 -0000	1.9
+++ dir.h	31 Dec 2005 09:05:39 -0000
@@ -21,9 +21,9 @@ typedef struct {
     char *curr;
     long size;
     long nfiles;
+    long loc;  /* [0, nfiles) */
     struct direct dirstr;
     char *bits;  /* used for d_isdir and d_isrep */
-    long bitpos; /* used for d_isdir and d_isrep */
 } DIR;
 
 
Index: win32.c
===================================================================
RCS file: /src/ruby/win32/win32.c,v
retrieving revision 1.182
diff -u -w -b -p -r1.182 win32.c
--- win32.c	28 Nov 2005 04:21:24 -0000	1.182
+++ win32.c	31 Dec 2005 09:05:43 -0000
@@ -1345,8 +1345,8 @@ rb_w32_cmdvector(const char *cmd, char *
 // return the pointer to the current file name. 
 //
 
-#define GetBit(bits, i) ((bits)[(i) / 8] &  (1 << (i) % 8))
-#define SetBit(bits, i) ((bits)[(i) / 8] |= (1 << (i) % 8))
+#define GetBit(bits, loc, i) (bits[(loc * 2 + i) / 8] &  (1 << (loc * 2 + i) % 8))
+#define SetBit(bits, loc, i) (bits[(loc * 2 + i) / 8] |= (1 << (loc * 2 + i) % 8))
 
 DIR *
 rb_w32_opendir(const char *filename)
@@ -1413,9 +1413,9 @@ rb_w32_opendir(const char *filename)
     p->bits = ALLOC_N(char, 1);
     p->bits[0] = 0;
     if (fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
-	SetBit(p->bits, 0);
+	SetBit(p->bits, p->nfiles, 0);
     if (fd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)
-	SetBit(p->bits, 1);
+	SetBit(p->bits, p->nfiles, 1);
     p->nfiles++;
     
     //
@@ -1448,9 +1448,9 @@ rb_w32_opendir(const char *filename)
 	    p->bits[p->nfiles / 4] = 0;
 	}
 	if (fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
-	    SetBit(p->bits, p->nfiles * 2);
+	    SetBit(p->bits, p->nfiles, 0);
 	if (fd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)
-	    SetBit(p->bits, p->nfiles * 2 + 1);
+	    SetBit(p->bits, p->nfiles, 1);
 
 	p->nfiles++;
 	idx += len+1;
@@ -1461,6 +1461,21 @@ rb_w32_opendir(const char *filename)
     return p;
 }
 
+//
+// Move to next entry
+//
+
+static void
+move_to_next_entry(DIR *dirp)
+{
+    if (dirp->curr) {
+	dirp->loc++;
+	dirp->curr += strlen(dirp->curr) + 1;
+	if (dirp->curr >= (dirp->start + dirp->size)) {
+	    dirp->curr = NULL;
+	}
+    }
+}
 
 //
 // Readdir just returns the current string pointer and bumps the
@@ -1470,7 +1485,6 @@ rb_w32_opendir(const char *filename)
 struct direct  *
 rb_w32_readdir(DIR *dirp)
 {
-    int         len;
     static int  dummy = 0;
 
     if (dirp->curr) {
@@ -1479,9 +1493,8 @@ rb_w32_readdir(DIR *dirp)
 	// first set up the structure to return
 	//
 
-	len = strlen(dirp->curr);
 	strcpy(dirp->dirstr.d_name, dirp->curr);
-	dirp->dirstr.d_namlen = len;
+	dirp->dirstr.d_namlen = strlen(dirp->curr);
 
 	//
 	// Fake inode
@@ -1491,19 +1504,14 @@ rb_w32_readdir(DIR *dirp)
 	//
 	// Attributes
 	//
-	dirp->dirstr.d_isdir = GetBit(dirp->bits, dirp->bitpos);
-	dirp->bitpos++;
-	dirp->dirstr.d_isrep = GetBit(dirp->bits, dirp->bitpos);
-	dirp->bitpos++;
+	dirp->dirstr.d_isdir = GetBit(dirp->bits, dirp->loc, 0);
+	dirp->dirstr.d_isrep = GetBit(dirp->bits, dirp->loc, 1);
 
 	//
 	// Now set up for the next call to readdir
 	//
 
-	dirp->curr += len + 1;
-	if (dirp->curr >= (dirp->start + dirp->size)) {
-	    dirp->curr = NULL;
-	}
+	move_to_next_entry(dirp);
 
 	return &(dirp->dirstr);
 
@@ -1518,7 +1526,7 @@ rb_w32_readdir(DIR *dirp)
 long
 rb_w32_telldir(DIR *dirp)
 {
-	return (long) dirp->curr;	/* ouch! pointer to long cast */
+    return dirp->loc;
 }
 
 //
@@ -1528,7 +1536,11 @@ rb_w32_telldir(DIR *dirp)
 void
 rb_w32_seekdir(DIR *dirp, long loc)
 {
-	dirp->curr = (char *) loc;	/* ouch! long to pointer cast */
+    rb_w32_rewinddir(dirp);
+
+    while (--loc >= 0) {
+	move_to_next_entry(dirp);
+    }
 }
 
 //
@@ -1539,7 +1551,7 @@ void
 rb_w32_rewinddir(DIR *dirp)
 {
 	dirp->curr = dirp->start;
-	dirp->bitpos = 0;
+    dirp->loc = 0;
 }
 
 //


///////////////////////////////////////////
// ruby 1.8 (keep binary compatibility)

Index: win32.c
===================================================================
RCS file: /src/ruby/win32/win32.c,v
retrieving revision 1.103.2.38
diff -u -w -b -p -r1.103.2.38 win32.c
--- win32.c	29 Dec 2005 19:55:58 -0000	1.103.2.38
+++ win32.c	31 Dec 2005 10:04:13 -0000
@@ -1448,6 +1448,21 @@ rb_w32_opendir(const char *filename)
     return p;
 }
 
+//
+// Peek next dir entry
+//
+
+static char *
+next_entry(DIR *dirp, char *p)
+{
+    if (p) {
+	p += strlen(p) + 1;
+	if (p >= (dirp->start + dirp->size)) {
+	    p = NULL;
+	}
+    }
+    return p;
+}
 
 //
 // Readdir just returns the current string pointer and bumps the
@@ -1457,7 +1472,6 @@ rb_w32_opendir(const char *filename)
 struct direct  *
 rb_w32_readdir(DIR *dirp)
 {
-    int         len;
     static int  dummy = 0;
 
     if (dirp->curr) {
@@ -1466,9 +1480,8 @@ rb_w32_readdir(DIR *dirp)
 	// first set up the structure to return
 	//
 
-	len = strlen(dirp->curr);
 	strcpy(dirp->dirstr.d_name, dirp->curr);
-	dirp->dirstr.d_namlen = len;
+	dirp->dirstr.d_namlen = strlen(dirp->curr);
 
 	//
 	// Fake inode
@@ -1479,10 +1492,7 @@ rb_w32_readdir(DIR *dirp)
 	// Now set up for the next call to readdir
 	//
 
-	dirp->curr += len + 1;
-	if (dirp->curr >= (dirp->start + dirp->size)) {
-	    dirp->curr = NULL;
-	}
+	dirp->curr = next_entry(dirp, dirp->curr);
 
 	return &(dirp->dirstr);
 
@@ -1497,7 +1507,12 @@ rb_w32_readdir(DIR *dirp)
 long
 rb_w32_telldir(DIR *dirp)
 {
-	return (long) dirp->curr;	/* ouch! pointer to long cast */
+    char *p; long loc = 0;
+
+    for (p = dirp->start; p != dirp->curr; p = next_entry(dirp, p)) {
+	loc++;
+    }
+    return loc;
 }
 
 //
@@ -1507,7 +1522,11 @@ rb_w32_telldir(DIR *dirp)
 void
 rb_w32_seekdir(DIR *dirp, long loc)
 {
-	dirp->curr = (char *) loc;	/* ouch! long to pointer cast */
+    dirp->curr = dirp->start;
+
+    while (--loc >= 0) {
+	dirp->curr = next_entry(dirp, dirp->curr);
+    }
 }
 
 //

////////////////////////////////////
// test case

require 'test/unit'

require 'tmpdir'
require 'fileutils'

class TestDir < Test::Unit::TestCase

  ROOT = File.join(Dir.tmpdir, "__test_dir__#{$$}")

  def setup
    Dir.mkdir(ROOT)
    for i in ?a..?z
      if i % 2 == 0
        FileUtils.touch(File.join(ROOT, i.chr))
      else
        FileUtils.mkdir(File.join(ROOT, i.chr))
      end
    end
  end

  def teardown
    FileUtils.rm_rf ROOT if File.directory?(ROOT)
  end

  def test_seek
    dir = Dir.open(ROOT)
    begin
      cache = []
      loop do
        pos = dir.tell
        break unless name = dir.read
        cache << [pos, name]
      end
      cache.sort_by { rand }
      for x in cache
        dir.seek(x[0])
        assert_equal(x[1], dir.read)
      end
    ensure
      dir.close
    end
  end
end