[PATCH v3 1/6] xmllite: Use empty_element to store various names.

Jeff Smith whydoubt at gmail.com
Tue Oct 29 14:35:59 CDT 2019


Rather than maintain global state for prefix, local name, and qualified
name, always determine the value based on node type.  For the xml
declaration, DTD, and processing instruction nodes it is reasonable to
place those into empty_element fields.

Signed-off-by: Jeff Smith <whydoubt at gmail.com>
---
 dlls/xmllite/reader.c       | 171 +++++++++++++++---------------------
 dlls/xmllite/tests/reader.c |   5 +-
 2 files changed, 72 insertions(+), 104 deletions(-)

diff --git a/dlls/xmllite/reader.c b/dlls/xmllite/reader.c
index e813ca37fe..dd033f37c5 100644
--- a/dlls/xmllite/reader.c
+++ b/dlls/xmllite/reader.c
@@ -74,15 +74,6 @@ typedef enum
     XmlReadResume_Last
 } XmlReaderResume;
 
-typedef enum
-{
-    StringValue_LocalName,
-    StringValue_Prefix,
-    StringValue_QualifiedName,
-    StringValue_Value,
-    StringValue_Last
-} XmlReaderStringValue;
-
 static const WCHAR usasciiW[] = {'U','S','-','A','S','C','I','I',0};
 static const WCHAR utf16W[] = {'U','T','F','-','1','6',0};
 static const WCHAR utf8W[] = {'U','T','F','-','8',0};
@@ -289,12 +280,12 @@ typedef struct
     struct list ns;
     struct list elements;
     int chunk_read_off;
-    strval strvalues[StringValue_Last];
+    strval strvalue;
     UINT depth;
     UINT max_depth;
     BOOL is_empty_element;
     struct element empty_element; /* used for empty elements without end tag <a />,
-                                     and to keep <?xml reader position */
+                                     xml declaration, DTD, and processing instructions */
     UINT resume[XmlReadResume_Last]; /* offsets used to resume reader */
 } xmlreader;
 
@@ -484,18 +475,6 @@ static inline void reader_init_cstrvalue(WCHAR *str, UINT len, strval *v)
     v->str = str;
 }
 
-static void reader_free_strvalue(xmlreader *reader, XmlReaderStringValue type)
-{
-    reader_free_strvalued(reader, &reader->strvalues[type]);
-}
-
-static void reader_free_strvalues(xmlreader *reader)
-{
-    int type;
-    for (type = 0; type < StringValue_Last; type++)
-        reader_free_strvalue(reader, type);
-}
-
 /* This helper should only be used to test if strings are the same,
    it doesn't try to sort. */
 static inline int strval_eq(const xmlreader *reader, const strval *str1, const strval *str2)
@@ -515,6 +494,7 @@ static void reader_clear_elements(xmlreader *reader)
         reader_free(reader, elem);
     }
     list_init(&reader->elements);
+    reader_free_strvalued(reader, &reader->empty_element.prefix);
     reader_free_strvalued(reader, &reader->empty_element.localname);
     reader_free_strvalued(reader, &reader->empty_element.qname);
     reader->is_empty_element = FALSE;
@@ -669,37 +649,27 @@ static void reader_pop_element(xmlreader *reader)
 
 /* Always make a copy, cause strings are supposed to be null terminated. Null pointer for 'value'
    means node value is to be determined. */
-static void reader_set_strvalue(xmlreader *reader, XmlReaderStringValue type, const strval *value)
+static void reader_set_strvalue(xmlreader *reader, const strval *value)
 {
-    strval *v = &reader->strvalues[type];
+    strval *v = &reader->strvalue;
 
-    reader_free_strvalue(reader, type);
+    reader_free_strvalued(reader, v);
     if (!value)
     {
         v->str = NULL;
         v->start = 0;
         v->len = 0;
-        return;
     }
-
-    if (value->str == strval_empty.str)
+    else if (value->str == strval_empty.str)
+    {
         *v = *value;
+    }
     else
     {
-        if (type == StringValue_Value)
-        {
-            /* defer allocation for value string */
-            v->str = NULL;
-            v->start = value->start;
-            v->len = value->len;
-        }
-        else
-        {
-            v->str = reader_alloc(reader, (value->len + 1)*sizeof(WCHAR));
-            memcpy(v->str, reader_get_strptr(reader, value), value->len*sizeof(WCHAR));
-            v->str[value->len] = 0;
-            v->len = value->len;
-        }
+        /* defer allocation for value string */
+        v->str = NULL;
+        v->start = value->start;
+        v->len = value->len;
     }
 }
 
