Re: [Gnash-commit] gnash server/asobj/xmlnode.h server/asobj/xmlno...

[strk]

Rob, all the changes to the getter-setters are bogus.
You *need* to check for fn.nargs == 0, as that will
tell you wheter the function is called as a "getter"
or a "setter".

When called as a "setter" (fn.nargs == 1) fn.result
is NULL, as a setter is not intended to return
any value, but rather to set it using the first argument.

After this commit 'make check' segfaults on XML tests.
Example:

#0  0xb7c65edd in gnash::as_value::drop_refs (this=0x0) at 
/home/strk/src/gnash/gnash-head/server/as_value.cpp:571
#1  0xb7c53c79 in gnash::as_value::set_null (this=0x0) at as_value.h:429
#2  0xb7d327fa in xmlnode_lastchild (address@hidden) at 
/home/strk/src/gnash/gnash-head/server/asobj/xmlnode.cpp:666

--strk;


> @@ -579,31 +631,26 @@
>  static void
>  xmlnode_nodetype(const fn_call& fn)
>  {
> +//    GNASH_REPORT_FUNCTION;
> +    
>      assert(dynamic_cast<XMLNode*>(fn.this_ptr));
>      XMLNode *ptr = static_cast<XMLNode*>(fn.this_ptr);
>  
> -    if ( fn.nargs == 0 ) {
> -     fn.result->set_double(ptr->nodeType());
> -    } else {
> -     IF_VERBOSE_ASCODING_ERRORS(
> -         log_aserror("Tried to set read-only property XMLNode.nodeType");
> -         );
> -    }
> +    fn.result->set_int(ptr->nodeType());
>  }

