Skip to content

Commit 1808ce1

Browse files
cfiscfis
authored
Merge pull request #227 from xml4r/dev
Dev
2 parents 4b6ffbf + 681b3fa commit 1808ce1

14 files changed

Lines changed: 196 additions & 65 deletions

‎ext/libxml/libxml.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ void Init_libxml_ruby(void)
4646
mLibXML = rb_define_module("LibXML");
4747

4848
rxml_init_memory();
49+
rxml_init_registry();
4950
rxml_init_xml();
5051
rxml_init_io();
5152
rxml_init_error();

‎ext/libxml/ruby_libxml.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
#include "ruby_xml_dtd.h"
3838
#include "ruby_xml_schema.h"
3939
#include "ruby_xml_relaxng.h"
40+
#include "ruby_xml_registry.h"
4041

4142
extern VALUE mLibXML;
4243

‎ext/libxml/ruby_xml_attr.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ static VALUE rxml_attr_doc_get(VALUE self)
140140
if (xattr->doc == NULL)
141141
return Qnil;
142142
else
143-
return rxml_document_wrap(xattr->doc);
143+
return rxml_registry_lookup(xattr->doc);
144144
}
145145

146146
/*

‎ext/libxml/ruby_xml_attr_decl.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ static VALUE rxml_attr_decl_doc_get(VALUE self)
4343
if (xattr->doc == NULL)
4444
return Qnil;
4545
else
46-
return rxml_document_wrap(xattr->doc);
46+
return rxml_registry_lookup(xattr->doc);
4747
}
4848

4949

‎ext/libxml/ruby_xml_document.c‎

Lines changed: 29 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ void rxml_document_free(void* data)
6464
{
6565
xmlDocPtr xdoc = (xmlDocPtr)data;
6666
if (!xdoc) return;
67-
xdoc->_private = NULL;
67+
rxml_registry_unregister(xdoc);
6868
xmlFreeDoc(xdoc);
6969
}
7070

@@ -78,15 +78,12 @@ VALUE rxml_document_wrap(xmlDocPtr xdoc)
7878
{
7979
VALUE result = Qnil;
8080

81-
// Is this node is already wrapped?
82-
if (xdoc->_private != NULL)
83-
{
84-
result = (VALUE)xdoc->_private;
85-
}
86-
else
81+
// Is this document already wrapped?
82+
result = rxml_registry_lookup(xdoc);
83+
if (NIL_P(result))
8784
{
8885
result = TypedData_Wrap_Struct(cXMLDocument, &rxml_document_data_type, xdoc);
89-
xdoc->_private = (void*)result;
86+
rxml_registry_register(xdoc, result);
9087
}
9188

9289
return result;
@@ -133,7 +130,7 @@ static VALUE rxml_document_initialize(int argc, VALUE *argv, VALUE self)
133130

134131
// Link the ruby object to the document and the document to the ruby object
135132
RTYPEDDATA_DATA(self) = xdoc;
136-
xdoc->_private = (void*)self;
133+
rxml_registry_register(xdoc, self);
137134

138135
return self;
139136
}
@@ -221,18 +218,18 @@ rxml_document_canonicalize(int argc, VALUE *argv, VALUE self)
221218
// Do stuff if ruby hash passed as argument
222219
if (!NIL_P(option_hash))
223220
{
224-
VALUE o_comments = Qnil;
225-
VALUE o_mode = Qnil;
226-
VALUE o_i_ns_prefixes = Qnil;
227-
221+
VALUE o_comments = Qnil;
222+
VALUE o_mode = Qnil;
223+
VALUE o_i_ns_prefixes = Qnil;
224+
228225
Check_Type(option_hash, T_HASH);
229226

230227
o_comments = rb_hash_aref(option_hash, ID2SYM(rb_intern("comments")));
231228
comments = (RTEST(o_comments) ? 1 : 0);
232229

233230
o_mode = rb_hash_aref(option_hash, ID2SYM(rb_intern("mode")));
234231
if (!NIL_P(o_mode))
235-
{
232+
{
236233
Check_Type(o_mode, T_FIXNUM);
237234
c14n_mode = NUM2INT(o_mode);
238235
//TODO: clean this up
@@ -242,25 +239,25 @@ rxml_document_canonicalize(int argc, VALUE *argv, VALUE self)
242239

243240
o_i_ns_prefixes = rb_hash_aref(option_hash, ID2SYM(rb_intern("inclusive_ns_prefixes")));
244241
if (!NIL_P(o_i_ns_prefixes))
245-
{
242+
{
246243
int i;
247244
int p = 0; //pointer array index
248245
VALUE *list_in = NULL;
249246
long list_size = 0;
250247

251-
Check_Type(o_i_ns_prefixes, T_ARRAY);
248+
Check_Type(o_i_ns_prefixes, T_ARRAY);
252249
list_in = RARRAY_PTR(o_i_ns_prefixes);
253250
list_size = RARRAY_LEN(o_i_ns_prefixes);
254251

255252
if (list_size > 0)
256-
{
253+
{
257254
for(i=0; i < list_size; ++i) {
258255
if (p >= C14N_NS_LIMIT) { break; }
259256

260257
if (RTEST(list_in[i]))
261-
{
258+
{
262259
if (TYPE(list_in[i]) == T_STRING)
263-
{
260+
{
264261
inc_ns_prefixes_ptr[p] = (xmlChar *)StringValueCStr(list_in[i]);
265262
p++;
266263
}
@@ -279,29 +276,29 @@ rxml_document_canonicalize(int argc, VALUE *argv, VALUE self)
279276

280277
o_nodes = rb_hash_aref(option_hash, ID2SYM(rb_intern("nodes")));
281278
if (!NIL_P(o_nodes))
282-
{
279+
{
283280
int i;
284281
int p = 0; // index of pointer array
285282
VALUE * list_in = NULL;
286283
long node_list_size = 0;
287284

288-
if (CLASS_OF(o_nodes) == cXMLXPathObject)
289-
{
290-
o_nodes = rb_funcall(o_nodes, rb_intern("to_a"), 0);
291-
}
292-
else
293-
{
294-
Check_Type(o_nodes, T_ARRAY);
295-
}
285+
if (CLASS_OF(o_nodes) == cXMLXPathObject)
286+
{
287+
o_nodes = rb_funcall(o_nodes, rb_intern("to_a"), 0);
288+
}
289+
else
290+
{
291+
Check_Type(o_nodes, T_ARRAY);
292+
}
296293
list_in = RARRAY_PTR(o_nodes);
297294
node_list_size = RARRAY_LEN(o_nodes);
298295

299296
for (i=0; i < node_list_size; ++i)
300-
{
297+
{
301298
if (p >= C14N_NODESET_LIMIT) { break; }
302299

303300
if (RTEST(list_in[i]))
304-
{
301+
{
305302
xmlNodePtr node_ptr;
306303
TypedData_Get_Struct(list_in[i], xmlNode, &rxml_node_data_type, node_ptr);
307304
node_ptr_array[p] = node_ptr;
@@ -936,9 +933,9 @@ static VALUE rxml_document_version_get(VALUE self)
936933
static VALUE rxml_document_xhtml_q(VALUE self)
937934
{
938935
xmlDocPtr xdoc;
939-
xmlDtdPtr xdtd;
936+
xmlDtdPtr xdtd;
940937
TypedData_Get_Struct(self, xmlDoc, &rxml_document_data_type, xdoc);
941-
xdtd = xmlGetIntSubset(xdoc);
938+
xdtd = xmlGetIntSubset(xdoc);
942939
if (xdtd != NULL && xmlIsXHTML(xdtd->SystemID, xdtd->ExternalID) > 0)
943940
return (Qtrue);
944941
else

‎ext/libxml/ruby_xml_dtd.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ static void rxml_dtd_mark(void* data)
4747
xmlDtdPtr xdtd = (xmlDtdPtr)data;
4848
if (xdtd && xdtd->doc)
4949
{
50-
VALUE doc = (VALUE)xdtd->doc->_private;
51-
rb_gc_mark(doc);
50+
VALUE doc = rxml_registry_lookup(xdtd->doc);
51+
if (!NIL_P(doc))
52+
rb_gc_mark(doc);
5253
}
5354
}
5455

‎ext/libxml/ruby_xml_node.c‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ VALUE cXMLNode;
3030
* have a parent and do not belong to a document). In these cases,
3131
* the bindings manage the memory. They do this by installing a free
3232
* function and storing a back pointer to the Ruby object from the xmlnode
33-
* using the _private member on libxml structures. When the Ruby object
34-
* goes out of scope, the underlying libxml structure is freed. Libxml
35-
* itself then frees all child node (recursively).
33+
* using a pointer-keyed registry (see ruby_xml_registry.c). When the
34+
* Ruby object goes out of scope, the underlying libxml structure is freed.
35+
* Libxml itself then frees all child node (recursively).
3636
*
3737
* For all other nodes (the vast majority), the bindings create temporary
3838
* Ruby objects that get freed once they go out of scope. Thus there can be
@@ -60,8 +60,7 @@ static void rxml_node_free(void *data)
6060
responsible for freeing the underlying node.*/
6161
if (xnode->doc == NULL && xnode->parent == NULL)
6262
{
63-
// Remove the back linkage from libxml to Ruby
64-
xnode->_private = NULL;
63+
rxml_registry_unregister(xnode);
6564
xmlFreeNode(xnode);
6665
}
6766
}
@@ -108,14 +107,14 @@ void rxml_node_manage(xmlNodePtr xnode, VALUE node)
108107
{
109108
VALUE *type_ptr = (VALUE *)&RTYPEDDATA(node)->type;
110109
*type_ptr = (*type_ptr & TYPED_DATA_EMBEDDED) | (VALUE)&rxml_node_managed_data_type;
111-
xnode->_private = (void*)node;
110+
rxml_registry_register(xnode, node);
112111
}
113112