@@ -1414,8 +1384,8 @@ static HRESULT reader_parse_xmldecl(xmlreader *reader)
 
     reader->nodetype = XmlNodeType_XmlDeclaration;
     reader->empty_element.position = position;
-    reader_set_strvalue(reader, StringValue_LocalName, &strval_xml);
-    reader_set_strvalue(reader, StringValue_QualifiedName, &strval_xml);
+    reader_strvaldup(reader, &strval_xml, &reader->empty_element.localname);
+    reader_strvaldup(reader, &strval_xml, &reader->empty_element.qname);
 
     return S_OK;
 }
@@ -1441,7 +1411,7 @@ static HRESULT reader_parse_comment(xmlreader *reader)
         reader->nodetype = XmlNodeType_Comment;
         reader->resume[XmlReadResume_Body] = start;
         reader->resumestate = XmlReadResumeState_Comment;
-        reader_set_strvalue(reader, StringValue_Value, NULL);
+        reader_set_strvalue(reader, NULL);
     }
 
     /* will exit when there's no more data, it won't attempt to
@@ -1462,7 +1432,7 @@ static HRESULT reader_parse_comment(xmlreader *reader)
                     /* skip rest of markup '->' */
                     reader_skipn(reader, 3);
 
-                    reader_set_strvalue(reader, StringValue_Value, &value);
+                    reader_set_strvalue(reader, &value);
                     reader->resume[XmlReadResume_Body] = 0;
                     reader->resumestate = XmlReadResumeState_Initial;
                     return S_OK;
@@ -1648,9 +1618,11 @@ static HRESULT reader_parse_pi(xmlreader *reader)
     case XmlReadResumeState_PITarget:
         hr = reader_parse_pitarget(reader, &target);
         if (FAILED(hr)) return hr;
-        reader_set_strvalue(reader, StringValue_LocalName, &target);
-        reader_set_strvalue(reader, StringValue_QualifiedName, &target);
-        reader_set_strvalue(reader, StringValue_Value, &strval_empty);
+        reader_free_strvalued(reader, &reader->empty_element.localname);
+        reader_strvaldup(reader, &target, &reader->empty_element.localname);
+        reader_free_strvalued(reader, &reader->empty_element.qname);
+        reader_strvaldup(reader, &target, &reader->empty_element.qname);
+        reader_set_strvalue(reader, &strval_empty);
         reader->resumestate = XmlReadResumeState_PIBody;
         reader->resume[XmlReadResume_Body] = reader_get_cur(reader);
     default:
@@ -1684,7 +1656,7 @@ static HRESULT reader_parse_pi(xmlreader *reader)
                 reader->nodetype = XmlNodeType_ProcessingInstruction;
                 reader->resumestate = XmlReadResumeState_Initial;
                 reader->resume[XmlReadResume_Body] = 0;
-                reader_set_strvalue(reader, StringValue_Value, &value);
+                reader_set_strvalue(reader, &value);
                 return S_OK;
             }
         }
@@ -1706,9 +1678,7 @@ static HRESULT reader_parse_whitespace(xmlreader *reader)
         reader->resumestate = XmlReadResumeState_Whitespace;
         reader->resume[XmlReadResume_Body] = reader_get_cur(reader);
         reader->nodetype = XmlNodeType_Whitespace;
-        reader_set_strvalue(reader, StringValue_LocalName, &strval_empty);
-        reader_set_strvalue(reader, StringValue_QualifiedName, &strval_empty);
-        reader_set_strvalue(reader, StringValue_Value, &strval_empty);
+        reader_set_strvalue(reader, &strval_empty);
         /* fallthrough */
     case XmlReadResumeState_Whitespace:
     {
@@ -1720,7 +1690,7 @@ static HRESULT reader_parse_whitespace(xmlreader *reader)
 
         start = reader->resume[XmlReadResume_Body];
         reader_init_strvalue(start, reader_get_cur(reader)-start, &value);
-        reader_set_strvalue(reader, StringValue_Value, &value);
+        reader_set_strvalue(reader, &value);
         TRACE("%s\n", debug_strval(reader, &value));
         reader->resumestate = XmlReadResumeState_Initial;
     }
@@ -1919,8 +1889,10 @@ static HRESULT reader_parse_dtd(xmlreader *reader)
     reader_skipn(reader, 1);
 
     reader->nodetype = XmlNodeType_DocumentType;
