[ruby-core:40402] Re: BitField extension

Hi,

(11/10/24 21:03), [e-mail] wrote:
> I'm now having trouble clone'ing my object though. I'm running into a
> segfault in my code when I call bitfield.clone. I made my sources
> available at http://xnorfz.de/stuff/BitField.tar

1) you need BitField#initialize_copy to use clone and dup methods.

2) overidding Class#new is the deprecated style which was used in 1.6
   or earlier, you should define BitField.allocate and
   BitField#initialize instead.

3) memory allocated with ALLOC() should be freed with xfree(), not
   free().

4) you don't need Check_Type just before NUM2INT() because it is
   implied.

5) cBitField doesn't seem to need to be global.


--- BitField/BitField.cpp~	2011-10-24 20:59:31.000000000 +0900
+++ BitField/BitField.cpp	2011-10-26 14:38:01.000000000 +0900
@@ -3,21 +3,27 @@
 #include <boost/dynamic_bitset.hpp>
 #include "BitField.hpp"
 
+static VALUE bf_s_alloc(VALUE klass)
+{
+	BitField *bf = BitFieldNew();
+	return Data_Wrap_Struct(klass, 0, xfree, bf);
+}
+
 static VALUE bf_init(VALUE self, VALUE size)
 {
-	rb_iv_set(self, "@size", size);
+	BitField *ptr;
+	Data_Get_Struct(self, BitField, ptr);
+	ptr->data.resize(NUM2INT(size));
 	return self;
 }
 
-static VALUE bf_new(VALUE self, VALUE size)
+static VALUE bf_init_copy(VALUE self, VALUE orig)
 {
-	VALUE argv[1];
-	Check_Type(size, T_FIXNUM);
-	BitField *bf = BitFieldNew(FIX2INT(size));
-	VALUE tdata = Data_Wrap_Struct(self, 0, free, bf);
-	argv[0] = size;
-	rb_obj_call_init(tdata, 1, argv);
-	return tdata;
+	BitField *ptr, *ptr2;
+	Data_Get_Struct(self, BitField, ptr);
+	Data_Get_Struct(orig, BitField, ptr2);
+	ptr->data = ptr2->data;
+	return self;
 }
 
 static VALUE bf_size(VALUE self)
@@ -36,7 +42,6 @@
 {
     BitField *ptr;
     Data_Get_Struct(self, BitField, ptr);
-    Check_Type(position, T_FIXNUM);
     return INT2NUM(ptr->data[FIX2INT(position)].flip());
 }
 
@@ -76,14 +81,12 @@
 		}
 	} else {
 		/* Sanity checks for position and value */
-		Check_Type(position, T_FIXNUM);
-		Check_Type(value, T_FIXNUM);
-
-		int pos = FIX2INT(position);
-		if(pos < 0 || pos >= ptr->data.size()) {
+		unsigned int pos = FIX2UINT(position);
+		int val = FIX2INT(value);
+		if(pos >= ptr->data.size()) {
 			rb_raise(rb_eRangeError, "BitField out of range with value %d.", pos);
 		}
-		ptr->data[pos] = FIX2INT(value) % 2;
+		ptr->data[pos] = val % 2;
 	}
 	return Qnil;
 }
@@ -109,7 +112,6 @@
 		return range_ary;
 	} else {
 		/* Sanity checks for position and value */
-		Check_Type(position, T_FIXNUM);
 		int pos = FIX2INT(position);
 		return INT2NUM(ptr->data[pos]);
 	}
@@ -125,17 +127,12 @@
 }
 
 
-VALUE cBitField;
-
 extern "C" {
 	void Init_bitfield() {
+		VALUE cBitField;
+
 		cBitField = rb_define_class("BitField", rb_cObject);
-		rb_define_singleton_method(
-			cBitField,
-			"new",
-			reinterpret_cast<VALUE(*)(...)>(bf_new),
-			1
-		);
+		rb_define_alloc_func(cBitField, bf_s_alloc);
 		rb_define_method(
 			cBitField,
 			"size",
@@ -150,6 +147,12 @@
 		);
 		rb_define_method(
 			cBitField,
+			"initialize_copy",
+			reinterpret_cast<VALUE(*)(...)>(bf_init_copy),
+			1
+		);
+		rb_define_method(
+			cBitField,
 			"[]=",
 			reinterpret_cast<VALUE(*)(...)>(bf_bit_set),
 			2
@@ -183,11 +186,14 @@
 
 
 /* Library Code */
+BitField *BitFieldNew(void)
+{
+	return new(ALLOC(BitField)) BitField();
+}
+
 BitField *BitFieldNew(int size)
 {
-	BitField *bf;
-	bf = ALLOC(BitField);
-	new(bf) BitField();
+	BitField *bf = BitFieldNew();
 	bf->data.resize(size);
 	return bf;
 }
--- BitField/BitField.hpp~	2011-10-24 06:55:15.000000000 +0900
+++ BitField/BitField.hpp	2011-10-26 14:38:06.000000000 +0900
@@ -1,12 +1,11 @@
 #ifndef __BITFIELD_H__
 #define __BITFIELD_H__
 
-RUBY_EXTERN VALUE cBitField;
-
 typedef struct _bitfield {
 	boost::dynamic_bitset<> data;
 } BitField;
 
+BitField *BitFieldNew(void);
 BitField *BitFieldNew(int size);
 void BitFieldSet(BitField *bf, int position, int data);
 int BitFieldGet(BitField *bf, int position);


-- 
Nobu Nakada