114113
void rxml_node_unmanage(xmlNodePtr xnode, VALUE node)
115114
{
116115
VALUE *type_ptr = (VALUE *)&RTYPEDDATA(node)->type;
117116
*type_ptr = (*type_ptr & TYPED_DATA_EMBEDDED) | (VALUE)&rxml_node_data_type;
118-
xnode->_private = NULL;
117+
rxml_registry_unregister(xnode);
119118
}
120119

121120
xmlNodePtr rxml_node_root(xmlNodePtr xnode)
@@ -134,20 +133,16 @@ void rxml_node_mark(xmlNodePtr xnode)
134133
{
135134
if (xnode->doc)
136135
{
137-
if (xnode->doc->_private)
138-
{
139-
VALUE doc = (VALUE)xnode->doc->_private;
140-
rb_gc_mark(doc);
141-
}
136+
VALUE doc = rxml_registry_lookup(xnode->doc);
137+
if (!NIL_P(doc))
138+
rb_gc_mark(doc);
142139
}
143140
else if (xnode->parent)
144141
{
145142
xmlNodePtr root = rxml_node_root(xnode);
146-
if (root->_private)
147-
{
148-
VALUE node = (VALUE)root->_private;
143+
VALUE node = rxml_registry_lookup(root);
144+
if (!NIL_P(node))
149145
rb_gc_mark(node);
150-
}
151146
}
152147
}
153148

