ruby-core

Hi,

At Thu, 20 May 2004 01:07:23 +0900,
Jeff Mitchell wrote in [ruby-core:02905]:
> ext/dl/ptr.c:26:
> 
> static void
> rb_dlmem_delete(void *ptr)
> {
>   rb_secure(4);
>   rb_hash_delete(DLMemoryTable, DLLONG2NUM(ptr));
> }
> 
> Eventually ptr crosses the boundary of fixnum, causing a bignum to be
> created.  It appears to be an error to create a non-immediate VALUE
> while inside a free() function (memory corruption or segfault).

It's not a problem.

> Here's my attempted fix.  Note that I change the keys of
> DLMemoryTable.  From what I could decipher from the Japanese
> documentation, it doesn't appear that DLMemoryTable is part of the
> API.

Your modification doesn't work with odd value pointers, though
I agree that DLMemoryTable shouldn't be a matter.

I guess that the purpose of conversion from addresses and
PtrData to Integer is to get rid of marking for GC, then
st_table would be better rather than Hash.

> There was also a memory leak in dlptr_free().

Indeed, but it should be dlfree().

At Thu, 13 May 2004 04:00:41 +0900,
Jeff Mitchell wrote in [ruby-core:02867]:
> A simpler yet unorthodox example is:
> 
> require 'dl'
> srand(2727)
> FOO = "foo"
> loop {
>    DL::PtrData.new(2*FOO.object_id) ;
> }

It feels too dangerous too me.


Index: ext/dl/dl.c
===================================================================
RCS file: /cvs/ruby/src/ruby/ext/dl/dl.c,v
retrieving revision 1.20
diff -U2 -p -d -r1.20 dl.c
--- ext/dl/dl.c	16 Jun 2003 07:25:38 -0000	1.20
+++ ext/dl/dl.c	21 May 2004 02:16:53 -0000
@@ -1,2 +1,3 @@
+/* -*- mode: C; c-file-style: "gnu" -*-
 /*
  * $Id: dl.c,v 1.20 2003/06/16 07:25:38 usa Exp $
@@ -165,7 +166,7 @@ dlsizeof(const char *cstr)
   for (i=0; i<len; i++) {
     n = 1;
-    if (isdigit(cstr[i+1])) {
+    if (ISDIGIT(cstr[i+1])) {
       dlen = 1;
-      while (isdigit(cstr[i+dlen])) { dlen ++; };
+      while (ISDIGIT(cstr[i+dlen])) { dlen ++; };
       dlen --;
       d = ALLOCA_N(char, dlen + 1);
@@ -564,6 +565,12 @@ VALUE
 rb_dl_strdup(VALUE self, VALUE str)
 {
+  void *ptr;
+  long len;
+
   SafeStringValue(str);
-  return rb_dlptr_new(strdup(RSTRING(str)->ptr), RSTRING(str)->len, dlfree);
+  len = RSTRING(str)->len;
+  ptr = memcpy(dlmalloc(len + 1), RSTRING(str)->ptr, len + 1);
+  ((char *)ptr)[len] = '\0';
+  return rb_dlptr_new(ptr, len, dlfree);
 }
 
Index: ext/dl/dl.h
===================================================================
RCS file: /cvs/ruby/src/ruby/ext/dl/dl.h,v
retrieving revision 1.9
diff -U2 -p -d -r1.9 dl.h
--- ext/dl/dl.h	1 Dec 2003 23:02:44 -0000	1.9
+++ ext/dl/dl.h	21 May 2004 02:19:17 -0000
@@ -1,3 +1,3 @@
-/* -*- C -*-
+/* -*- mode: C; c-file-style: "gnu" -*-
  * $Id: dl.h,v 1.9 2003/12/01 23:02:44 ttate Exp $
  */
Index: ext/dl/handle.c
===================================================================
RCS file: /cvs/ruby/src/ruby/ext/dl/handle.c,v
retrieving revision 1.11
diff -U2 -p -d -r1.11 handle.c
--- ext/dl/handle.c	13 Nov 2003 11:46:31 -0000	1.11
+++ ext/dl/handle.c	21 May 2004 02:19:26 -0000
@@ -1,3 +1,3 @@
-/* -*- C -*-
+/* -*- mode: C; c-file-style: "gnu" -*-
  * $Id: handle.c,v 1.11 2003/11/13 11:46:31 ttate Exp $
  */
