[macruby-changes] [523] MacRuby/trunk/objc.m

source_changes at macosforge.org source_changes at macosforge.org
Fri Aug 29 19:49:58 PDT 2008


Revision: 523
          http://trac.macosforge.org/projects/ruby/changeset/523
Author:   lsansonetti at apple.com
Date:     2008-08-29 19:49:58 -0700 (Fri, 29 Aug 2008)
Log Message:
-----------
refactoring of the C structure boxing code, should also fix the memory crashers

Modified Paths:
--------------
    MacRuby/trunk/objc.m

Modified: MacRuby/trunk/objc.m
===================================================================
--- MacRuby/trunk/objc.m	2008-08-30 02:47:11 UTC (rev 522)
+++ MacRuby/trunk/objc.m	2008-08-30 02:49:58 UTC (rev 523)
@@ -436,53 +436,6 @@
     return true;
 }
 
-static void *
-bs_element_boxed_get_data(bs_element_boxed_t *bs_boxed, VALUE rval,
-			  bool *success)
-{
-    void *data;
-
-    assert(bs_boxed->ffi_type != NULL);
-
-    if (NIL_P(rval) && bs_boxed->ffi_type == &ffi_type_pointer) {
-	*success = true;
-	return NULL;
-    }
-
-    if (rb_obj_is_kind_of(rval, rb_cBoxed) == Qfalse) {
-	*success = false;
-	return NULL;
-    } 
-    
-    Data_Get_Struct(rval, void, data);
-
-    if (bs_boxed->type == BS_ELEMENT_STRUCT) {
-	bs_element_struct_t *bs_struct;
-	unsigned i;
-
-	bs_struct = (bs_element_struct_t *)bs_boxed->value;
-
-	/* Resync the ivars if necessary.
-	 * This is required as a field may nest another structure, which
-	 * could have been modified as a copy in the Ruby world.
-	 */
-	for (i = 0; i < bs_struct->fields_count; i++) {
-	    VALUE *v;
-	    v = &((VALUE *)(data + bs_boxed->ffi_type->size))[i];
-	    if (*v != 0) {
-		char buf[512];
-		snprintf(buf, sizeof buf, "%s=", bs_struct->fields[i].name);
-		rb_funcall(rval, rb_intern(buf), 1, *v);
-		*v = 0;
-	    }
-	}
-    }
-
-    *success = true;		
-
-    return data;
-}
-
 static void
 rb_bs_boxed_assert_ffitype_ok(bs_element_boxed_t *bs_boxed)
 {
@@ -494,12 +447,11 @@
     assert(bs_boxed->ffi_type != NULL);
 }
 
+static void rb_objc_ocval_to_rval(void **ocval, const char *octype, VALUE *rbval);
+
 static VALUE
 rb_bs_boxed_new_from_ocdata(bs_element_boxed_t *bs_boxed, void *ocval)
 {
-    void *data;
-    size_t soffset;
-
     if (ocval == NULL)
 	return Qnil;
 
@@ -508,17 +460,32 @@
 
     rb_bs_boxed_assert_ffitype_ok(bs_boxed);
 
-    soffset = 0;
     if (bs_boxed->type == BS_ELEMENT_STRUCT) {
-	soffset = ((bs_element_struct_t *)bs_boxed->value)->fields_count 
-		* sizeof(VALUE);
+	bs_element_struct_t *bs_struct = (bs_element_struct_t *)bs_boxed->value;
+	VALUE *data;
+	int i;
+	size_t pos;
+
+	data = (VALUE *)xmalloc(bs_struct->fields_count * sizeof(VALUE));
+	for (i = 0, pos = 0; i < bs_struct->fields_count; i++) {
+	    bs_element_struct_field_t *bs_field = 
+		(bs_element_struct_field_t *)&bs_struct->fields[i];
+	    VALUE fval;
+
+	    rb_objc_ocval_to_rval(ocval + pos, bs_field->type, &fval);
+	    GC_WB(&data[i], fval);
+	    pos += rb_objc_octype_to_ffitype(bs_field->type)->size;
+	}
+	return Data_Wrap_Struct(bs_boxed->klass, NULL, NULL, data);     
     }
+    else {
+	void *data;
 
-    data = xmalloc(soffset + bs_boxed->ffi_type->size);
-    memcpy(data, ocval, bs_boxed->ffi_type->size);
-    memset(data + bs_boxed->ffi_type->size, 0, soffset);
+	data = xmalloc(bs_boxed->ffi_type->size);
+	memcpy(data, ocval, bs_boxed->ffi_type->size);
 
-    return Data_Wrap_Struct(bs_boxed->klass, NULL, NULL, data);     
+	return Data_Wrap_Struct(bs_boxed->klass, NULL, NULL, data);     
+    }
 }
 
 static long
