On Thu, 2009-07-30 at 12:12 +0100, John-Mark Bell wrote: > Precis: > > This is round 2 of Bo Yang's libdom core changes. > > > Added files
> Index: test/README > =================================================================== > --- /dev/null 2009-04-16 19:17:07.000000000 +0100 > +++ test/README 2009-07-30 11:27:55.000000000 +0100 > @@ -0,0 +1,2 @@ > +Note the lib/ directory contain the old test related files, although > they > +are not workable, I leave them here for later use if there is a > chance. You removed the lib directory, so this is redundant. > Index: include/dom/core/typeinfo.h > =================================================================== > --- /dev/null 2009-04-16 19:17:07.000000000 +0100 > +++ include/dom/core/typeinfo.h 2009-07-30 11:28:50.000000000 +0100 > @@ -0,0 +1,47 @@ > +/* > + * This file is part of libdom. > + * Licensed under the MIT License, > + * http://www.opensource.org/licenses/mit-license.php > + * Copyright 2009 Bo Yang <[email protected]> > + */ > + > +#ifndef dom_core_typeinfo_h_ > +#define dom_core_typeinfo_h_ > + > +#include <stdbool.h> > + > +#include <dom/core/exceptions.h> > + > +struct dom_string; > + > +typedef struct dom_type_info dom_type_info; > + > +typedef enum { > + DERIVATION_RESTRICTION = 0x00000001, > + DERIVATION_EXTENSION = 0x00000002, > + DERIVATION_UNION = 0x00000004, > + DERIVATION_LIST = 0x00000008 These need DOM_ at the front. Even better, would be DOM_TYPE_INFO > Index: src/utils/character_valid.h > =================================================================== > --- /dev/null 2009-04-16 19:17:07.000000000 +0100 > +++ src/utils/character_valid.h 2009-07-30 11:28:52.000000000 +0100 > @@ -0,0 +1,51 @@ > +/* > + * This file is part of libdom. > + * Licensed under the MIT License, > + * > http://www.opensource.org/licenses/mit-license.php > + * Copyright 2009 Bo Yang <[email protected]> > + * > + * This file contains the API used to validate whether certain > character in > + * name/value is legal according the XML 1.0 standard. See > + * > + * http://www.w3.org/TR/2004/REC-xml-20040204/ > + * http://www.w3.org/TR/REC-xml/ > + * > + * for detail. > + */ > + > +#ifndef dom_utils_character_valid_h_ > +#define dom_utils_character_valid_h_ > + > +#include <stdbool.h> > + > +struct xml_char_range { > + unsigned int start; > + unsigned int end; > +}; > + > +struct xml_char_group { > + int len; This should be a size_t. > Index: src/utils/hashtable.h > =================================================================== > --- /dev/null 2009-04-16 19:17:07.000000000 +0100 > +++ src/utils/hashtable.h 2009-07-30 11:28:52.000000000 +0100 > @@ -0,0 +1,40 @@ > +/* > + * This file is part of libdom. > + * Licensed under the MIT License, > + * http://www.opensource.org/licenses/mit-license.php > + * Copyright 2006 Rob Kendrick <[email protected]> > + * Copyright 2009 Bo Yang <[email protected]> > + */ > + > +#ifndef dom_utils_hashtable_h_ > +#define dom_utils_hashtable_h_ > + > +#include <stdbool.h> > +#include <dom/functypes.h> > + > +typedef struct hash_table hash_table; > +/* The hash function */ > +typedef unsigned int (*hash_func)(void *key); > +/* Function to clone/delete key */ > +typedef void *(*key_func)(void *key, void *pw, dom_alloc alloc, void > *alloc_pw, > + bool clone); > +/* Function to clone/delete value */ > +typedef void *(*value_func)(void *value, void *pw, dom_alloc alloc, > + void *alloc_pw, bool clone); > + > +struct hash_table *hash_create(unsigned int chains, hash_func hash, > + dom_alloc alloc, void *ptr); > +struct hash_table *hash_clone(struct hash_table *ht, dom_alloc alloc, > + void *pw, key_func kf, void *key_pw, value_func vf, void > *value_pw); > +void hash_destroy(struct hash_table *ht, key_func kf, void *key_pw, > + value_func vf, void *value_pw); > +bool hash_add(struct hash_table *ht, void *key, void *value, bool > replace); > +void *hash_get(struct hash_table *ht, void *key); > +void *hash_del(struct hash_table *ht, void *key); > +void *hash_iterate(struct hash_table *ht, unsigned int *c1, > + unsigned int **c2); > +unsigned int hash_get_length(struct hash_table *ht); > +unsigned int hash_get_chains(struct hash_table *ht); > +hash_func hash_get_func(struct hash_table *ht); These all need _dom_. > Index: src/utils/resource_mgr.h > =================================================================== > --- /dev/null 2009-04-16 19:17:07.000000000 +0100 > +++ src/utils/resource_mgr.h 2009-07-30 11:28:52.000000000 +0100 > @@ -0,0 +1,40 @@ > +/* > + * This file is part of libdom. > + * Licensed under the MIT License, > + * > http://www.opensource.org/licenses/mit-license.php > + * Copyright 2009 Bo Yang <[email protected]> > + */ > + > +#ifndef dom_utils_resource_mgr_h_ > +#define dom_utils_resource_mgr_h_ > + > +#include <dom/functypes.h> > +#include <dom/core/exceptions.h> > + > +#include "hashtable.h" > + > +struct lwc_context_s; > +struct lwc_string_s; > +struct dom_string; > + > +typedef struct resource_mgr { > + dom_alloc alloc; > + void *pw; > + struct lwc_context_s *ctx; > +} resource_mgr; > + > +void *resource_mgr_alloc(struct resource_mgr *res, void *ptr, size_t > size); > + > +dom_exception resource_mgr_create_string(struct resource_mgr *res, > + const uint8_t *data, size_t len, struct dom_string **result); > + > +dom_exception resource_mgr_create_lwcstring(struct resource_mgr *res, > + const uint8_t *data, size_t len, struct lwc_string_s **result); > + > +dom_exception resource_mgr_create_string_from_lwcstring(struct > resource_mgr *res, > + struct lwc_string_s *str, struct dom_string **result); > + > +dom_exception resource_mgr_create_hashtable(struct resource_mgr *res, > + size_t chains, hash_func f, struct hash_table **ht); _dom_. > Index: src/core/string.h > =================================================================== > --- /dev/null 2009-04-16 19:17:07.000000000 +0100 > +++ src/core/string.h 2009-07-30 11:28:53.000000000 +0100 > @@ -0,0 +1,28 @@ > +/* > + * This file is part of libdom. > + * Licensed under the MIT License, > + * http://www.opensource.org/licenses/mit-license.php > + * Copyright 2009 Bo Yang <[email protected]> > + */ > + > +#ifndef dom_internal_core_string_h_ > +#define dom_internal_core_string_h_ > + > +#include <dom/core/string.h> > + > +/* Create a DOM string from a lwc_string > + * This function call mainly used for create a string from lwc_string */ > +dom_exception dom_string_create_from_lwcstring(dom_alloc alloc, void *pw, > + struct lwc_context_s *ctx, struct lwc_string_s *str, > + struct dom_string **ret); > + > +/* Make the dom_string be interned in the lwc_context_s */ > +dom_exception dom_string_intern(struct dom_string *str, > + struct lwc_context_s *ctx, struct lwc_string_s **lwcstr); > + > +/* Compare the raw data of two lwc_strings for equality when the two string > + * belong to different lwc_context */ > +int dom_lwc_string_compare_raw(struct lwc_string_s *s1, struct lwc_string_s > *s2); > + These all want a '_' at the start. > Index: src/bootstrap/implementation.h > =================================================================== > --- /dev/null 2009-04-16 19:17:07.000000000 +0100 > +++ src/bootstrap/implementation.h 2009-07-30 11:29:00.000000000 +0100 > @@ -0,0 +1,14 @@ > +/* > + * This file is part of libdom. > + * Licensed under the MIT License, > + * http://www.opensource.org/licenses/mit-license.php > + * Copyright 2009 Bo Yang <[email protected]> > + */ > + > +#ifndef dom_bootstrap_implementation_h_ > +#define dom_bootstrap_implementation_h_ > + > +dom_exception dom_implementation_initialise(dom_alloc alloc, void *pw); > +void dom_implementation_finalise(void); Ditto. > Index: include/dom/core/string.h > =================================================================== > --- include/dom/core/string.h (revision 8909) > +++ include/dom/core/string.h (working copy) > @@ -10,6 +10,7 @@ > > #include <inttypes.h> > #include <stddef.h> > +#include <libwapcaplet/libwapcaplet.h> > > #include <dom/functypes.h> > #include <dom/core/exceptions.h> > @@ -25,6 +26,16 @@ > dom_exception dom_string_create(dom_alloc alloc, void *pw, > const uint8_t *ptr, size_t len, struct dom_string **str); > > +/* Clone a dom_string */ > +dom_exception dom_string_clone(dom_alloc alloc, void *pw, struct dom_string > *str, > + struct dom_string **ret); > + > +/* Get the internal lwc_string */ > +dom_exception dom_string_get_intern(struct dom_string *str, > + struct lwc_context_s **ctx, struct lwc_string_s **lwcstr); > +/* Map the lwc_error to dom_exception */ > +dom_exception dom_exception_from_lwc_error(lwc_error err); > + Does this really belong here? > Index: include/dom/core/nodelist.h > =================================================================== > --- include/dom/core/nodelist.h (revision 8909) > +++ include/dom/core/nodelist.h (working copy) > @@ -25,4 +25,8 @@ > #define dom_nodelist_item(l, i, n) _dom_nodelist_item((dom_nodelist *) (l), \ > (unsigned long) (i), (dom_node **) (n)) > > +bool _dom_nodelist_equal(struct dom_nodelist *l1, struct dom_nodelist *l2); > +#define dom_nodelist_equal(l1, l2) _dom_nodelist_equal( \ > + (struct dom_nodelist *) (l1), (struct dom_nodelist *) (l2)) > + Shouldn't this be internal to the library? > Index: src/utils/namespace.c > =================================================================== > --- src/utils/namespace.c (revision 8909) > +++ src/utils/namespace.c (working copy) > @@ -1,8 +1,9 @@ > /* > * This file is part of libdom. > * Licensed under the MIT License, > - * http://www.opensource.org/licenses/mit-license.php > + * > http://www.opensource.org/licenses/mit-license.php > * Copyright 2007 John-Mark Bell <[email protected]> > + * Copyright 2009 Bo Yang <[email protected]> > */ > > #include <string.h> > @@ -10,6 +11,7 @@ > #include <dom/dom.h> > > #include "utils/namespace.h" > +#include "utils/validate.h" > #include "utils/utils.h" > > > @@ -37,7 +39,7 @@ > * Initialise the namespace component > * > * \param alloc Pointer to memory (de)allocation function > - * \param pw Pointer to client-specific private data > + * \param pw Pointer to client-specific private data > * \return DOM_NO_ERR on success. > */ > dom_exception _dom_namespace_initialise(dom_alloc alloc, void *pw) > @@ -110,31 +112,40 @@ > /** > * Ensure a QName is valid > * > - * \param qname The qname to validate > + * \param qname The qname to validate > * \param namespace The namespace URI associated with the QName, or NULL > - * \return DOM_NO_ERR if valid, > - * DOM_INVALID_CHARACTER_ERR if ::qname contains an invalid > character, > - * DOM_NAMESPACE_ERR if ::qname is malformed, or it has a > - * prefix and ::namespace is NULL, or > - * ::qname has a prefix "xml" and > - * ::namespace is not > - * "http://www.w3.org/XML/1998/namespace";, > - * or ::qname has a prefix "xmlns" and > - * ::namespace is not > - * "http://www.w3.org/2000/xmlns";, or > - * ::namespace is > - * "http://www.w3.org/2000/xmlns"; and > - * ::qname is not (or is not prefixed by) > - * "xmlns". > + * \return DOM_NO_ERR if valid, > + * DOM_INVALID_CHARACTER_ERR if ::qname contains an invalid > character, > + * DOM_NAMESPACE_ERR if ::qname is malformed, or it > has a > + * prefix and > ::namespace is NULL, or > + * ::qname has > a prefix "xml" and > + * ::namespace > is not > + * > "http://www.w3.org/XML/1998/namespace";, > + * or ::qname > has a prefix "xmlns" and > + * ::namespace > is not > + * > "http://www.w3.org/2000/xmlns";, or > + * ::namespace > is > + * > "http://www.w3.org/2000/xmlns"; and > + * ::qname is > not (or is not prefixed by) > + * "xmlns". > */ The above whitespace changes look wrong. > @@ -195,8 +221,8 @@ > /** > * Split a QName into a namespace prefix and localname string > * > - * \param qname The qname to split > - * \param prefix Pointer to location to receive prefix > + * \param qname The qname to split > + * \param prefix Pointer to location to receive prefix > * \param localname Pointer to location to receive localname > * \return DOM_NO_ERR on success. > * Ditto. > Index: src/core/comment.c > =================================================================== > --- src/core/comment.c (revision 8909) > +++ src/core/comment.c (working copy) > @@ -3,19 +3,26 @@ > * Licensed under the MIT License, > * http://www.opensource.org/licenses/mit-license.php > * Copyright 2007 John-Mark Bell <[email protected]> > + * Copyright 2009 Bo Yang <[email protected]> > */ > > #include "core/characterdata.h" > #include "core/comment.h" > #include "core/document.h" > > +#include "utils/utils.h" > + > /** > - * A DOM comment node > + * A DOM Comment node > */ > struct dom_comment { > struct dom_characterdata base; /**< Base node */ > }; > > +static struct dom_node_protect_vtable comment_protect_vtable = { > + DOM_COMMENT_PROTECT_VTABLE > +}; > + > /** > * Create a comment node > * > @@ -31,7 +38,7 @@ > * The returned node will already be referenced. > */ > dom_exception dom_comment_create(struct dom_document *doc, > - struct dom_string *name, struct dom_string *value, > + struct lwc_string_s *name, struct dom_string *value, > struct dom_comment **result) > { > struct dom_comment *c; > @@ -42,6 +49,10 @@ > if (c == NULL) > return DOM_NO_MEM_ERR; > > + /* Set the virtual table */ > + ((dom_node_internal *) c)->base.vtable = &characterdata_vtable; > + ((dom_node_internal *) c)->vtable = &comment_protect_vtable; > + > /* And initialise the node */ > err = dom_characterdata_initialise(&c->base, doc, DOM_COMMENT_NODE, > name, value); > @@ -72,3 +83,36 @@ > /* Free node */ > dom_document_alloc(doc, comment, 0); > } > + > + > +/* The protected virtual functions */ > +void _dom_comment_destroy(struct dom_node_internal *node) > +{ > + struct dom_document *doc; > + doc = dom_node_get_owner(node); > + > + dom_comment_destroy(doc, (struct dom_comment *) node); > +} > + > +dom_exception _dom_comment_alloc(struct dom_document *doc, > + struct dom_node_internal *n, struct dom_node_internal **ret) > +{ > + UNUSED(n); > + dom_comment *c; > + > + c = dom_document_alloc(doc, NULL, sizeof(struct dom_comment)); > + if (c == NULL) > + return DOM_NO_MEM_ERR; > + > + *ret = (dom_node_internal *) c; > + dom_node_set_owner(*ret, doc); > + > + return DOM_NO_ERR; > +} > + > +dom_exception _dom_comment_copy(struct dom_node_internal *new, > + struct dom_node_internal *old) > +{ > + return _dom_characterdata_copy(new, old); > +} These all need doxygen comments. > Index: src/core/string.c > =================================================================== > --- src/core/string.c (revision 8909) > +++ src/core/string.c (working copy) > @@ -121,7 +138,146 @@ > return DOM_NO_ERR; > } > > +/* Clone a dom_string if necessary. This method is used to create a new > string /** * Clone... > +/* Compare the raw data of two lwc_strings for equality when the two string Ditto. > + * belong to different lwc_context > + * > + * \param s1 The first lwc_string > + * \param s2 The second lwc_string > + * \return 0 for equal, non-zeor otherwise Whitespace. "zero". > Index: src/core/text.c > =================================================================== > --- src/core/text.c (revision 8909) > +++ src/core/text.c (working copy) > @@ -3,18 +3,22 @@ > * Licensed under the MIT License, > * http://www.opensource.org/licenses/mit-license.php > * Copyright 2007 John-Mark Bell <[email protected]> > + * Copyright 2009 Bo Yang <[email protected]> > */ > +#include <assert.h> > > #include <dom/core/string.h> > #include <dom/core/text.h> > > +#include <libwapcaplet/libwapcaplet.h> > + > #include "core/characterdata.h" > #include "core/document.h" > #include "core/text.h" > #include "utils/utils.h" > > /* The virtual table for dom_text */ > -static struct dom_text_vtable text_vtable = { > +struct dom_text_vtable text_vtable = { > { > { > DOM_NODE_VTABLE > @@ -24,16 +28,33 @@ > DOM_TEXT_VTABLE > }; > > -/* The destroy virtual function */ > -void _dom_text_destroy(struct dom_node_internal *node); > -void _dom_text_destroy(struct dom_node_internal *node) > -{ > - struct dom_document *doc; > - dom_node_get_owner_document(node, &doc); > +static struct dom_node_protect_vtable text_protect_vtable = { > + DOM_TEXT_PROTECT_VTABLE > +}; > > - dom_text_destroy(doc, (struct dom_text *) node); > -} > +/* Following comes helper functions */ > +typedef enum walk_operation { > + COLLECT, > + DELETE > +} walk_operation; > +typedef enum walk_order { > + LEFT, > + RIGHT > +} walk_order; > > +/* Walk the logic-adjacent text in document order */ > +static dom_exception walk_logic_adjacent_text_in_order( > + dom_node_internal *node, walk_operation opt, > + walk_order order, dom_string **ret, bool *cont); > +/* Walk the logic-adjacent text */ > +static dom_exception walk_logic_adjacent_text(dom_text *text, > + walk_operation opt, dom_string **ret); > + > +/* > */ > +/*----------------------------------------------------------------------*/ > +/* > */ What is the purpose of these empty comments? > +/* Constructor and Destructor */ > + > /** > * Create a text node > * > @@ -49,7 +70,7 @@ > * The returned node will already be referenced. > */ > dom_exception dom_text_create(struct dom_document *doc, > - struct dom_string *name, struct dom_string *value, > + struct lwc_string_s *name, struct dom_string *value, > struct dom_text **result) > { > struct dom_text *t; > @@ -67,6 +88,10 @@ > return err; > } > > + /* Compose the vtable */ > + ((struct dom_node *) t)->vtable = &text_vtable; > + ((struct dom_node_internal *) t)->vtable = &text_protect_vtable; > + > *result = t; > > return DOM_NO_ERR; > @@ -103,7 +128,7 @@ > */ > dom_exception dom_text_initialise(struct dom_text *text, > struct dom_document *doc, dom_node_type type, > - struct dom_string *name, struct dom_string *value) > + struct lwc_string_s *name, struct dom_string *value) > { > dom_exception err; > > @@ -113,10 +138,6 @@ > if (err != DOM_NO_ERR) > return err; > > - /* Compose the vtable */ > - ((struct dom_node *) text)->vtable = &text_vtable; > - text->base.base.destroy = &_dom_text_destroy; > - > /* Perform our type-specific initialisation */ > text->element_content_whitespace = false; > > @@ -136,6 +157,11 @@ > dom_characterdata_finalise(doc, &text->base); > } > > +/* > */ > +/*----------------------------------------------------------------------*/ > +/* > */ Ditto. > +/* > */ > +/*----------------------------------------------------------------------*/ > +/* > */ Ditto. > +/* Helper functions */ > + > +/** > + * Walk the logic adjacent text in certain order > + * > + * \param node The start Text node > + * \param opt The operation on each Text Node > + * \param order The order Whitespace. > +/** > + * Traverse the logic adjacent text. > + * > + * \param text The Text Node from which we start traversal Ditto. > Index: src/core/characterdata.c > =================================================================== > --- src/core/characterdata.c (revision 8909) > +++ src/core/characterdata.c (working copy) > @@ -72,6 +78,11 @@ > dom_node_finalise(doc, &cdata->base); > } > > +/* > */ > +/*----------------------------------------------------------------------*/ > +/* > */ More insanely wide empty comments. > @@ -373,3 +382,36 @@ > return DOM_NO_ERR; > } > > + > +/* > */ > +/*----------------------------------------------------------------------*/ > +/* > */ Ditto. > Index: src/core/element.c > =================================================================== > --- src/core/element.c (revision 8909) > +++ src/core/element.c (working copy) > @@ -3,31 +3,130 @@ > * Licensed under the MIT License, > * http://www.opensource.org/licenses/mit-license.php > * Copyright 2007 John-Mark Bell <[email protected]> > + * Copyright 2009 Bo Yang <[email protected]> > */ > > #include <stdio.h> > +#include <string.h> > +#include <assert.h> > > +#include <dom/dom.h> > #include <dom/core/attr.h> > #include <dom/core/element.h> > #include <dom/core/node.h> > #include <dom/core/string.h> > +#include <dom/core/document.h> > > #include "core/attr.h" > #include "core/document.h" > #include "core/element.h" > #include "core/node.h" > +#include "core/namednodemap.h" > +#include "utils/validate.h" > #include "utils/namespace.h" > #include "utils/utils.h" > +#include "utils/hashtable.h" > > +/* The two numbers are just random ones, maybe we should change it after some > + * more consideration */ > +#define CHAINS_ATTRIBUTES 31 > +#define CHAINS_NAMESPACE 7 > +#define CHAINS_NS_ATTRIBUTES 31 > + > +/* TODO: Make the "id" definition temporarily here, in future, it should > + * be obtained from the language bindings. > + */ > +#define ID_ATTRIBUTE "id" I'd like to see this fixed before merge. > +static unsigned int hash_lwcstring(void *key); _dom_element? Alternatively, move this to the hashtable implementation. > +/* > */ > +/*----------------------------------------------------------------------*/ > +/* > */ More empty comments. > +/* > */ > +/*----------------------------------------------------------------------*/ > +/* > */ Ditto. > +/* > */ > +/*----------------------------------------------------------------------*/ > +/* > */ Oh look, another one. > +/* TODO: How to deal with default attribue: > + * > + * Ask a language binding for default attributes. > + * > + * So, when we copy a element we copy all its attributes because they > + * are all specified. For the methods like importNode and adoptNode, > + * this will make _dom_element_copy can be used in them. > + */ > +dom_exception _dom_element_copy(struct dom_node_internal *new, > + struct dom_node_internal *old) > +{ > + dom_element *ne = (dom_element *) new; > + dom_element *oe = (dom_element *) old; > + dom_document *od, *nd; > + struct hash_table *ht, *value; > + lwc_string *key; > + lwc_context *oc, *nc; > + unsigned int c1, *c2 = NULL; > + dom_exception err; > + > + err = _dom_node_copy(new, old); > + if (err != DOM_NO_ERR) > + return err; > + > + od = dom_node_get_owner(old); > + nd = dom_node_get_owner(new); > + assert(od != NULL); > + assert(nd != NULL); > + > + oc = dom_document_get_intern_context(od); > + nc = dom_document_get_intern_context(nd); > + assert(oc != NULL); > + assert(nc != NULL); > + > + /* copy the hash tables */ > + err = attr_clone(oe->attributes, od, &ht, nd, new); > + if (err != DOM_NO_ERR) > + return err; > + ne->attributes = ht; > + > + err = dom_document_create_hashtable(nd, > hash_get_chains(oe->ns_attributes), > + hash_get_func(oe->ns_attributes), &ht); > + if (err != DOM_NO_ERR) > + return err; > + ne->ns_attributes = ht; > + > + while ( (key = (lwc_string *) hash_iterate(oe->ns_attributes, &c1, &c2) > + ) != NULL) { > + value = (struct hash_table *) hash_get(oe->ns_attributes, key); > + if (ht != NULL) { > + lwc_error lerr; > + lwc_string *nk; > + > + if (oc != nc) { > + lerr = lwc_context_intern(nc, > lwc_string_data(key), > + lwc_string_length(key), &nk); > + if (lerr != lwc_error_ok) { > + hash_destroy(ht, _key, nc, > _nsattributes, nc); > + return > dom_exception_from_lwc_error(lerr); > + } > + } else { > + lwc_context_string_ref(oc, key); > + nk = key; > + } > + > + struct hash_table *h; > + err = attr_clone(value, od, &h, nd, new); > + if (err != DOM_NO_ERR) { > + lwc_context_string_unref(nc, nk); > + hash_destroy(ht, _key, nc, _nsattributes, nc); > + return err; > + } > + if (hash_add(ht, nk, h, false) == false) { > + lwc_context_string_unref(nc, nk); > + hash_destroy(ht, _key, nc, _nsattributes, nc); > + return DOM_NO_MEM_ERR; > + } > + } > + } The above looks suspiciously like hash_clone. It should call that, instead. > +/** > + * Implementation function for NamedNodeMap, see core/namednodemap.h for > details > + */ > +bool attributes_equal(void *p1, void *p2) > +{ > + return p1 == p2; Is this comparing lwc_strings? If so, it should use lwc_context_string_isequal(ctx, p1, p2); It would appear that you've no way of retrieving the internment context here. That seems like a bug. > +/** > + * Clone all the element's attributes > + * > + * \param sh The source attributes hash table > + * \param sd The source document > + * \param th The target hash table > + * \param td The target document > + * \param parent The parent of the cloned Node > + */ > +dom_exception attr_clone(struct hash_table *sh, struct dom_document *sd, > + struct hash_table **th, struct dom_document *td, > dom_node_internal *parent) > +{ > + lwc_string *key; > + lwc_context *sc, *tc; > + dom_attr *attr; > + unsigned int c1, *c2 = NULL; > + dom_exception err; > + > + err = dom_document_create_hashtable(td, hash_get_chains(sh), > + hash_get_func(sh), th); > + if (err != DOM_NO_ERR) > + return err; > + > + sc = dom_document_get_intern_context(sd); > + tc = dom_document_get_intern_context(td); > + assert(sc != NULL); > + assert(tc != NULL); > + > + while ( (key = (lwc_string *) hash_iterate(sh, &c1, &c2)) != NULL) { > + attr = (dom_attr *) hash_get(sh, key); > + if (attr != NULL) { > + lwc_string *nk; > + lwc_error lerr; > + dom_attr *na; > + > + /* we should copy this entry */ > + if (sc != tc) { > + lerr = lwc_context_intern(tc, > lwc_string_data(key), > + lwc_string_length(key), &nk); > + if (lerr != lwc_error_ok) { > + hash_destroy(*th, _key, tc, _value, tc); > + return > dom_exception_from_lwc_error(lerr); > + } > + } else { > + nk = key; > + lwc_context_string_ref(sc, nk); > + } > + > + err = dom_document_import_node(td, attr, true, (void *) > &na); > + if (err != DOM_NO_ERR) > + return err; > + > + dom_node_set_parent(na, parent); > + > + if (hash_add(*th, nk, na, false) == false) { > + hash_destroy(*th, _key, tc, _value, tc); > + lwc_context_string_unref(tc, nk); > + dom_node_unref(na); > + return DOM_NO_MEM_ERR; > + } > + } > + } > + > + return DOM_NO_ERR; > +} This is just hash_clone. Remove it. > +/** > + * The key_func of the hash table, see utils/hashtable.h for details > + */ > +void *_key(void *key, void *key_pw, dom_alloc alloc, void *pw, > + bool clone) > +{ > + assert(key != NULL); > + assert(key_pw != NULL); > + > + UNUSED(alloc); > + UNUSED(pw); > + > + if (clone == false) { > + lwc_context_string_unref((lwc_context *) key_pw, (lwc_string *) > key); > + return NULL; > + } else { > + /* TODO: If we want to change tha attr_clone function, we should > + * fill this part > + */ Please do. > + return NULL; > + } > +} > + > +/** > + * The value_func of the hash table, see utils/hashtable.h for details > + */ > +void *_value(void *value, void *value_pw, dom_alloc alloc, > + void *pw, bool clone) > +{ > + assert(value != NULL); > + assert(value_pw != NULL); > + This fails to compile because value_pw is not used. > + UNUSED(alloc); > + UNUSED(pw); > + > + if (clone == false) { > + dom_node_internal *a = (dom_node_internal *) value; > + a->parent = NULL; > + dom_node_try_destroy(a); > + return NULL; > + } else { > + /* TODO: If we want to change tha attr_clone function, we should > + * fill this part > + */ Please do. > + return NULL; > + } > +} > + > +/** > + * The value_func of the hash table, see utils/hashtable.h for details > + */ > +void *_nsattributes(void *value, void *value_pw, dom_alloc alloc, > + void *pw, bool clone) > +{ > + assert(value != NULL); > + assert(value_pw != NULL); > + > + UNUSED(alloc); > + UNUSED(pw); > + > + if (clone == false) { > + hash_destroy((struct hash_table *) value, _key, value_pw, > + _value, value_pw); > + return NULL; > + } else { > + /* TODO: If we want to change tha attr_clone function, we should > + * fill this part > + */ Ditto. > Index: src/core/cdatasection.c > =================================================================== > --- src/core/cdatasection.c (revision 8909) > +++ src/core/cdatasection.c (working copy) > +/* > */ > +/*--------------------------------------------------------------------------*/ > +/* > */ You're making me cry with these. > Index: src/core/attr.c > =================================================================== > --- src/core/attr.c (revision 8909) > +++ src/core/attr.c (working copy) > @@ -3,10 +3,12 @@ > * Licensed under the MIT License, > * http://www.opensource.org/licenses/mit-license.php > * Copyright 2007 John-Mark Bell <[email protected]> > + * Copyright 2009 Bo Yang <[email protected]> > */ > > #include <stddef.h> > #include <string.h> > +#include <assert.h> > > #include <dom/core/attr.h> > #include <dom/core/document.h> > @@ -27,22 +29,32 @@ > struct dom_attr { > struct dom_node_internal base; /**< Base node */ > > - bool specified; /**< Whether attribute was specified > - * or defaulted */ > + bool specified; /**< Whether the attribute is specified > + * or default */ Whitespace! > +/* > */ > +/* -------------------------------------------------------------------- */ > +/* > */ ... > +/* > */ > +/* -------------------------------------------------------------------- */ > +/* > */ Ok. I'm giving up with these. Please fix them all. > Index: src/core/entity_ref.h > =================================================================== > --- src/core/entity_ref.h (revision 8909) > +++ src/core/entity_ref.h (working copy) > @@ -9,19 +9,38 @@ > #define dom_internal_core_entityreference_h_ > > #include <dom/core/exceptions.h> > +#include <dom/core/entity_ref.h> > > struct dom_document; > struct dom_entity_reference; > struct dom_string; > +struct lwc_string_s; > > dom_exception dom_entity_reference_create(struct dom_document *doc, > - struct dom_string *name, struct dom_string *value, > + struct lwc_string_s *name, struct dom_string *value, > struct dom_entity_reference **result); > > void dom_entity_reference_destroy(struct dom_document *doc, > struct dom_entity_reference *entity); > > +#define dom_entity_reference_initialise dom_node_initialise > +#define dom_entity_reference_finalise dom_node_finalise > + > +/* Following comes the protected vtable */ > +void _dom_ef_destroy(struct dom_node_internal *node); > +dom_exception _dom_ef_alloc(struct dom_document *doc, > + struct dom_node_internal *n, struct dom_node_internal **ret); > +dom_exception _dom_ef_copy(struct dom_node_internal *new, > + struct dom_node_internal *old); > + > +#define DOM_EF_PROTECT_VTABLE \ > + _dom_ef_destroy, \ > + _dom_ef_alloc, \ > + _dom_ef_copy What is "EF"? > Index: src/core/document.c > =================================================================== > --- src/core/document.c (revision 8909) > +++ src/core/document.c (working copy) > @@ -3,16 +3,23 @@ > * Licensed under the MIT License, > * http://www.opensource.org/licenses/mit-license.php > * Copyright 2007 John-Mark Bell <[email protected]> > + * Copyright 2009 Bo Yang <[email protected]> > */ > > +#include <assert.h> > + > #include <string.h> > > +#include <libwapcaplet/libwapcaplet.h> > + > #include <dom/functypes.h> > #include <dom/bootstrap/implpriv.h> > +#include <dom/core/attr.h> > +#include <dom/core/element.h> > #include <dom/core/document.h> > #include <dom/core/implementation.h> > -#include <dom/core/string.h> > > +#include "core/string.h" > #include "core/attr.h" > #include "core/cdatasection.h" > #include "core/comment.h" > @@ -24,6 +31,7 @@ > #include "core/nodelist.h" > #include "core/pi.h" > #include "core/text.h" > +#include "utils/validate.h" > #include "utils/namespace.h" > #include "utils/utils.h" > > @@ -37,19 +45,9 @@ > struct dom_doc_nl *prev; /**< Previous item */ > }; > > -/** > - * Item in list of active namednodemaps > - */ > -struct dom_doc_nnm { > - struct dom_namednodemap *map; /**< Named node map */ > > - struct dom_doc_nnm *next; /**< Next map */ > - struct dom_doc_nnm *prev; /**< Previous map */ > -}; > - > - > -/** Interned node name strings, indexed by node type */ > -/* Index 0 is unused */ > +/** Node name strings, indexed by node type > + * Index 0 is unused */ > static struct dom_string *__nodenames_utf8[DOM_NODE_TYPE_COUNT + 1]; Is this actually needed any more? The rest looks fine (excepting the comments I've stopped pointing out). J.
