From 907389f38b7af99e4b843cd7e89f327e54ef27fc Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Mon, 1 Jun 2026 19:35:32 +0900 Subject: [PATCH 1/2] Use Data_Make_Struct for data allocation Allocate data with Data_Make_Struct so Ruby owns the memory as soon as the object is created. This avoids leaking the ALLOC_N buffer if Data_Wrap_Struct raises while allocating the Ruby object. --- ext/cbor/buffer_class.c | 5 +++-- ext/cbor/packer_class.c | 5 ++--- ext/cbor/unpacker_class.c | 5 ++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/ext/cbor/buffer_class.c b/ext/cbor/buffer_class.c index 14300dd..0356ea6 100644 --- a/ext/cbor/buffer_class.c +++ b/ext/cbor/buffer_class.c @@ -63,10 +63,11 @@ static void Buffer_free(void* data) static VALUE Buffer_alloc(VALUE klass) { - msgpack_buffer_t* b = ALLOC_N(msgpack_buffer_t, 1); + msgpack_buffer_t* b; + VALUE self = Data_Make_Struct(klass, msgpack_buffer_t, msgpack_buffer_mark, Buffer_free, b); msgpack_buffer_init(b); - return Data_Wrap_Struct(klass, msgpack_buffer_mark, Buffer_free, b); + return self; } static ID get_partial_read_method(VALUE io) diff --git a/ext/cbor/packer_class.c b/ext/cbor/packer_class.c index 2a99ab9..8e84913 100644 --- a/ext/cbor/packer_class.c +++ b/ext/cbor/packer_class.c @@ -57,11 +57,10 @@ static void Packer_free(msgpack_packer_t* pk) static VALUE Packer_alloc(VALUE klass) { - msgpack_packer_t* pk = ALLOC_N(msgpack_packer_t, 1); + msgpack_packer_t* pk; + VALUE self = Data_Make_Struct(klass, msgpack_packer_t, msgpack_packer_mark, Packer_free, pk); msgpack_packer_init(pk); - VALUE self = Data_Wrap_Struct(klass, msgpack_packer_mark, Packer_free, pk); - msgpack_packer_set_to_msgpack_method(pk, s_to_msgpack, self); pk->buffer_ref = MessagePack_Buffer_wrap(PACKER_BUFFER_(pk), self); diff --git a/ext/cbor/unpacker_class.c b/ext/cbor/unpacker_class.c index c729e64..c7ae3b3 100644 --- a/ext/cbor/unpacker_class.c +++ b/ext/cbor/unpacker_class.c @@ -57,11 +57,10 @@ static void Unpacker_free(msgpack_unpacker_t* uk) static VALUE Unpacker_alloc(VALUE klass) { - msgpack_unpacker_t* uk = ALLOC_N(msgpack_unpacker_t, 1); + msgpack_unpacker_t* uk; + VALUE self = Data_Make_Struct(klass, msgpack_unpacker_t, msgpack_unpacker_mark, Unpacker_free, uk); msgpack_unpacker_init(uk); - VALUE self = Data_Wrap_Struct(klass, msgpack_unpacker_mark, Unpacker_free, uk); - uk->buffer_ref = MessagePack_Buffer_wrap(UNPACKER_BUFFER_(uk), self); return self; From 08f34f5b9af330e9add9400716c253cb96f02de6 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Mon, 1 Jun 2026 20:27:01 +0900 Subject: [PATCH 2/2] Use typed data for wrapped CBOR objects Prefer TypedData for Buffer, Packer, and Unpacker objects so the extension builds with Ruby 4.1 where the old Data_* access macros are no longer available. Keep MessagePack_Buffer_wrap as a wrap operation because it exposes an existing embedded buffer. --- ext/cbor/buffer_class.c | 59 ++++++++++++++++++++++++++++++++++----- ext/cbor/core_ext.c | 4 +-- ext/cbor/packer_class.c | 52 ++++++++++++++++++++++++++++++---- ext/cbor/packer_class.h | 3 +- ext/cbor/unpacker_class.c | 46 ++++++++++++++++++++++++++---- 5 files changed, 143 insertions(+), 21 deletions(-) diff --git a/ext/cbor/buffer_class.c b/ext/cbor/buffer_class.c index 0356ea6..626e646 100644 --- a/ext/cbor/buffer_class.c +++ b/ext/cbor/buffer_class.c @@ -38,9 +38,10 @@ static ID s_write; static ID s_append; static ID s_close; +static msgpack_buffer_t* Buffer_get(VALUE self); + #define BUFFER(from, name) \ - msgpack_buffer_t *name = NULL; \ - Data_Get_Struct(from, msgpack_buffer_t, name); \ + msgpack_buffer_t *name = Buffer_get(from); \ if(name == NULL) { \ rb_raise(rb_eArgError, "NULL found for " # name " when shouldn't be."); \ } @@ -51,20 +52,61 @@ static ID s_close; rb_raise(rb_eTypeError, "instance of String needed"); \ } +static void Buffer_mark(void* data) +{ + msgpack_buffer_t* b = data; + msgpack_buffer_mark(b); +} + static void Buffer_free(void* data) { if(data == NULL) { return; } - msgpack_buffer_t* b = (msgpack_buffer_t*) data; + msgpack_buffer_t* b = data; msgpack_buffer_destroy(b); - xfree(b); + xfree(data); +} + +#ifdef TypedData_Make_Struct +static size_t Buffer_memsize(const void* data) +{ + return data == NULL ? 0 : sizeof(msgpack_buffer_t); +} + +static const rb_data_type_t buffer_data_type = { + "CBOR::Buffer", + {Buffer_mark, Buffer_free, Buffer_memsize,}, + NULL, NULL, 0 +}; + +static const rb_data_type_t wrapped_buffer_data_type = { + "CBOR::WrappedBuffer", + {Buffer_mark, NULL, Buffer_memsize,}, + &buffer_data_type, NULL, 0 +}; +#endif + +static msgpack_buffer_t* Buffer_get(VALUE self) +{ + msgpack_buffer_t* b = NULL; +#ifdef TypedData_Make_Struct + TypedData_Get_Struct(self, msgpack_buffer_t, &buffer_data_type, b); +#else + Data_Get_Struct(self, msgpack_buffer_t, b); +#endif + return b; } static VALUE Buffer_alloc(VALUE klass) { msgpack_buffer_t* b; - VALUE self = Data_Make_Struct(klass, msgpack_buffer_t, msgpack_buffer_mark, Buffer_free, b); + VALUE self; +#ifdef TypedData_Make_Struct + self = TypedData_Make_Struct(klass, msgpack_buffer_t, &buffer_data_type, b); +#else + self = Data_Make_Struct(klass, msgpack_buffer_t, Buffer_mark, Buffer_free, b); +#endif msgpack_buffer_init(b); return self; @@ -119,7 +161,11 @@ void MessagePack_Buffer_initialize(msgpack_buffer_t* b, VALUE io, VALUE options) VALUE MessagePack_Buffer_wrap(msgpack_buffer_t* b, VALUE owner) { b->owner = owner; - return Data_Wrap_Struct(cMessagePack_Buffer, msgpack_buffer_mark, NULL, b); +#ifdef TypedData_Make_Struct + return TypedData_Wrap_Struct(cMessagePack_Buffer, &wrapped_buffer_data_type, b); +#else + return Data_Wrap_Struct(cMessagePack_Buffer, Buffer_mark, NULL, b); +#endif } static VALUE Buffer_initialize(int argc, VALUE* argv, VALUE self) @@ -513,4 +559,3 @@ void MessagePack_Buffer_module_init(VALUE mMessagePack) rb_define_alias(cMessagePack_Buffer, "to_s", "to_str"); rb_define_method(cMessagePack_Buffer, "to_a", Buffer_to_a, 0); } - diff --git a/ext/cbor/core_ext.c b/ext/cbor/core_ext.c index ed268d6..f8973cc 100644 --- a/ext/cbor/core_ext.c +++ b/ext/cbor/core_ext.c @@ -49,8 +49,7 @@ static inline VALUE delegete_to_pack(int argc, VALUE* argv, VALUE self) return delegete_to_pack(argc, argv, self); \ } \ VALUE packer = argv[0]; \ - msgpack_packer_t *pk; \ - Data_Get_Struct(packer, msgpack_packer_t, pk); + msgpack_packer_t *pk = MessagePack_Packer_get(packer); static VALUE NilClass_to_msgpack(int argc, VALUE* argv, VALUE self) { @@ -198,4 +197,3 @@ void MessagePack_core_ext_module_init() rb_define_method(rb_cCBOR_Simple, "to_cbor", Simple_to_msgpack, -1); rb_define_method(rb_cCBOR_Tagged, "to_cbor", Tagged_to_msgpack, -1); } - diff --git a/ext/cbor/packer_class.c b/ext/cbor/packer_class.c index 8e84913..05d00f4 100644 --- a/ext/cbor/packer_class.c +++ b/ext/cbor/packer_class.c @@ -39,15 +39,23 @@ static ID s_write; //static VALUE s_packer_value; //static msgpack_packer_t* s_packer; +static msgpack_packer_t* Packer_get(VALUE self); + #define PACKER(from, name) \ - msgpack_packer_t* name; \ - Data_Get_Struct(from, msgpack_packer_t, name); \ + msgpack_packer_t* name = Packer_get(from); \ if(name == NULL) { \ rb_raise(rb_eArgError, "NULL found for " # name " when shouldn't be."); \ } -static void Packer_free(msgpack_packer_t* pk) +static void Packer_mark(void* data) { + msgpack_packer_t* pk = data; + msgpack_packer_mark(pk); +} + +static void Packer_free(void* data) +{ + msgpack_packer_t* pk = data; if(pk == NULL) { return; } @@ -55,10 +63,45 @@ static void Packer_free(msgpack_packer_t* pk) xfree(pk); } +#ifdef TypedData_Make_Struct +static size_t Packer_memsize(const void* data) +{ + return data == NULL ? 0 : sizeof(msgpack_packer_t); +} + +static const rb_data_type_t packer_data_type = { + "CBOR::Packer", + {Packer_mark, Packer_free, Packer_memsize,}, + NULL, NULL, 0 +}; +#endif + +static msgpack_packer_t* Packer_get(VALUE self) +{ + msgpack_packer_t* pk = NULL; +#ifdef TypedData_Make_Struct + TypedData_Get_Struct(self, msgpack_packer_t, &packer_data_type, pk); +#else + Data_Get_Struct(self, msgpack_packer_t, pk); +#endif + return pk; +} + +msgpack_packer_t* MessagePack_Packer_get(VALUE self) +{ + PACKER(self, pk); + return pk; +} + static VALUE Packer_alloc(VALUE klass) { msgpack_packer_t* pk; - VALUE self = Data_Make_Struct(klass, msgpack_packer_t, msgpack_packer_mark, Packer_free, pk); + VALUE self; +#ifdef TypedData_Make_Struct + self = TypedData_Make_Struct(klass, msgpack_packer_t, &packer_data_type, pk); +#else + self = Data_Make_Struct(klass, msgpack_packer_t, Packer_mark, Packer_free, pk); +#endif msgpack_packer_init(pk); msgpack_packer_set_to_msgpack_method(pk, s_to_msgpack, self); @@ -300,4 +343,3 @@ void MessagePack_Packer_module_init(VALUE mMessagePack) rb_define_module_function(mMessagePack, "encode", MessagePack_pack_module_method, -1); rb_define_module_function(mMessagePack, "dump", MessagePack_dump_module_method, -1); } - diff --git a/ext/cbor/packer_class.h b/ext/cbor/packer_class.h index fdabd67..bdeb00e 100644 --- a/ext/cbor/packer_class.h +++ b/ext/cbor/packer_class.h @@ -31,9 +31,10 @@ extern VALUE cMessagePack_Packer; +msgpack_packer_t* MessagePack_Packer_get(VALUE self); + void MessagePack_Packer_module_init(VALUE mMessagePack); VALUE MessagePack_pack(int argc, VALUE* argv); #endif - diff --git a/ext/cbor/unpacker_class.c b/ext/cbor/unpacker_class.c index c7ae3b3..b28ec43 100644 --- a/ext/cbor/unpacker_class.c +++ b/ext/cbor/unpacker_class.c @@ -39,15 +39,23 @@ static VALUE eMalformedFormatError; static VALUE eStackError; static VALUE eTypeError; +static msgpack_unpacker_t* Unpacker_get(VALUE self); + #define UNPACKER(from, name) \ - msgpack_unpacker_t *name = NULL; \ - Data_Get_Struct(from, msgpack_unpacker_t, name); \ + msgpack_unpacker_t *name = Unpacker_get(from); \ if(name == NULL) { \ rb_raise(rb_eArgError, "NULL found for " # name " when shouldn't be."); \ } -static void Unpacker_free(msgpack_unpacker_t* uk) +static void Unpacker_mark(void* data) +{ + msgpack_unpacker_t* uk = data; + msgpack_unpacker_mark(uk); +} + +static void Unpacker_free(void* data) { + msgpack_unpacker_t* uk = data; if(uk == NULL) { return; } @@ -55,10 +63,39 @@ static void Unpacker_free(msgpack_unpacker_t* uk) xfree(uk); } +#ifdef TypedData_Make_Struct +static size_t Unpacker_memsize(const void* data) +{ + return data == NULL ? 0 : sizeof(msgpack_unpacker_t); +} + +static const rb_data_type_t unpacker_data_type = { + "CBOR::Unpacker", + {Unpacker_mark, Unpacker_free, Unpacker_memsize,}, + NULL, NULL, 0 +}; +#endif + +static msgpack_unpacker_t* Unpacker_get(VALUE self) +{ + msgpack_unpacker_t* uk = NULL; +#ifdef TypedData_Make_Struct + TypedData_Get_Struct(self, msgpack_unpacker_t, &unpacker_data_type, uk); +#else + Data_Get_Struct(self, msgpack_unpacker_t, uk); +#endif + return uk; +} + static VALUE Unpacker_alloc(VALUE klass) { msgpack_unpacker_t* uk; - VALUE self = Data_Make_Struct(klass, msgpack_unpacker_t, msgpack_unpacker_mark, Unpacker_free, uk); + VALUE self; +#ifdef TypedData_Make_Struct + self = TypedData_Make_Struct(klass, msgpack_unpacker_t, &unpacker_data_type, uk); +#else + self = Data_Make_Struct(klass, msgpack_unpacker_t, Unpacker_mark, Unpacker_free, uk); +#endif msgpack_unpacker_init(uk); uk->buffer_ref = MessagePack_Buffer_wrap(UNPACKER_BUFFER_(uk), self); @@ -425,4 +462,3 @@ void MessagePack_Unpacker_module_init(VALUE mMessagePack) rb_define_module_function(mMessagePack, "unpack", MessagePack_unpack_module_method, -1); rb_define_module_function(mMessagePack, "decode", MessagePack_unpack_module_method, -1); } -