@@ -583,8 +550,6 @@
     return val;
 }
 
-static void rb_objc_ocval_to_rbval(void **ocval, const char *octype, VALUE *rbval);
-
 static VALUE
 rb_pointer_aref(VALUE recv, VALUE i)
 {
@@ -600,71 +565,81 @@
 
     idx = FIX2INT(i);
 
-    rb_objc_ocval_to_rbval(data->ptr + (idx * rb_objc_octype_size(data->type)),
-			   data->type, &ret);
+    rb_objc_ocval_to_rval(data->ptr + (idx * rb_objc_octype_size(data->type)),
+			  data->type, &ret);
 
     return ret;
 }
 
-static void *
-rb_objc_rval_to_boxed_data(VALUE rval, bs_element_boxed_t *bs_boxed, bool *ok)
+static bool
+rb_objc_rval_copy_boxed_data(VALUE rval, bs_element_boxed_t *bs_boxed, void *ocval)
 {
-    void *data;
+    rb_bs_boxed_assert_ffitype_ok(bs_boxed);
 
-    if (TYPE(rval) == T_ARRAY && bs_boxed->type == BS_ELEMENT_STRUCT) {
-	bs_element_struct_t *bs_struct;
+    if (bs_boxed->type == BS_ELEMENT_STRUCT) {
+	bs_element_struct_t *bs_struct = (bs_element_struct_t *)bs_boxed->value;
+	bool is_ary;
 	long i, n;
 	size_t pos;
+	VALUE *data = NULL;
 
-	bs_struct = (bs_element_struct_t *)bs_boxed->value;
-
-	rb_bs_boxed_assert_ffitype_ok(bs_boxed);
-
-	n = RARRAY_LEN(rval);
-	if (n < bs_struct->fields_count)
-	    rb_raise(rb_eArgError, 
-		    "not enough elements in array `%s' to create " \
-		    "structure `%s' (%ld for %d)", 
-		    RSTRING_PTR(rb_inspect(rval)), bs_struct->name, n, 
-		    bs_struct->fields_count);
-
-	if (n > bs_struct->fields_count) {
-	    VALUE new_rval = rb_ary_new();
-	    VALUE orig = rval;
-	    rval = rb_ary_dup(rval);
-	    rebuild_new_struct_ary(bs_boxed->ffi_type->elements, rval, 
-		    new_rval);
-	    n = RARRAY_LEN(new_rval);
-	    if (RARRAY_LEN(rval) != 0 || n != bs_struct->fields_count) {
+	is_ary = TYPE(rval) == T_ARRAY;
+	if (is_ary) {
+	    n = RARRAY_LEN(rval);
+	    if (n < bs_struct->fields_count)
 		rb_raise(rb_eArgError, 
-			"too much elements in array `%s' to create " \
+			"not enough elements in array `%s' to create " \
 			"structure `%s' (%ld for %d)", 
-			RSTRING_PTR(rb_inspect(orig)), 
-			bs_struct->name, RARRAY_LEN(orig), 
+			RSTRING_PTR(rb_inspect(rval)), bs_struct->name, n, 
 			bs_struct->fields_count);
+
+	    if (n > bs_struct->fields_count) {
+		VALUE new_rval = rb_ary_new();
+		VALUE orig = rval;
+		rval = rb_ary_dup(rval);
+		rebuild_new_struct_ary(bs_boxed->ffi_type->elements, rval, 
+			new_rval);
+		n = RARRAY_LEN(new_rval);
+		if (RARRAY_LEN(rval) != 0 || n != bs_struct->fields_count) {
+		    rb_raise(rb_eArgError, 
+			    "too much elements in array `%s' to create " \
+			    "structure `%s' (%ld for %d)", 
+			    RSTRING_PTR(rb_inspect(orig)), 
+			    bs_struct->name, RARRAY_LEN(orig), 
+			    bs_struct->fields_count);
+		}
+		rval = new_rval;
 	    }
-	    rval = new_rval;
 	}
+	else {
+	    if (TYPE(rval) != T_DATA)
+		return false;
+	    Data_Get_Struct(rval, VALUE, data);
+	}
 
-	pos = bs_struct->fields_count * sizeof(VALUE);
-	data = xmalloc(bs_boxed->ffi_type->size + pos);
-	memset(data + bs_boxed->ffi_type->size, 0, pos);
-	pos = 0;
+	for (i = 0, pos = 0; i < bs_struct->fields_count; i++) {
+	    char *field_type;
+	    VALUE o;
 
-	for (i = 0; i < bs_struct->fields_count; i++) {
-	    VALUE o = RARRAY_AT(rval, i);
-	    char *field_type = bs_struct->fields[i].type;
-	    rb_objc_rval_to_ocval(o, field_type, data + pos);
+	    field_type = bs_struct->fields[i].type;
+	    o = is_ary ? RARRAY_AT(rval, i) : data[i];
+	    rb_objc_rval_to_ocval(o, field_type, ocval + pos);
 	    pos += rb_objc_octype_to_ffitype(field_type)->size;
 	}
-
-	*ok = true;
     }
     else {
-	data = bs_element_boxed_get_data(bs_boxed, rval, ok);
+	void *data;
+
+	Data_Get_Struct(rval, void, data);
+	if (data == NULL) {
+	    *(void **)ocval = NULL; 
+	}
+	else {
+	    memcpy(ocval, data, bs_boxed->ffi_type->size);
+	}
     }
 
-    return data;
+    return true;
 }
 
 static void
@@ -682,17 +657,7 @@
 
     if (bs_boxeds != NULL
 	&& st_lookup(bs_boxeds, (st_data_t)octype, (st_data_t *)&bs_boxed)) {
-	void *data;
-
-	data = rb_objc_rval_to_boxed_data(rval, bs_boxed, &ok);
-	if (ok) {
-	    if (data == NULL)
-		*(void **)ocval = NULL;
-	    else {
-		memcpy(ocval, data, bs_boxed->ffi_type->size);
-		xfree(data);
-	    }
-	}
+	ok = rb_objc_rval_copy_boxed_data(rval, bs_boxed, (void *)ocval);
 	goto bails; 
     }
 
@@ -752,12 +717,9 @@
 			}
 		    }
 		    else if (st_lookup(bs_boxeds, (st_data_t)octype + 1, 
-				(st_data_t *)&bs_boxed)) {
-			void *data;
-
-			data = rb_objc_rval_to_boxed_data(rval, bs_boxed, &ok);
-			if (ok)
-			    *(void **)ocval = data;
+			     (st_data_t *)&bs_boxed)) {
+			*(void **)ocval = xmalloc(bs_boxed->ffi_type->size);
+			ok = rb_objc_rval_copy_boxed_data(rval, bs_boxed, *(void **)ocval);
 		    }
 		    else {
 			ok = false;
@@ -881,7 +843,7 @@
 }
 
 static void
-rb_objc_ocval_to_rbval(void **ocval, const char *octype, VALUE *rbval)
+rb_objc_ocval_to_rval(void **ocval, const char *octype, VALUE *rbval)
 {
     bool ok;
 
@@ -1208,7 +1170,7 @@
 		VALUE retval;
 		buf[0] = sig->types[0];
 		buf[1] = '\0';
-		rb_objc_ocval_to_rbval(&ffi_ret, buf, &retval);
+		rb_objc_ocval_to_rval(&ffi_ret, buf, &retval);
 		return retval;
 	    }
 	    return Qnil;
@@ -1292,7 +1254,7 @@
 
     if (ffi_rettype != &ffi_type_void) {
 	VALUE resp;
-	rb_objc_ocval_to_rbval(ffi_ret, rettype, &resp);
+	rb_objc_ocval_to_rval(ffi_ret, rettype, &resp);
 	return resp;
     }
     else {
@@ -1563,7 +1525,7 @@
 	    type = rb_objc_method_get_type(method, cif->nargs, bs_method,
 		    i, buf, sizeof buf);
 
-	    rb_objc_ocval_to_rbval(args[i + 2], type, &val);
+	    rb_objc_ocval_to_rval(args[i + 2], type, &val);
 	    argv[i] = val;
 	}
     }
