Add a "node unwrap" macro and use it

This commit introduces a `Noko_Node_Get_Struct` macro for unwrapping the underlying data pointer from Nokogiri objects. Then we change all places that unwrap nodes to use the new macro. We also converted xmlNode to use `TypedData_Wrap_Struct` so we can provide a compaction function. Note that we use DATA_PTR instead of "typed" unwrappers everywhere because the large number of C structs that inherit from xmlNode would make the unwrapping code unwieldy and type-check-heavy. Co-Authored-By: Matt Valentine-House <[email protected]> Co-Authored-By: Mike Dalessio <[email protected]>
sparklemotion · Jul 11, 2022 · 720d6f4 · 720d6f4
1 parent cc0e807
commit 720d6f4
Show file tree

Hide file tree

Showing 14 changed files with 96 additions and 88 deletions.
diff --git a/ext/nokogiri/gumbo.c b/ext/nokogiri/gumbo.c
@@ -401,7 +401,7 @@ static xmlNodePtr
 extract_xml_node(VALUE node)
 {
   xmlNodePtr xml_node;
-  Data_Get_Struct(node, xmlNode, xml_node);
+  Noko_Node_Get_Struct(node, xmlNode, xml_node);
   return xml_node;
 }
 

diff --git a/ext/nokogiri/nokogiri.h b/ext/nokogiri/nokogiri.h
@@ -170,6 +170,8 @@ int noko_io_read(void *ctx, char *buffer, int len);
 int noko_io_write(void *ctx, char *buffer, int len);
 int noko_io_close(void *ctx);
 
+#define Noko_Node_Get_Struct(obj,type,sval) ((sval) = (type*)DATA_PTR(obj))
+
 VALUE noko_xml_node_wrap(VALUE klass, xmlNodePtr node) ;
 VALUE noko_xml_node_wrap_node_set_result(xmlNodePtr node, VALUE node_set) ;
 VALUE noko_xml_node_attrs(xmlNodePtr node) ;

diff --git a/ext/nokogiri/xml_attr.c b/ext/nokogiri/xml_attr.c
@@ -16,7 +16,7 @@ set_value(VALUE self, VALUE content)
   xmlChar *value;
   xmlNode *cur;
 
-  Data_Get_Struct(self, xmlAttr, attr);
+  Noko_Node_Get_Struct(self, xmlAttr, attr);
 
   if (attr->children) {
     xmlFreeNodeList(attr->children);
@@ -68,7 +68,7 @@ new (int argc, VALUE *argv, VALUE klass)
     rb_raise(rb_eArgError, "parameter must be a Nokogiri::XML::Document");
   }
 