Index: ext/dl/ptr.c
===================================================================
RCS file: /cvs/ruby/src/ruby/ext/dl/ptr.c,v
retrieving revision 1.19
diff -U2 -p -d -r1.19 ptr.c
--- ext/dl/ptr.c	22 Oct 2003 14:29:20 -0000	1.19
+++ ext/dl/ptr.c	21 May 2004 02:24:42 -0000
@@ -1,3 +1,3 @@
-/* -*- C -*-
+/* -*- mode: C; c-file-style: "gnu" -*-
  * $Id: ptr.c,v 1.19 2003/10/22 14:29:20 ttate Exp $
  */
@@ -5,10 +5,11 @@
 #include <ruby.h>
 #include <ctype.h>
-#include <version.h> /* for ruby version code */
+#include <st.h>
 #include "dl.h"
 
 VALUE rb_cDLPtrData;
 VALUE rb_mDLMemorySpace;
-static VALUE DLMemoryTable;
+static st_table *DLMemoryTable;
+static int finalized;
 
 #ifndef T_SYMBOL
@@ -16,17 +17,28 @@ static VALUE DLMemoryTable;
 #endif
 
-#if RUBY_VERSION_CODE < 171
-static VALUE
-rb_hash_delete(VALUE hash, VALUE key)
+static void
+free_memtbl(void *ptr)
 {
-  return rb_funcall(hash, rb_intern("delete"), 1, key);
+  if (!DLMemoryTable) return;
+  if (DLMemoryTable->num_entries) {
+    finalized = 1;
+  }
+  else {
+    st_free_table(DLMemoryTable);
+    DLMemoryTable = 0;
+  }
 }
-#endif
 
 static void
 rb_dlmem_delete(void *ptr)
 {
-  rb_secure(4);
-  rb_hash_delete(DLMemoryTable, DLLONG2NUM(ptr));
+  st_data_t key = (st_data_t)ptr;
+
+  if (!DLMemoryTable) return;
+  st_delete(DLMemoryTable, &key, 0);
+  if (finalized && !DLMemoryTable->num_entries) {
+    st_free_table(DLMemoryTable);
+    DLMemoryTable = 0;
+  }
 }
 
@@ -34,9 +46,7 @@ static void
 rb_dlmem_aset(void *ptr, VALUE obj)
 {
-  if (obj == Qnil) {
-    rb_dlmem_delete(ptr);
-  }
-  else{
-    rb_hash_aset(DLMemoryTable, DLLONG2NUM(ptr), DLLONG2NUM(obj));
+  st_data_t val;
+  if (!st_lookup(DLMemoryTable, (st_data_t)ptr, &val) || (VALUE)val != obj) {
+    st_add_direct(DLMemoryTable, (st_data_t)ptr, (st_data_t)obj);
   }
 }
@@ -45,8 +55,9 @@ static VALUE
 rb_dlmem_aref(void *ptr)
 {
-  VALUE val;
+  st_data_t val;
 
-  val = rb_hash_aref(DLMemoryTable, DLLONG2NUM(ptr));
-  return val == Qnil ? Qnil : (VALUE)DLNUM2LONG(val);
+  if (!st_lookup(DLMemoryTable, (st_data_t)ptr, &val))
+    return Qnil;
+  return (VALUE)val;
 }
 
@@ -70,4 +81,5 @@ dlptr_free(struct ptr_data *data)
   if (data->ssize) dlfree(data->ssize);
   if (data->ids) dlfree(data->ids);
+  dlfree(data);
 }
 