@@ -1947,7 +1909,7 @@
 
     resp = Qnil;
     if (ffi_rettype != &ffi_type_void) {
-	rb_objc_ocval_to_rbval(ffi_ret, bs_func->retval->type, &resp);
+	rb_objc_ocval_to_rval(ffi_ret, bs_func->retval->type, &resp);
     	if (bs_func->retval->already_retained && !rb_objc_resourceful(resp))
 	    CFMakeCollectable((void *)resp);
     }
@@ -1970,7 +1932,7 @@
 	    rb_bug("cannot locate symbol for unresolved bridgesupport " \
 		    "constant `%s'", bs_const->name);
 
-	rb_objc_ocval_to_rbval(sym, bs_const->type, &v);
+	rb_objc_ocval_to_rval(sym, bs_const->type, &v);
 
 	CFMutableDictionaryRef iv_dict = rb_class_ivar_dict(rb_cObject);
 	assert(iv_dict != NULL);
@@ -2006,26 +1968,32 @@
 {
     bs_element_boxed_t *bs_boxed = rb_klass_get_bs_boxed(recv);
     bs_element_struct_t *bs_struct = (bs_element_struct_t *)bs_boxed->value;    
-    void *data;
+    VALUE *data;
     unsigned i;
-    size_t pos;
 
     if (argc > 0 && argc != bs_struct->fields_count)
 	rb_raise(rb_eArgError, "wrong number of arguments (%d for %d)",
 		 argc, bs_struct->fields_count);
 
-    pos = bs_struct->fields_count * sizeof(VALUE);
-    data = (void *)xmalloc(pos + bs_boxed->ffi_type->size);
-    memset(data, 0, pos + bs_boxed->ffi_type->size);
-    pos = 0;
+    data = (VALUE *)xmalloc(bs_struct->fields_count * sizeof(VALUE));
 
-    for (i = 0; i < argc; i++) {
+    for (i = 0; i < bs_struct->fields_count; i++) {
 	bs_element_struct_field_t *bs_field = 
 	    (bs_element_struct_field_t *)&bs_struct->fields[i];
+	size_t fdata_size;
+	void *fdata;
+	VALUE fval;
 
-	rb_objc_rval_to_ocval(argv[i], bs_field->type, data + pos);
-    
-        pos += rb_objc_octype_to_ffitype(bs_field->type)->size;
+	fdata_size = rb_objc_octype_to_ffitype(bs_field->type)->size;
+	fdata = alloca(fdata_size);
+	if (i < argc) {
+	    rb_objc_rval_to_ocval(argv[i], bs_field->type, fdata);
+	}
+	else {
+	    memset(fdata, 0, fdata_size);
+	}
+	rb_objc_ocval_to_rval(fdata, bs_field->type, &fval);
+	GC_WB(&data[i], fval);
     }
 
     return Data_Wrap_Struct(recv, NULL, NULL, data);
@@ -2057,8 +2025,7 @@
     bs_element_struct_t *bs_struct = (bs_element_struct_t *)bs_boxed->value;
     unsigned i;
     const char *ivar_id_str;
-    void *data;
-    size_t pos;
+    VALUE *data;
 
     /* FIXME we should cache the ivar IDs somewhere in the 
      * bs_element_struct_fields 
@@ -2067,26 +2034,16 @@
     ivar_id_str = rb_id2name(rb_bs_struct_field_ivar_id());
     ivar_id_str++; /* skip first '@' */
 
-    Data_Get_Struct(recv, void, data);
+    Data_Get_Struct(recv, VALUE, data);
     assert(data != NULL);
 
-    rb_objc_wb_range(data + bs_boxed->ffi_type->size,
-		     bs_struct->fields_count * sizeof(VALUE));
-
-    for (i = 0, pos = 0; i < bs_struct->fields_count; i++) {
+    for (i = 0; i < bs_struct->fields_count; i++) {
 	bs_element_struct_field_t *bs_field =
 	    (bs_element_struct_field_t *)&bs_struct->fields[i];
 
 	if (strcmp(ivar_id_str, bs_field->name) == 0) {
-	    VALUE *pval;
-
-	    pval = &((VALUE *)(data + bs_boxed->ffi_type->size))[i];
-	    if (*pval == 0) {
-		rb_objc_ocval_to_rbval(data + pos, bs_field->type, pval);
-	    }
-	    return *pval;
+	    return data[i];
 	}
-        pos += rb_objc_octype_to_ffitype(bs_field->type)->size;
     }
 
     rb_bug("can't find field `%s' in recv `%s'", ivar_id_str,
@@ -2102,7 +2059,7 @@
     bs_element_struct_t *bs_struct = (bs_element_struct_t *)bs_boxed->value;
     unsigned i;
     const char *ivar_id_str;
-    void *data;
+    VALUE *data;
     size_t pos;
 
     /* FIXME we should cache the ivar IDs somewhere in the 
@@ -2112,7 +2069,7 @@
     ivar_id_str = rb_id2name(rb_bs_struct_field_ivar_id());
     ivar_id_str++; /* skip first '@' */
 
-    Data_Get_Struct(recv, void, data);
+    Data_Get_Struct(recv, VALUE, data);
     assert(data != NULL);
 
     for (i = 0, pos = 0; i < bs_struct->fields_count; i++) {
@@ -2120,11 +2077,18 @@
 	    (bs_element_struct_field_t *)&bs_struct->fields[i];
 
 	if (strcmp(ivar_id_str, bs_field->name) == 0) {
-	    rb_objc_rval_to_ocval(value, bs_field->type, data + pos);
-	    /* We do not update the cache because `value' may have been
-	     * transformed (ex. fixnum to float).
-	     */
-	    ((VALUE *)(data + bs_boxed->ffi_type->size))[i] = 0;
+	    size_t fdata_size;
+	    void *fdata;
+	    VALUE fval;
+
+	    fdata_size = rb_objc_octype_to_ffitype(bs_field->type)->size;
+	    fdata = alloca(fdata_size);
+
+	    rb_objc_rval_to_ocval(value, bs_field->type, fdata);
+	    rb_objc_ocval_to_rval(fdata, bs_field->type, &fval);
+
+	    GC_WB(&data[i], fval);	
+
 	    return value;
 	}
         pos += rb_objc_octype_to_ffitype(bs_field->type)->size;
@@ -2141,16 +2105,17 @@
 {
     bs_element_boxed_t *bs_boxed = rb_klass_get_bs_boxed(CLASS_OF(recv));
     bs_element_struct_t *bs_struct = (bs_element_struct_t *)bs_boxed->value;    
+    VALUE *data;
     VALUE ary;
     unsigned i;
-
+    
+    Data_Get_Struct(recv, VALUE, data);
+    assert(data != NULL);
+    
     ary = rb_ary_new();
 
     for (i = 0; i < bs_struct->fields_count; i++) {
-	VALUE obj;
-
-	obj = rb_funcall(recv, rb_intern(bs_struct->fields[i].name), 0, NULL);
-	rb_ary_push(ary, obj);
+	rb_ary_push(ary, data[i]);
     }
 
     return ary;
@@ -2160,8 +2125,6 @@
 rb_bs_boxed_is_equal(VALUE recv, VALUE other)
 {
     bs_element_boxed_t *bs_boxed;  
-    bool ok;
-    void *d1, *d2; 
 
     if (recv == other)
 	return Qtrue;
@@ -2173,40 +2136,21 @@
     if (bs_boxed != rb_klass_get_bs_boxed(CLASS_OF(other)))
 	return Qfalse;
 
-    d1 = bs_element_boxed_get_data(bs_boxed, recv, &ok);
-    if (!ok)
-	rb_raise(rb_eRuntimeError, "can't retrieve data for boxed `%s'",
-		 RSTRING_PTR(rb_inspect(recv)));
-
-    d2 = bs_element_boxed_get_data(bs_boxed, other, &ok);
-    if (!ok)
-	rb_raise(rb_eRuntimeError, "can't retrieve data for boxed `%s'",
-		 RSTRING_PTR(rb_inspect(recv)));
-
-    if (d1 == d2)
-	return Qtrue;
-    else if (d1 == NULL || d2 == NULL)
-	return Qfalse;
-
-    return memcmp(d1, d2, bs_boxed->ffi_type->size) == 0 ? Qtrue : Qfalse;
+    return CFEqual((CFTypeRef)recv, (CFTypeRef)other) ? Qtrue : Qfalse;
 }
 
 static VALUE
 rb_bs_struct_dup(VALUE recv)
 {
     bs_element_boxed_t *bs_boxed = rb_klass_get_bs_boxed(CLASS_OF(recv));
-    void *data;
-    bool ok;
+    bs_element_struct_t *bs_struct = (bs_element_struct_t *)bs_boxed->value;    
+    VALUE *data, *new_data;
 
-    data = bs_element_boxed_get_data(bs_boxed, recv, &ok);
-    if (!ok)
-	rb_raise(rb_eRuntimeError, "can't retrieve data for boxed `%s'",
-		 RSTRING_PTR(rb_inspect(recv)));
+    Data_Get_Struct(recv, VALUE, data);
+    new_data = (VALUE *)xmalloc(bs_struct->fields_count * sizeof(VALUE));
+    memcpy(new_data, data, bs_struct->fields_count * sizeof(VALUE));
 
-    if (data == NULL)
-	return Qnil;
-
-    return rb_bs_boxed_new_from_ocdata(bs_boxed, data);
+    return Data_Wrap_Struct(CLASS_OF(recv), NULL, NULL, new_data);
 }
 
 static VALUE
@@ -2221,15 +2165,15 @@
     rb_str_cat2(str, rb_obj_classname(recv));
 
     if (!bs_struct->opaque) {
-	for (i = 0; i < bs_struct->fields_count; i++) {
-	    VALUE obj;
+	VALUE *data;
 
-	    obj = rb_funcall(recv, rb_intern(bs_struct->fields[i].name), 
-			     0, NULL);
+	Data_Get_Struct(recv, VALUE, data);
+
+	for (i = 0; i < bs_struct->fields_count; i++) {
 	    rb_str_cat2(str, " ");
 	    rb_str_cat2(str, bs_struct->fields[i].name);
 	    rb_str_cat2(str, "=");
-	    rb_str_append(str, rb_inspect(obj));
+	    rb_str_append(str, rb_inspect(data[i]));
 	}
     }
 
@@ -2278,8 +2222,9 @@
     if (bs_boxed->type == BS_ELEMENT_STRUCT) {
 	bs_element_struct_t *bs_struct;
 	bs_struct = (bs_element_struct_t *)bs_boxed->value;
-	for (i = 0; i < bs_struct->fields_count; i++)
+	for (i = 0; i < bs_struct->fields_count; i++) {
 	    rb_ary_push(ary, ID2SYM(rb_intern(bs_struct->fields[i].name)));
+	}
     }
     return ary;
 }
@@ -2733,22 +2678,9 @@
 imp_rb_boxed_getValue(void *rcv, SEL sel, void *buffer)
 {
     bs_element_boxed_t *bs_boxed;
-    void *data;
-    bool ok;  
 
     bs_boxed = rb_klass_get_bs_boxed(CLASS_OF(rcv));
-
-    data = bs_element_boxed_get_data(bs_boxed, (VALUE)rcv, &ok);
-    if (!ok)
-	[NSException raise:@"NSException" 
-	    format:@"can't get internal data for boxed type `%s'",
-	    RSTRING_PTR(rb_inspect((VALUE)rcv))];
-    if (data == NULL) {
-	*(void **)buffer = NULL; 
-    }
-    else {
- 	memcpy(buffer, data, bs_boxed->ffi_type->size);
-    }
+    assert(rb_objc_rval_copy_boxed_data((VALUE)rcv, bs_boxed, buffer));
 }
 
 static void
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.macosforge.org/pipermail/macruby-changes/attachments/20080829/095b10f8/attachment-0001.html 


More information about the macruby-changes mailing list