>  
>  // Both a getter and a (do-nothing) setter for firstChild
>  static void
>  xmlnode_firstchild(const fn_call& fn)
>  {
> +//    GNASH_REPORT_FUNCTION;
>      assert(dynamic_cast<XMLNode*>(fn.this_ptr));
>      XMLNode *ptr = static_cast<XMLNode*>(fn.this_ptr);
> -
> -    if ( fn.nargs == 0 ) {
> -     fn.result->set_as_object(ptr->firstChild());
> +    XMLNode *node = ptr->firstChild();
> +    if (node == NULL) {
> +     fn.result->set_null();
>      } else {
> -     IF_VERBOSE_ASCODING_ERRORS(
> -         log_aserror("Tried to set read-only property XMLNode.firstChild");
> -         );
> +     fn.result->set_as_object(node);
>      }
>  }
>  
> @@ -611,15 +658,14 @@
>  static void
>  xmlnode_lastchild(const fn_call& fn)
>  {
> +//    GNASH_REPORT_FUNCTION;
>      assert(dynamic_cast<XMLNode*>(fn.this_ptr));
>      XMLNode *ptr = static_cast<XMLNode*>(fn.this_ptr);
> -
> -    if ( fn.nargs == 0 ) {
> -     fn.result->set_as_object(ptr->lastChild());
> +    XMLNode *node = ptr->lastChild();
> +    if (node == NULL) {
> +     fn.result->set_null();
>      } else {
> -     IF_VERBOSE_ASCODING_ERRORS(
> -         log_aserror("Tried to set read-only property XMLNode.lastChild");
> -         );
> +     fn.result->set_as_object(node);
>      }
>  }
>  
> @@ -627,17 +673,15 @@
>  static void
>  xmlnode_nextsibling(const fn_call& fn)
>  {
> +    GNASH_REPORT_FUNCTION;
> +    
>      assert(dynamic_cast<XMLNode*>(fn.this_ptr));
>      XMLNode *ptr = static_cast<XMLNode*>(fn.this_ptr);
> -
> -    if ( fn.nargs == 0 ) {
> -     log_error("FIXME: XMLNode.nextSibling unimplemented");
> -     //fn.result->set_as_object(ptr->nextSibling());
> +    XMLNode *node = ptr->nextSibling();
> +    if (node == NULL) {
>       fn.result->set_null();
>      } else {
> -     IF_VERBOSE_ASCODING_ERRORS(
> -         log_aserror("Tried to set read-only property XMLNode.nextSibling");
> -         );
> +     fn.result->set_as_object(node);
>      }
>  }
>  
> @@ -645,23 +689,21 @@
>  static void
>  xmlnode_previoussibling(const fn_call& fn)
>  {
> +    GNASH_REPORT_FUNCTION;
>      assert(dynamic_cast<XMLNode*>(fn.this_ptr));
>      XMLNode *ptr = static_cast<XMLNode*>(fn.this_ptr);
> -
> -    if ( fn.nargs == 0 ) {
> -     log_error("FIXME: XMLNode.previousSibling unimplemented");
> -     //fn.result->set_as_object(ptr->previousSibling());
> +    XMLNode *node = ptr->previousSibling();
> +    if (node == NULL) {
>       fn.result->set_null();
>      } else {
> -     IF_VERBOSE_ASCODING_ERRORS(
> -         log_aserror("Tried to set read-only property 
> XMLNode.previousSibling");
> -         );
> +     fn.result->set_as_object(node);
>      }
>  }
>  
>  // extern (used by Global.cpp)
>  void xmlnode_class_init(as_object& global)
>  {
> +//    GNASH_REPORT_FUNCTION;
>      // This is going to be the global XMLNode "class"/"function"
>      static boost::intrusive_ptr<builtin_function> cl;
>  
> 
> Index: testsuite/actionscript.all/XMLNode.as
> ===================================================================
> RCS file: /sources/gnash/gnash/testsuite/actionscript.all/XMLNode.as,v
> retrieving revision 1.8
> retrieving revision 1.9
> diff -u -b -r1.8 -r1.9
> --- testsuite/actionscript.all/XMLNode.as     18 Jan 2007 15:30:53 -0000      
> 1.8
> +++ testsuite/actionscript.all/XMLNode.as     15 Feb 2007 04:05:41 -0000      
> 1.9
> @@ -1,5 +1,5 @@
>  // 
> -//   Copyright (C) 2005, 2006 Free Software Foundation, Inc.
> +//   Copyright (C) 2005, 2006, 2007 Free Software Foundation, Inc.
>  //
>  // This program is free software; you can redistribute it and/or modify
>  // it under the terms of the GNU General Public License as published by
> @@ -16,26 +16,29 @@
>  // along with this program; if not, write to the Free Software
>  // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>  
> -//
> -
>  // Test case for XML ActionScript class
>  // compile this test case with Ming makeswf, and then
>  // execute it like this gnash -1 -r 0 -v out.swf
>  
> -rcsid="$Id: XMLNode.as,v 1.8 2007/01/18 15:30:53 strk Exp $";
> +rcsid="$Id: XMLNode.as,v 1.9 2007/02/15 04:05:41 rsavoye Exp $";
>  
>  #include "dejagnu.as"
>  
> -check(XMLNode);
> -var textnode = new XMLNode(3, "text content");
> +var doc = new XML();
>  
> -check(textnode);
> +check(doc);
> +var textnode = doc.createTextNode("text content");
> +check_equals(typeOf(textnode), 'object');
>  
>  // test the XMLNode constuctor
>  //dejagnu(node, "XMLNode::XMLNode()");
>  
>  //note("Test the existance of all the methods");
>  
> +//check_equals(typeOf(myXML.createElement), 'function');
> +
> +
> +
>  check(textnode.appendChild);
>  check(textnode.cloneNode);
>  check(textnode.hasChildNodes);
> @@ -63,30 +66,47 @@
>  
>  //note("Now test the functionality of the methods");
>  
> -var childnode1 = new XMLNode(3, "first child");
> -check_equals(childnode1.nodeType, 3);
> -textnode.appendChild(childnode1);
> -
> -check_equals(textnode.hasChildNodes(), true);
> -check_equals(textnode.firstChild, childnode1);
> -check_equals(textnode.lastChild, childnode1);
> -check_equals(childnode1.nextSibling, undefined);
> -check_equals(childnode1.previousSibling, undefined);
> -
> -var nextnode = new XMLNode(3, "second child");
> -check_equals(nextnode.nodeType, 3);
> -textnode.appendChild(nextnode);
> -
> -check_equals(textnode.hasChildNodes(), true);
> -check_equals(textnode.firstChild, childnode1);
> -check_equals(textnode.lastChild, nextnode);
> -xcheck_equals(childnode1.nextSibling, nextnode);
> -check_equals(childnode1.previousSibling, undefined);
> -xcheck_equals(nextnode.previousSibling, childnode1);
>  
> -//var out = textnode.toString();
> -//trace(out);
> +var node1 = doc.createElement("node1");
> +var node2 = doc.createElement("node2");
> +var textnode1 = doc.createTextNode("first text node");
> +var textnode2 = doc.createTextNode("second text node");
> +check_equals(textnode1.nodeType, 3);
> +node1.appendChild(textnode1);
> +node2.appendChild(textnode2);
> +node1.appendChild(node2);
> +check_equals(node1.hasChildNodes(), true);
> +
> +check_equals(node1.firstChild.nodeValue, "second text node");
> +check_equals(node1.lastChild.nodeValue, "second text node");
> +xcheck_equals(node2.lastChild, "null"); // FIXME
> +
> +var node3 = doc.createElement("node3");
> +var textnode3 = doc.createTextNode("third text node");
> +node3.appendChild(textnode3);
> +node1.appendChild(node3);
> +
> +// trace(node1.toString());
> +trace("===========================================");
> +
> +// trace(node1.firstChild.nodeValue);
> +// trace(node1.lastChild.nodeValue);
> +check_equals(node1.firstChild.nodeValue, "second text node");
> +check_equals(node1.lastChild.nodeValue, "third text node");
> +
> +trace(node1.lastChild.previousSibling);
> +trace(node1.firstChild.nextSibling);
> +
> +check_equals(node1.firstChild.nodeName, "node2");
> +check_equals(node1.lastChild.nodeName, "node3");
> +
> +xcheck_equals(node2.previousSibling.nodeValue, "second text node");
>  
>  // TODO: test removeNode, insertNode
>  
> +// for (var aNode = node1.firstChild; node1 != null; aNode = 
> node1.nextSibling) {
> +//     trace(aNode);
> +// }
> +
> +
>  totals();
> 
> Index: ChangeLog
> ===================================================================
> RCS file: /sources/gnash/gnash/ChangeLog,v
> retrieving revision 1.2362
> retrieving revision 1.2363
> diff -u -b -r1.2362 -r1.2363
> --- ChangeLog 14 Feb 2007 22:48:55 -0000      1.2362
> +++ ChangeLog 15 Feb 2007 04:05:42 -0000      1.2363
> @@ -1,3 +1,10 @@
> +2007-02-14  Rob Savoye  <address@hidden>
> +
> +     * testsuite/actionscript.all/XMLNode.as: Fix tests for newly work
> +     XLNode class.
> +     * server/asobj/xmlnode.{cpp,h}: Make nextSibling, previousSibling,
> +     firstChild, and lastChild work correctly.
> +
>  2007-02-14 Sandro Santilli <address@hidden>
>  
>       * server/swf/tag_loaders.{cpp,h}: implement serialnumber_loader.
> @@ -79,6 +86,10 @@
>       
>  2007-02-13  Rob Savoye  <address@hidden>
>  
> +     * configure.ac: Disable dmalloc check for now. Configure
> +     extension/fileio directory.
> +     * extensions/Makefile.am: Build in the fileio directory.
> +     
>       * server/asobj/xmlnode.cpp: Implement toString(). Make appendChild
>       actually work.
>       * server/asobj/xml.cpp: Re-implement toString(). Make
> 
> 
> _______________________________________________
> Gnash-commit mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/gnash-commit

-- 

 ()   ASCII Ribbon Campaign
 /\   Keep it simple!