-    reader_set_strvalue(reader, StringValue_LocalName, &name);
-    reader_set_strvalue(reader, StringValue_QualifiedName, &name);
+    reader_free_strvalued(reader, &reader->empty_element.localname);
+    reader_strvaldup(reader, &name, &reader->empty_element.localname);
+    reader_free_strvalued(reader, &reader->empty_element.qname);
+    reader_strvaldup(reader, &name, &reader->empty_element.qname);
 
     return S_OK;
 }
@@ -2342,9 +2314,7 @@ static HRESULT reader_parse_element(xmlreader *reader)
 
         reader->nodetype = XmlNodeType_Element;
         reader->resumestate = XmlReadResumeState_Initial;
-        reader_set_strvalue(reader, StringValue_Prefix, &prefix);
-        reader_set_strvalue(reader, StringValue_QualifiedName, &qname);
-        reader_set_strvalue(reader, StringValue_Value, &strval_empty);
+        reader_set_strvalue(reader, &strval_empty);
         break;
     }
     default:
@@ -2386,7 +2356,6 @@ static HRESULT reader_parse_endtag(xmlreader *reader)
 
     reader->nodetype = XmlNodeType_EndElement;
     reader->is_empty_element = FALSE;
-    reader_set_strvalue(reader, StringValue_Prefix, &prefix);
 
     return S_OK;
 }
@@ -2415,7 +2384,7 @@ static HRESULT reader_parse_cdata(xmlreader *reader)
         reader->nodetype = XmlNodeType_CDATA;
         reader->resume[XmlReadResume_Body] = start;
         reader->resumestate = XmlReadResumeState_CDATA;
-        reader_set_strvalue(reader, StringValue_Value, NULL);
+        reader_set_strvalue(reader, NULL);
     }
 
     while (*ptr)
@@ -2430,7 +2399,7 @@ static HRESULT reader_parse_cdata(xmlreader *reader)
             reader_skipn(reader, 3);
             TRACE("%s\n", debug_strval(reader, &value));
 
-            reader_set_strvalue(reader, StringValue_Value, &value);
+            reader_set_strvalue(reader, &value);
             reader->resume[XmlReadResume_Body] = 0;
             reader->resumestate = XmlReadResumeState_Initial;
             return S_OK;
@@ -2467,7 +2436,7 @@ static HRESULT reader_parse_chardata(xmlreader *reader)
         reader->nodetype = is_wchar_space(*ptr) ? XmlNodeType_Whitespace : XmlNodeType_Text;
         reader->resume[XmlReadResume_Body] = start;
         reader->resumestate = XmlReadResumeState_CharData;
-        reader_set_strvalue(reader, StringValue_Value, NULL);
+        reader_set_strvalue(reader, NULL);
     }
 
     position = reader->position;
@@ -2486,7 +2455,7 @@ static HRESULT reader_parse_chardata(xmlreader *reader)
 
             reader->empty_element.position = position;
             reader_init_strvalue(start, reader_get_cur(reader)-start, &value);
-            reader_set_strvalue(reader, StringValue_Value, &value);
+            reader_set_strvalue(reader, &value);
             reader->resume[XmlReadResume_Body] = 0;
             reader->resumestate = XmlReadResumeState_Initial;
             return S_OK;
@@ -2732,7 +2701,7 @@ static void reader_reset_parser(xmlreader *reader)
     reader_clear_elements(reader);
     reader_clear_attrs(reader);
     reader_clear_ns(reader);
-    reader_free_strvalues(reader);
+    reader_free_strvalued(reader, &reader->strvalue);
 
     reader->depth = 0;
     reader->nodetype = XmlNodeType_None;
@@ -2954,9 +2923,7 @@ static void reader_set_current_attribute(xmlreader *reader, struct attribute *at
 {
     reader->attr = attr;
     reader->chunk_read_off = 0;
-    reader_set_strvalue(reader, StringValue_Prefix, &attr->prefix);
-    reader_set_strvalue(reader, StringValue_QualifiedName, &attr->qname);
-    reader_set_strvalue(reader, StringValue_Value, &attr->value);
+    reader_set_strvalue(reader, &attr->value);
 }
 
 static HRESULT reader_move_to_first_attribute(xmlreader *reader)
@@ -3115,20 +3082,8 @@ static HRESULT WINAPI xmlreader_MoveToElement(IXmlReader* iface)
 
     This->attr = NULL;
 
-    /* FIXME: support other node types with 'attributes' like DTD */
-    if (This->is_empty_element) {
-        reader_set_strvalue(This, StringValue_Prefix, &This->empty_element.prefix);
-        reader_set_strvalue(This, StringValue_QualifiedName, &This->empty_element.qname);
-    }
-    else {
-        struct element *element = LIST_ENTRY(list_head(&This->elements), struct element, entry);
-        if (element) {
-            reader_set_strvalue(This, StringValue_Prefix, &element->prefix);
-            reader_set_strvalue(This, StringValue_QualifiedName, &element->qname);
-        }
-    }
     This->chunk_read_off = 0;
-    reader_set_strvalue(This, StringValue_Value, &strval_empty);
+    reader_set_strvalue(This, &strval_empty);
 
     return S_OK;
 }