@@ -95,5 +107,5 @@ rb_dlptr_new2(VALUE klass, void *ptr, lo
   if (ptr) {
     val = rb_dlmem_aref(ptr);
-    if (val == Qnil) {
+    if (NIL_P(val)) {
       val = Data_Make_Struct(klass, struct ptr_data,
 			     0, dlptr_free, data);
@@ -150,5 +162,5 @@ rb_dlptr2cptr(VALUE val)
     ptr = data->ptr;
   }
-  else if (val == Qnil) {
+  else if (NIL_P(val)) {
     ptr = NULL;
   }
@@ -190,4 +202,5 @@ rb_dlptr_initialize(int argc, VALUE argv
   long s = 0;
 
+  rb_secure(4);
   switch (rb_scan_args(argc, argv, "12", &ptr, &size, &sym)) {
   case 1:
@@ -209,11 +222,17 @@ rb_dlptr_initialize(int argc, VALUE argv
   if (p) {
     Data_Get_Struct(self, struct ptr_data, data);
-    if (data->ptr && data->free) {
-      /* Free previous memory. Use of inappropriate initialize may cause SEGV. */
-      (*(data->free))(data->ptr);
+    if (data->ptr) {
+      rb_dlmem_delete(data->ptr);
+      if (data->free) {
+	/* Free previous memory. Use of inappropriate initialize may cause SEGV. */
+	(*data->free)(data->ptr);
+      }
     }
     data->ptr  = p;
     data->size = s;
     data->free = f;
+    if (p) {
+      if (NIL_P(rb_dlmem_aref(p))) rb_dlmem_aset(p, self);
+    }
   }
 
@@ -526,5 +545,5 @@ rb_dlptr_define_data_type(int argc, VALU
   Data_Get_Struct(self, struct ptr_data, data);
 
-  if (argc == 1 || (argc == 2 && type == Qnil)) {
+  if (NIL_P(type)) {
     if (NUM2INT(data_type) == DLPTR_CTYPE_UNKNOWN) {
       data->ctype = DLPTR_CTYPE_UNKNOWN;
@@ -570,7 +589,7 @@ rb_dlptr_define_data_type(int argc, VALU
     data->stype[i] = *ctype;
     ctype ++;
-    if (isdigit(*ctype)) {
+    if (ISDIGIT(*ctype)) {
       char *p, *d;
-      for (p=ctype; isdigit(*p); p++) ;
+      for (p=ctype; ISDIGIT(*p); p++) ;
       d = ALLOCA_N(char, p - ctype + 1);
       strncpy(d, ctype, p - ctype);
@@ -867,5 +886,5 @@ rb_dlptr_aset(int argc, VALUE argv[], VA
     src = RSTRING(val)->ptr;
     len = RSTRING(val)->len;
-    if (num == Qnil) {
+    if (NIL_P(num)) {
       memcpy(dst, src, len);
     }
@@ -1008,12 +1027,9 @@ rb_dlptr_size(int argc, VALUE argv[], VA
 }
 
-static VALUE
-dlmem_each_i(VALUE assoc, void *data)
+static int
+dlmem_each_i(st_data_t key, st_data_t val, st_data_t arg)
 {
-  VALUE key, val;
-  key = rb_ary_entry(assoc, 0);
-  val = rb_ary_entry(assoc, 1);
-  rb_yield(rb_assoc_new(key,(VALUE)DLNUM2LONG(val)));
-  return Qnil;
+  rb_yield_values(DLLONG2NUM(key), (VALUE)val);
+  return ST_CONTINUE;
 }
 
@@ -1021,5 +1037,5 @@ VALUE
 rb_dlmem_each(VALUE self)
 {
-  rb_iterate(rb_each, DLMemoryTable, dlmem_each_i, 0);
+  st_foreach(DLMemoryTable, dlmem_each_i, 0);
   return Qnil;
 }
@@ -1060,6 +1076,8 @@ Init_dlptr()
 
   rb_mDLMemorySpace = rb_define_module_under(rb_mDL, "MemorySpace");
-  DLMemoryTable = rb_hash_new();
-  rb_define_const(rb_mDLMemorySpace, "MemoryTable", DLMemoryTable);
+  rb_extend_object(rb_mDLMemorySpace, rb_mEnumerable);
+  DLMemoryTable = st_init_numtable();
+  rb_define_const(rb_mDLMemorySpace, "MemoryTable",
+		  Data_Wrap_Struct(rb_cData, 0, free_memtbl, DLMemoryTable));
   rb_define_module_function(rb_mDLMemorySpace, "each", rb_dlmem_each, 0);
 }
Index: ext/dl/sym.c
===================================================================
RCS file: /cvs/ruby/src/ruby/ext/dl/sym.c,v
retrieving revision 1.24
diff -U2 -p -d -r1.24 sym.c
--- ext/dl/sym.c	11 Dec 2003 02:39:58 -0000	1.24
+++ ext/dl/sym.c	21 May 2004 02:19:13 -0000
@@ -1,3 +1,3 @@
-/* -*- C -*-
+/* -*- mode: C; c-file-style: "gnu" -*-
  * $Id: sym.c,v 1.24 2003/12/11 02:39:58 nobu Exp $
  */
Index: ext/dl/lib/dl/import.rb
===================================================================
RCS file: /cvs/ruby/src/ruby/ext/dl/lib/dl/import.rb,v
retrieving revision 1.10
diff -U2 -p -d -r1.10 import.rb
--- ext/dl/lib/dl/import.rb	16 Oct 2003 17:47:17 -0000	1.10
+++ ext/dl/lib/dl/import.rb	21 May 2004 02:21:32 -0000
@@ -10,13 +10,9 @@ module DL
     module Internal
       def init_types()
-	if( !@types )
-	  @types = ::DL::Types.new
-	end
+	@types ||= ::DL::Types.new
       end
 
       def init_sym()
-	if( !@SYM )
-	  @SYM = {}
-	end
+	@SYM ||= {}
       end
 


-- 
Nobu Nakada

Thread

Prev Next

In This Thread

Prev Next