@@ -156,11 +151,8 @@ VALUE rxml_node_wrap(xmlNodePtr xnode)
156151
VALUE result = Qnil;
157152

158153
// Is this node already wrapped?
159-
if (xnode->_private)
160-
{
161-
result = (VALUE)xnode->_private;
162-
}
163-
else
154+
result = rxml_registry_lookup(xnode);
155+
if (NIL_P(result))
164156
{
165157
result = TypedData_Wrap_Struct(cXMLNode, &rxml_node_data_type, xnode);
166158
}
@@ -580,7 +572,7 @@ static VALUE rxml_node_doc(VALUE self)
580572
if (xdoc == NULL)
581573
return (Qnil);
582574

583-
return (VALUE)xdoc->_private;
575+
return rxml_registry_lookup(xdoc);
584576
}
585577

586578
/*

‎ext/libxml/ruby_xml_reader.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,11 @@ static void rxml_reader_mark(void* data)
7070
{
7171
xmlTextReaderPtr xreader = (xmlTextReaderPtr)data;
7272
xmlDocPtr xdoc = xmlTextReaderCurrentDoc(xreader);
73-
if (xdoc && xdoc->_private)
73+
if (xdoc)
7474
{
75-
VALUE doc = (VALUE)xdoc->_private;
76-
rb_gc_mark(doc);
75+
VALUE doc = rxml_registry_lookup(xdoc);
76+
if (!NIL_P(doc))
77+
rb_gc_mark(doc);
7778
}
7879
}
7980

‎ext/libxml/ruby_xml_registry.c‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/* Please see the LICENSE file for copyright and distribution information */
2+
3+
#include "ruby_libxml.h"
4+
#include "ruby_xml_registry.h"
5+
6+
#include <ruby/st.h>
7+
8+
static st_table *rxml_registry;
9+
10+
void rxml_init_registry(void)
11+
{
12+
rxml_registry = st_init_numtable();
13+
}
14+
15+
void rxml_registry_register(void *ptr, VALUE obj)
16+
{
17+
st_insert(rxml_registry, (st_data_t)ptr, (st_data_t)obj);
18+
}
19+
20+
void rxml_registry_unregister(void *ptr)
21+
{
22+
st_delete(rxml_registry, (st_data_t *)&ptr, NULL);
23+
}
24+
25+
VALUE rxml_registry_lookup(void *ptr)
26+
{
27+
st_data_t val;
28+
if (st_lookup(rxml_registry, (st_data_t)ptr, &val))
29+
return (VALUE)val;
30+
return Qnil;
31+
}

‎ext/libxml/ruby_xml_registry.h‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/* Please see the LICENSE file for copyright and distribution information */
2+
3+
#ifndef __RXML_REGISTRY__
4+
#define __RXML_REGISTRY__
5+
6+
/* Pointer-to-VALUE registry for mapping C structs (xmlDocPtr, xmlNodePtr,
7+
etc.) back to their Ruby wrappers.
8+
9+
The registry is a global st_table keyed by C pointer address. It is NOT a
10+
GC root -- entries do not prevent garbage collection. Objects stay alive
11+
through the normal mark chains (mark functions look up the registry instead
12+
of reading _private).
13+
14+
Registered pointers MUST be unregistered before the underlying C struct is
15+
freed, typically in the wrapper's dfree function. */
16+
17+
void rxml_init_registry(void);
18+
void rxml_registry_register(void *ptr, VALUE obj);
19+
void rxml_registry_unregister(void *ptr);
20+
VALUE rxml_registry_lookup(void *ptr); /* Qnil on miss */
21+
22+
#endif

0 commit comments

Comments
 (0)