@@ -3136,6 +3091,7 @@ static HRESULT WINAPI xmlreader_MoveToElement(IXmlReader* iface)
 static HRESULT WINAPI xmlreader_GetQualifiedName(IXmlReader* iface, LPCWSTR *name, UINT *len)
 {
     xmlreader *This = impl_from_IXmlReader(iface);
+    XmlNodeType nodetype;
     struct attribute *attribute = This->attr;
     struct element *element;
     UINT length;
@@ -3145,7 +3101,7 @@ static HRESULT WINAPI xmlreader_GetQualifiedName(IXmlReader* iface, LPCWSTR *nam
     if (!len)
         len = &length;
 
-    switch (reader_get_nodetype(This))
+    switch ((nodetype = reader_get_nodetype(This)))
     {
     case XmlNodeType_Text:
     case XmlNodeType_CDATA:
@@ -3175,8 +3131,8 @@ static HRESULT WINAPI xmlreader_GetQualifiedName(IXmlReader* iface, LPCWSTR *nam
             *len = 5;
         } else if (attribute->prefix.len)
         {
-            *name = This->strvalues[StringValue_QualifiedName].str;
-            *len = This->strvalues[StringValue_QualifiedName].len;
+            *name = attribute->qname.str;
+            *len = attribute->qname.len;
         }
         else
         {
@@ -3184,10 +3140,17 @@ static HRESULT WINAPI xmlreader_GetQualifiedName(IXmlReader* iface, LPCWSTR *nam
             *len = attribute->localname.len;
         }
         break;
-    default:
-        *name = This->strvalues[StringValue_QualifiedName].str;
-        *len = This->strvalues[StringValue_QualifiedName].len;
+    case XmlNodeType_DocumentType:
+    case XmlNodeType_XmlDeclaration:
+    case XmlNodeType_ProcessingInstruction:
+        *name = This->empty_element.qname.str;
+        *len = This->empty_element.qname.len;
         break;
+    default:
+        FIXME("Unhandled node type %d\n", nodetype);
+        *name = NULL;
+        *len = 0;
+        return E_NOTIMPL;
     }
 
     return S_OK;
@@ -3204,7 +3167,6 @@ static struct ns *reader_lookup_nsdef(xmlreader *reader)
 static HRESULT WINAPI xmlreader_GetNamespaceUri(IXmlReader* iface, const WCHAR **uri, UINT *len)
 {
     xmlreader *This = impl_from_IXmlReader(iface);
-    const strval *prefix = &This->strvalues[StringValue_Prefix];
     XmlNodeType nodetype;
     struct ns *ns;
     UINT length;
@@ -3222,7 +3184,8 @@ static HRESULT WINAPI xmlreader_GetNamespaceUri(IXmlReader* iface, const WCHAR *
     case XmlNodeType_Element:
     case XmlNodeType_EndElement:
         {
-            ns = reader_lookup_ns(This, prefix);
+            struct element *element = reader_get_element(This);
+            ns = reader_lookup_ns(This, &element->prefix);
 
             /* pick top default ns if any */
             if (!ns)
@@ -3260,6 +3223,7 @@ static HRESULT WINAPI xmlreader_GetNamespaceUri(IXmlReader* iface, const WCHAR *
 static HRESULT WINAPI xmlreader_GetLocalName(IXmlReader* iface, LPCWSTR *name, UINT *len)
 {
     xmlreader *This = impl_from_IXmlReader(iface);
+    XmlNodeType nodetype;
     struct element *element;
     UINT length;
 
@@ -3268,7 +3232,7 @@ static HRESULT WINAPI xmlreader_GetLocalName(IXmlReader* iface, LPCWSTR *name, U
     if (!len)
         len = &length;
 
-    switch (reader_get_nodetype(This))
+    switch ((nodetype = reader_get_nodetype(This)))
     {
     case XmlNodeType_Text:
     case XmlNodeType_CDATA:
@@ -3286,10 +3250,17 @@ static HRESULT WINAPI xmlreader_GetLocalName(IXmlReader* iface, LPCWSTR *name, U
     case XmlNodeType_Attribute:
         reader_get_attribute_local_name(This, This->attr, name, len);
         break;
-    default:
-        *name = This->strvalues[StringValue_LocalName].str;
-        *len = This->strvalues[StringValue_LocalName].len;
+    case XmlNodeType_DocumentType:
+    case XmlNodeType_XmlDeclaration:
+    case XmlNodeType_ProcessingInstruction:
+        *name = This->empty_element.localname.str;
+        *len = This->empty_element.localname.len;
         break;
+    default:
+        FIXME("Unhandled node type %d\n", nodetype);
+        *name = NULL;
+        *len = 0;
+        return E_NOTIMPL;
     }
 
     return S_OK;
@@ -3315,8 +3286,10 @@ static HRESULT WINAPI xmlreader_GetPrefix(IXmlReader* iface, const WCHAR **ret,
     case XmlNodeType_EndElement:
     case XmlNodeType_Attribute:
     {
-        const strval *prefix = &This->strvalues[StringValue_Prefix];
         struct ns *ns;
+        struct element *element = reader_get_element(This);
+        const strval *prefix = (nodetype == XmlNodeType_Attribute) ?
+                &This->attr->prefix : &element->prefix;
 
         if (strval_eq(This, prefix, &strval_xml))
         {
@@ -3369,7 +3342,7 @@ static const strval *reader_get_value(xmlreader *reader, BOOL ensure_allocated)
         break;
     }
 
-    val = &reader->strvalues[StringValue_Value];
+    val = &reader->strvalue;
     if (!val->str && ensure_allocated)
     {
         WCHAR *ptr = reader_alloc(reader, (val->len+1)*sizeof(WCHAR));
@@ -3385,7 +3358,7 @@ static const strval *reader_get_value(xmlreader *reader, BOOL ensure_allocated)
 static HRESULT WINAPI xmlreader_GetValue(IXmlReader* iface, const WCHAR **value, UINT *len)
 {
     xmlreader *reader = impl_from_IXmlReader(iface);
-    const strval *val = &reader->strvalues[StringValue_Value];
+    const strval *val = &reader->strvalue;
     UINT off;
 
     TRACE("(%p)->(%p %p)\n", reader, value, len);
@@ -3651,7 +3624,6 @@ HRESULT WINAPI CreateXmlReader(REFIID riid, void **obj, IMalloc *imalloc)
 {
     xmlreader *reader;
     HRESULT hr;
-    int i;
 
     TRACE("(%s, %p, %p)\n", wine_dbgstr_guid(riid), obj, imalloc);
 
@@ -3679,8 +3651,7 @@ HRESULT WINAPI CreateXmlReader(REFIID riid, void **obj, IMalloc *imalloc)
     reader->max_depth = 256;
 
     reader->chunk_read_off = 0;
-    for (i = 0; i < StringValue_Last; i++)
-        reader->strvalues[i] = strval_empty;
+    reader->strvalue = strval_empty;
 
     hr = IXmlReader_QueryInterface(&reader->IXmlReader_iface, riid, obj);
     IXmlReader_Release(&reader->IXmlReader_iface);
diff --git a/dlls/xmllite/tests/reader.c b/dlls/xmllite/tests/reader.c
index 41adad1598..c93ee01fa1 100644
--- a/dlls/xmllite/tests/reader.c
+++ b/dlls/xmllite/tests/reader.c
@@ -1320,10 +1320,9 @@ static void test_read_public_dtd(void)
     str = NULL;
     hr = IXmlReader_GetQualifiedName(reader, &str, &len);
     ok(hr == S_OK, "got 0x%08x\n", hr);
-todo_wine {
     ok(len == lstrlenW(dtdnameW), "got %u\n", len);
     ok(!lstrcmpW(str, dtdnameW), "got %s\n", wine_dbgstr_w(str));
-}
+
     IXmlReader_Release(reader);
 }
 
@@ -1373,10 +1372,8 @@ static void test_read_system_dtd(void)
     str = NULL;
     hr = IXmlReader_GetQualifiedName(reader, &str, &len);
     ok(hr == S_OK, "got 0x%08x\n", hr);
-todo_wine {
     ok(len == lstrlenW(dtdnameW), "got %u\n", len);
     ok(!lstrcmpW(str, dtdnameW), "got %s\n", wine_dbgstr_w(str));
-}
 
     read_node(reader, XmlNodeType_Comment);
 
-- 
2.21.0




More information about the wine-devel mailing list