-  Data_Get_Struct(document, xmlDoc, xml_doc);
+  Noko_Node_Get_Struct(document, xmlDoc, xml_doc);
 
   node = xmlNewDocProp(
            xml_doc,

diff --git a/ext/nokogiri/xml_attribute_decl.c b/ext/nokogiri/xml_attribute_decl.c
@@ -12,7 +12,7 @@ static VALUE
 attribute_type(VALUE self)
 {
   xmlAttributePtr node;
-  Data_Get_Struct(self, xmlAttribute, node);
+  Noko_Node_Get_Struct(self, xmlAttribute, node);
   return INT2NUM((long)node->atype);
 }
 
@@ -26,7 +26,7 @@ static VALUE
 default_value(VALUE self)
 {
   xmlAttributePtr node;
-  Data_Get_Struct(self, xmlAttribute, node);
+  Noko_Node_Get_Struct(self, xmlAttribute, node);
 
   if (node->defaultValue) { return NOKOGIRI_STR_NEW2(node->defaultValue); }
   return Qnil;
@@ -45,7 +45,7 @@ enumeration(VALUE self)
   xmlEnumerationPtr enm;
   VALUE list;
 
-  Data_Get_Struct(self, xmlAttribute, node);
+  Noko_Node_Get_Struct(self, xmlAttribute, node);
 
   list = rb_ary_new();
   enm = node->tree;

diff --git a/ext/nokogiri/xml_cdata.c b/ext/nokogiri/xml_cdata.c
@@ -25,7 +25,7 @@ new (int argc, VALUE *argv, VALUE klass)
 
   rb_scan_args(argc, argv, "2*", &doc, &content, &rest);
 
-  Data_Get_Struct(doc, xmlDoc, xml_doc);
+  Noko_Node_Get_Struct(doc, xmlDoc, xml_doc);
 
   if (!NIL_P(content)) {
     content_str = (xmlChar *)StringValuePtr(content);

diff --git a/ext/nokogiri/xml_document.c b/ext/nokogiri/xml_document.c
@@ -161,7 +161,7 @@ rb_xml_document_root_set(VALUE self, VALUE rb_new_root)
                rb_obj_class(rb_new_root));
     }
 
-    Data_Get_Struct(rb_new_root, xmlNode, c_new_root);
+    Noko_Node_Get_Struct(rb_new_root, xmlNode, c_new_root);
 
     /* If the new root's document is not the same as the current document,
      * then we need to dup the node in to this document. */

diff --git a/ext/nokogiri/xml_dtd.c b/ext/nokogiri/xml_dtd.c
@@ -44,7 +44,7 @@ entities(VALUE self)
   xmlDtdPtr dtd;
   VALUE hash;
 
-  Data_Get_Struct(self, xmlDtd, dtd);
+  Noko_Node_Get_Struct(self, xmlDtd, dtd);
 
   if (!dtd->entities) { return Qnil; }
 
@@ -67,7 +67,7 @@ notations(VALUE self)
   xmlDtdPtr dtd;
   VALUE hash;
 
-  Data_Get_Struct(self, xmlDtd, dtd);
+  Noko_Node_Get_Struct(self, xmlDtd, dtd);
 
   if (!dtd->notations) { return Qnil; }
 
@@ -90,7 +90,7 @@ attributes(VALUE self)
   xmlDtdPtr dtd;
   VALUE hash;
 
-  Data_Get_Struct(self, xmlDtd, dtd);
+  Noko_Node_Get_Struct(self, xmlDtd, dtd);
 
   hash = rb_hash_new();
 
@@ -113,7 +113,7 @@ elements(VALUE self)
   xmlDtdPtr dtd;
   VALUE hash;
 
-  Data_Get_Struct(self, xmlDtd, dtd);
+  Noko_Node_Get_Struct(self, xmlDtd, dtd);
 
   if (!dtd->elements) { return Qnil; }
 
@@ -138,8 +138,8 @@ validate(VALUE self, VALUE document)
   xmlValidCtxtPtr ctxt;
   VALUE error_list;
 
-  Data_Get_Struct(self, xmlDtd, dtd);
-  Data_Get_Struct(document, xmlDoc, doc);
+  Noko_Node_Get_Struct(self, xmlDtd, dtd);
+  Noko_Node_Get_Struct(document, xmlDoc, doc);
   error_list = rb_ary_new();
 
   ctxt = xmlNewValidCtxt();
@@ -165,7 +165,7 @@ static VALUE
 system_id(VALUE self)
 {
   xmlDtdPtr dtd;
-  Data_Get_Struct(self, xmlDtd, dtd);
+  Noko_Node_Get_Struct(self, xmlDtd, dtd);
 
   if (!dtd->SystemID) { return Qnil; }
 
@@ -182,7 +182,7 @@ static VALUE
 external_id(VALUE self)
 {
   xmlDtdPtr dtd;
-  Data_Get_Struct(self, xmlDtd, dtd);
+  Noko_Node_Get_Struct(self, xmlDtd, dtd);
 
   if (!dtd->ExternalID) { return Qnil; }
 

diff --git a/ext/nokogiri/xml_element_decl.c b/ext/nokogiri/xml_element_decl.c
@@ -14,7 +14,7 @@ static VALUE
 element_type(VALUE self)
 {
   xmlElementPtr node;
-  Data_Get_Struct(self, xmlElement, node);
+  Noko_Node_Get_Struct(self, xmlElement, node);
   return INT2NUM((long)node->etype);
 }
 
@@ -28,7 +28,7 @@ static VALUE
 content(VALUE self)
 {
   xmlElementPtr node;
-  Data_Get_Struct(self, xmlElement, node);
+  Noko_Node_Get_Struct(self, xmlElement, node);
 
   if (!node->content) { return Qnil; }
 
@@ -48,7 +48,7 @@ static VALUE
 prefix(VALUE self)
 {
   xmlElementPtr node;
-  Data_Get_Struct(self, xmlElement, node);
+  Noko_Node_Get_Struct(self, xmlElement, node);
 
   if (!node->prefix) { return Qnil; }
 

diff --git a/ext/nokogiri/xml_entity_decl.c b/ext/nokogiri/xml_entity_decl.c
@@ -12,7 +12,7 @@ static VALUE
 original_content(VALUE self)
 {
   xmlEntityPtr node;
-  Data_Get_Struct(self, xmlEntity, node);
+  Noko_Node_Get_Struct(self, xmlEntity, node);
 
   if (!node->orig) { return Qnil; }
 
@@ -29,7 +29,7 @@ static VALUE
 get_content(VALUE self)
 {
   xmlEntityPtr node;
-  Data_Get_Struct(self, xmlEntity, node);
+  Noko_Node_Get_Struct(self, xmlEntity, node);
 
   if (!node->content) { return Qnil; }
 
@@ -46,7 +46,7 @@ static VALUE
 entity_type(VALUE self)
 {
   xmlEntityPtr node;
-  Data_Get_Struct(self, xmlEntity, node);
+  Noko_Node_Get_Struct(self, xmlEntity, node);
 
   return INT2NUM((int)node->etype);
 }
@@ -61,7 +61,7 @@ static VALUE
 external_id(VALUE self)
 {
   xmlEntityPtr node;
-  Data_Get_Struct(self, xmlEntity, node);
+  Noko_Node_Get_Struct(self, xmlEntity, node);
 
   if (!node->ExternalID) { return Qnil; }
 
@@ -78,7 +78,7 @@ static VALUE
 system_id(VALUE self)
 {
   xmlEntityPtr node;
-  Data_Get_Struct(self, xmlEntity, node);
+  Noko_Node_Get_Struct(self, xmlEntity, node);
 
   if (!node->SystemID) { return Qnil; }