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@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);
Signed-off-by: Jeff Smith whydoubt@gmail.com --- dlls/xmllite/reader.c | 10 +++++----- dlls/xmllite/tests/reader.c | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/dlls/xmllite/reader.c b/dlls/xmllite/reader.c index dd033f37c5..701dd9cc31 100644 --- a/dlls/xmllite/reader.c +++ b/dlls/xmllite/reader.c @@ -1385,7 +1385,6 @@ static HRESULT reader_parse_xmldecl(xmlreader *reader) reader->nodetype = XmlNodeType_XmlDeclaration; reader->empty_element.position = position; reader_strvaldup(reader, &strval_xml, &reader->empty_element.localname); - reader_strvaldup(reader, &strval_xml, &reader->empty_element.qname);
return S_OK; } @@ -1620,8 +1619,6 @@ static HRESULT reader_parse_pi(xmlreader *reader) if (FAILED(hr)) return hr; 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); @@ -3141,11 +3138,14 @@ static HRESULT WINAPI xmlreader_GetQualifiedName(IXmlReader* iface, LPCWSTR *nam } break; case XmlNodeType_DocumentType: - case XmlNodeType_XmlDeclaration: - case XmlNodeType_ProcessingInstruction: *name = This->empty_element.qname.str; *len = This->empty_element.qname.len; break; + 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; diff --git a/dlls/xmllite/tests/reader.c b/dlls/xmllite/tests/reader.c index c93ee01fa1..e421178c4e 100644 --- a/dlls/xmllite/tests/reader.c +++ b/dlls/xmllite/tests/reader.c @@ -2534,7 +2534,8 @@ static void test_string_pointers(void) ok(empty == reader_value(reader, ""), "empty != value\n"); ok(empty == reader_prefix(reader, ""), "empty != prefix\n"); xml = reader_name(reader, "xml"); - ptr = reader_qname(reader, "xml"); todo_wine ok(xml == ptr, "xml != qname\n"); + ptr = reader_qname(reader, "xml"); + ok(xml == ptr, "xml != qname\n"); ok(empty == reader_namespace(reader, ""), "empty != namespace\n");
next_attribute(reader);
Signed-off-by: Jeff Smith whydoubt@gmail.com --- dlls/xmllite/tests/reader.c | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/dlls/xmllite/tests/reader.c b/dlls/xmllite/tests/reader.c index e421178c4e..2a6d8a053c 100644 --- a/dlls/xmllite/tests/reader.c +++ b/dlls/xmllite/tests/reader.c @@ -1270,11 +1270,10 @@ static const char test_public_dtd[] =
static void test_read_public_dtd(void) { - static const WCHAR dtdnameW[] = {'t','e','s','t','d','t','d',0}; IXmlReader *reader; - const WCHAR *str; + const WCHAR *name, *qname; XmlNodeType type; - UINT len, count; + UINT count; HRESULT hr;
hr = CreateXmlReader(&IID_IXmlReader, (void**)&reader, NULL); @@ -1314,14 +1313,10 @@ static void test_read_public_dtd(void) reader_value(reader, "externalid uri");
move_to_element(reader); - reader_name(reader, "testdtd"); - - len = 0; - str = NULL; - hr = IXmlReader_GetQualifiedName(reader, &str, &len); - ok(hr == S_OK, "got 0x%08x\n", hr); - ok(len == lstrlenW(dtdnameW), "got %u\n", len); - ok(!lstrcmpW(str, dtdnameW), "got %s\n", wine_dbgstr_w(str)); + name = reader_name(reader, "testdtd"); + qname = reader_qname(reader, "testdtd"); +todo_wine + ok(name == qname, "name != qname\n");
IXmlReader_Release(reader); } @@ -1332,11 +1327,10 @@ static const char test_system_dtd[] =
static void test_read_system_dtd(void) { - static const WCHAR dtdnameW[] = {'t','e','s','t','d','t','d',0}; IXmlReader *reader; - const WCHAR *str; + const WCHAR *name, *qname; XmlNodeType type; - UINT len, count; + UINT count; HRESULT hr;
hr = CreateXmlReader(&IID_IXmlReader, (void**)&reader, NULL); @@ -1366,14 +1360,10 @@ static void test_read_system_dtd(void) reader_value(reader, "externalid uri");
move_to_element(reader); - reader_name(reader, "testdtd"); - - len = 0; - str = NULL; - hr = IXmlReader_GetQualifiedName(reader, &str, &len); - ok(hr == S_OK, "got 0x%08x\n", hr); - ok(len == lstrlenW(dtdnameW), "got %u\n", len); - ok(!lstrcmpW(str, dtdnameW), "got %s\n", wine_dbgstr_w(str)); + name = reader_name(reader, "testdtd"); + qname = reader_qname(reader, "testdtd"); +todo_wine + ok(name == qname, "name != qname\n");
read_node(reader, XmlNodeType_Comment);
Signed-off-by: Jeff Smith whydoubt@gmail.com --- dlls/xmllite/reader.c | 96 +++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 48 deletions(-)
diff --git a/dlls/xmllite/reader.c b/dlls/xmllite/reader.c index 701dd9cc31..88db0df4c2 100644 --- a/dlls/xmllite/reader.c +++ b/dlls/xmllite/reader.c @@ -1846,54 +1846,6 @@ static HRESULT reader_parse_externalid(xmlreader *reader) return S_FALSE; }
-/* [28] doctypedecl ::= '<!DOCTYPE' S Name (S ExternalID)? S? ('[' intSubset ']' S?)? '>' */ -static HRESULT reader_parse_dtd(xmlreader *reader) -{ - static const WCHAR doctypeW[] = {'<','!','D','O','C','T','Y','P','E',0}; - strval name; - WCHAR *cur; - HRESULT hr; - - /* check if we have "<!DOCTYPE" */ - if (reader_cmp(reader, doctypeW)) return S_FALSE; - reader_shrink(reader); - - /* DTD processing is not allowed by default */ - if (reader->dtdmode == DtdProcessing_Prohibit) return WC_E_DTDPROHIBITED; - - reader_skipn(reader, 9); - if (!reader_skipspaces(reader)) return WC_E_WHITESPACE; - - /* name */ - hr = reader_parse_name(reader, &name); - if (FAILED(hr)) return WC_E_DECLDOCTYPE; - - reader_skipspaces(reader); - - hr = reader_parse_externalid(reader); - if (FAILED(hr)) return hr; - - reader_skipspaces(reader); - - cur = reader_get_ptr(reader); - if (*cur != '>') - { - FIXME("internal subset parsing not implemented\n"); - return E_NOTIMPL; - } - - /* skip '>' */ - reader_skipn(reader, 1); - - reader->nodetype = XmlNodeType_DocumentType; - 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; -} - /* [11 NS] LocalPart ::= NCName */ static HRESULT reader_parse_local(xmlreader *reader, strval *local, BOOL check_for_separator) { @@ -2009,6 +1961,54 @@ static HRESULT reader_parse_qname(xmlreader *reader, strval *prefix, strval *loc return S_OK; }
+/* [28] doctypedecl ::= '<!DOCTYPE' S Name (S ExternalID)? S? ('[' intSubset ']' S?)? '>' */ +static HRESULT reader_parse_dtd(xmlreader *reader) +{ + static const WCHAR doctypeW[] = {'<','!','D','O','C','T','Y','P','E',0}; + strval name; + WCHAR *cur; + HRESULT hr; + + /* check if we have "<!DOCTYPE" */ + if (reader_cmp(reader, doctypeW)) return S_FALSE; + reader_shrink(reader); + + /* DTD processing is not allowed by default */ + if (reader->dtdmode == DtdProcessing_Prohibit) return WC_E_DTDPROHIBITED; + + reader_skipn(reader, 9); + if (!reader_skipspaces(reader)) return WC_E_WHITESPACE; + + /* name */ + hr = reader_parse_name(reader, &name); + if (FAILED(hr)) return WC_E_DECLDOCTYPE; + + reader_skipspaces(reader); + + hr = reader_parse_externalid(reader); + if (FAILED(hr)) return hr; + + reader_skipspaces(reader); + + cur = reader_get_ptr(reader); + if (*cur != '>') + { + FIXME("internal subset parsing not implemented\n"); + return E_NOTIMPL; + } + + /* skip '>' */ + reader_skipn(reader, 1); + + reader->nodetype = XmlNodeType_DocumentType; + 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; +} + static WCHAR get_predefined_entity(const xmlreader *reader, const strval *name) { static const WCHAR entltW[] = {'l','t'};
Signed-off-by: Jeff Smith whydoubt@gmail.com --- dlls/xmllite/reader.c | 23 ++++++++++++------- dlls/xmllite/tests/reader.c | 46 +++++++++++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 10 deletions(-)
diff --git a/dlls/xmllite/reader.c b/dlls/xmllite/reader.c index 88db0df4c2..3875676f01 100644 --- a/dlls/xmllite/reader.c +++ b/dlls/xmllite/reader.c @@ -1965,7 +1965,7 @@ static HRESULT reader_parse_qname(xmlreader *reader, strval *prefix, strval *loc static HRESULT reader_parse_dtd(xmlreader *reader) { static const WCHAR doctypeW[] = {'<','!','D','O','C','T','Y','P','E',0}; - strval name; + strval prefix, local, qname; WCHAR *cur; HRESULT hr;
@@ -1980,7 +1980,7 @@ static HRESULT reader_parse_dtd(xmlreader *reader) if (!reader_skipspaces(reader)) return WC_E_WHITESPACE;
/* name */ - hr = reader_parse_name(reader, &name); + hr = reader_parse_qname(reader, &prefix, &local, &qname); if (FAILED(hr)) return WC_E_DECLDOCTYPE;
reader_skipspaces(reader); @@ -2002,9 +2002,12 @@ static HRESULT reader_parse_dtd(xmlreader *reader)
reader->nodetype = XmlNodeType_DocumentType; 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); + reader_strvaldup(reader, &local, &reader->empty_element.localname); + if (prefix.len) + { + reader_strvaldup(reader, &prefix, &reader->empty_element.prefix); + reader_strvaldup(reader, &qname, &reader->empty_element.qname); + }
return S_OK; } @@ -3138,9 +3141,13 @@ static HRESULT WINAPI xmlreader_GetQualifiedName(IXmlReader* iface, LPCWSTR *nam } break; case XmlNodeType_DocumentType: - *name = This->empty_element.qname.str; - *len = This->empty_element.qname.len; - break; + if (This->empty_element.prefix.len) + { + *name = This->empty_element.qname.str; + *len = This->empty_element.qname.len; + break; + } + /* fallthrough */ case XmlNodeType_XmlDeclaration: case XmlNodeType_ProcessingInstruction: *name = This->empty_element.localname.str; diff --git a/dlls/xmllite/tests/reader.c b/dlls/xmllite/tests/reader.c index 2a6d8a053c..a928456571 100644 --- a/dlls/xmllite/tests/reader.c +++ b/dlls/xmllite/tests/reader.c @@ -1315,7 +1315,6 @@ static void test_read_public_dtd(void) move_to_element(reader); name = reader_name(reader, "testdtd"); qname = reader_qname(reader, "testdtd"); -todo_wine ok(name == qname, "name != qname\n");
IXmlReader_Release(reader); @@ -1362,7 +1361,6 @@ static void test_read_system_dtd(void) move_to_element(reader); name = reader_name(reader, "testdtd"); qname = reader_qname(reader, "testdtd"); -todo_wine ok(name == qname, "name != qname\n");
read_node(reader, XmlNodeType_Comment); @@ -1370,6 +1368,49 @@ todo_wine IXmlReader_Release(reader); }
+static const char test_system_dtd_ns[] = + "<!DOCTYPE a:testdtd SYSTEM \"externalid uri\" >"; + +static void test_read_system_dtd_ns(void) +{ + IXmlReader *reader; + XmlNodeType type; + UINT count; + HRESULT hr; + + hr = CreateXmlReader(&IID_IXmlReader, (void**)&reader, NULL); + ok(hr == S_OK, "S_OK, got %08x\n", hr); + + hr = IXmlReader_SetProperty(reader, XmlReaderProperty_DtdProcessing, DtdProcessing_Parse); + ok(hr == S_OK, "got 0x%8x\n", hr); + + set_input_string(reader, test_system_dtd_ns); + + read_node(reader, XmlNodeType_DocumentType); + + count = 0; + hr = IXmlReader_GetAttributeCount(reader, &count); + ok(hr == S_OK, "got %08x\n", hr); + ok(count == 1, "got %d\n", count); + + hr = IXmlReader_MoveToFirstAttribute(reader); + ok(hr == S_OK, "got %08x\n", hr); + + type = XmlNodeType_None; + hr = IXmlReader_GetNodeType(reader, &type); + ok(hr == S_OK, "got %08x\n", hr); + ok(type == XmlNodeType_Attribute, "got %d\n", type); + + reader_name(reader, "SYSTEM"); + reader_value(reader, "externalid uri"); + + move_to_element(reader); + reader_name(reader, "testdtd"); + reader_qname(reader, "a:testdtd"); + + IXmlReader_Release(reader); +} + static struct test_entry element_tests[] = { { "<a/>", "a", "", S_OK }, { "<a />", "a", "", S_OK }, @@ -2652,6 +2693,7 @@ START_TEST(reader) test_read_comment(); test_read_pi(); test_read_system_dtd(); + test_read_system_dtd_ns(); test_read_public_dtd(); test_read_element(); test_isemptyelement();
Signed-off-by: Jeff Smith whydoubt@gmail.com --- dlls/xmllite/reader.c | 5 +++++ dlls/xmllite/tests/reader.c | 10 ++++++++++ 2 files changed, 15 insertions(+)
diff --git a/dlls/xmllite/reader.c b/dlls/xmllite/reader.c index 3875676f01..9731734258 100644 --- a/dlls/xmllite/reader.c +++ b/dlls/xmllite/reader.c @@ -1966,6 +1966,7 @@ static HRESULT reader_parse_dtd(xmlreader *reader) { static const WCHAR doctypeW[] = {'<','!','D','O','C','T','Y','P','E',0}; strval prefix, local, qname; + struct reader_position position; WCHAR *cur; HRESULT hr;
@@ -1980,6 +1981,7 @@ static HRESULT reader_parse_dtd(xmlreader *reader) if (!reader_skipspaces(reader)) return WC_E_WHITESPACE;
/* name */ + position = reader->position; hr = reader_parse_qname(reader, &prefix, &local, &qname); if (FAILED(hr)) return WC_E_DECLDOCTYPE;
@@ -2001,6 +2003,7 @@ static HRESULT reader_parse_dtd(xmlreader *reader) reader_skipn(reader, 1);
reader->nodetype = XmlNodeType_DocumentType; + reader->empty_element.position = position; reader_free_strvalued(reader, &reader->empty_element.localname); reader_strvaldup(reader, &local, &reader->empty_element.localname); if (prefix.len) @@ -3468,6 +3471,7 @@ static HRESULT WINAPI xmlreader_GetLineNumber(IXmlReader* iface, UINT *line_numb break; case XmlNodeType_Whitespace: case XmlNodeType_XmlDeclaration: + case XmlNodeType_DocumentType: *line_number = This->empty_element.position.line_number; break; default: @@ -3500,6 +3504,7 @@ static HRESULT WINAPI xmlreader_GetLinePosition(IXmlReader* iface, UINT *line_po break; case XmlNodeType_Whitespace: case XmlNodeType_XmlDeclaration: + case XmlNodeType_DocumentType: *line_position = This->empty_element.position.line_position; break; default: diff --git a/dlls/xmllite/tests/reader.c b/dlls/xmllite/tests/reader.c index a928456571..bcd0e1e552 100644 --- a/dlls/xmllite/tests/reader.c +++ b/dlls/xmllite/tests/reader.c @@ -1285,6 +1285,7 @@ static void test_read_public_dtd(void) set_input_string(reader, test_public_dtd);
read_node(reader, XmlNodeType_DocumentType); + TEST_READER_POSITION2(reader, 1, 11, ~0u, 51);
count = 0; hr = IXmlReader_GetAttributeCount(reader, &count); @@ -1293,6 +1294,7 @@ static void test_read_public_dtd(void)
hr = IXmlReader_MoveToFirstAttribute(reader); ok(hr == S_OK, "got %08x\n", hr); + TEST_READER_POSITION2(reader, 1, 19, ~0u, 51);
type = XmlNodeType_None; hr = IXmlReader_GetNodeType(reader, &type); @@ -1303,6 +1305,7 @@ static void test_read_public_dtd(void) reader_value(reader, "pubid");
next_attribute(reader); + TEST_READER_POSITION2(reader, 1, 19, ~0u, 51);
type = XmlNodeType_None; hr = IXmlReader_GetNodeType(reader, &type); @@ -1316,6 +1319,7 @@ static void test_read_public_dtd(void) name = reader_name(reader, "testdtd"); qname = reader_qname(reader, "testdtd"); ok(name == qname, "name != qname\n"); + TEST_READER_POSITION2(reader, 1, 11, ~0u, 51);
IXmlReader_Release(reader); } @@ -1341,6 +1345,7 @@ static void test_read_system_dtd(void) set_input_string(reader, test_system_dtd);
read_node(reader, XmlNodeType_DocumentType); + TEST_READER_POSITION2(reader, 1, 11, ~0u, 43);
count = 0; hr = IXmlReader_GetAttributeCount(reader, &count); @@ -1349,6 +1354,7 @@ static void test_read_system_dtd(void)
hr = IXmlReader_MoveToFirstAttribute(reader); ok(hr == S_OK, "got %08x\n", hr); + TEST_READER_POSITION2(reader, 1, 19, ~0u, 43);
type = XmlNodeType_None; hr = IXmlReader_GetNodeType(reader, &type); @@ -1362,6 +1368,7 @@ static void test_read_system_dtd(void) name = reader_name(reader, "testdtd"); qname = reader_qname(reader, "testdtd"); ok(name == qname, "name != qname\n"); + TEST_READER_POSITION2(reader, 1, 11, ~0u, 43);
read_node(reader, XmlNodeType_Comment);
@@ -1387,6 +1394,7 @@ static void test_read_system_dtd_ns(void) set_input_string(reader, test_system_dtd_ns);
read_node(reader, XmlNodeType_DocumentType); + TEST_READER_POSITION2(reader, 1, 11, ~0u, 45);
count = 0; hr = IXmlReader_GetAttributeCount(reader, &count); @@ -1395,6 +1403,7 @@ static void test_read_system_dtd_ns(void)
hr = IXmlReader_MoveToFirstAttribute(reader); ok(hr == S_OK, "got %08x\n", hr); + TEST_READER_POSITION2(reader, 1, 21, ~0u, 45);
type = XmlNodeType_None; hr = IXmlReader_GetNodeType(reader, &type); @@ -1407,6 +1416,7 @@ static void test_read_system_dtd_ns(void) move_to_element(reader); reader_name(reader, "testdtd"); reader_qname(reader, "a:testdtd"); + TEST_READER_POSITION2(reader, 1, 11, ~0u, 45);
IXmlReader_Release(reader); }
Hi, Jeff.
Sorry for the delay.
I think this looks good in principle, my suggestion would be to do a deeper cleanup first. Especially for 'empty_element' field.
Right now it's used for things that are not elements, with name suggesting otherwise. Using 'struct element' for it is also misleading for the same reason.
Maybe we can keep is as is, and use strictly for empty elements, because it's used together with element stack. Then we could add another structure, without defined type for example as a part of reader struct, to hold temporary data for "current" node - strings and position.
On Sun, Nov 3, 2019 at 1:50 PM Nikolay Sivov nsivov@codeweavers.com wrote:
I think this looks good in principle, my suggestion would be to do a deeper cleanup first. Especially for 'empty_element' field.
Right now it's used for things that are not elements, with name suggesting otherwise. Using 'struct element' for it is also misleading for the same reason.
Much of the difficulty seems to be caused by a shortcoming in XML terminology. Obviously the XML declaration, DTD, and processing instructions are not elements. However, they are strikingly similar to an empty element both practically and visually, and the xmllite reader method MoveToElement even treats them as such. Yet, I am not aware of a concise term that encompasses these semantics.
Maybe we can keep is as is, and use strictly for empty elements, because it's used together with element stack. Then we could add another structure, without defined type for example as a part of reader struct, to hold temporary data for "current" node - strings and position.
This other structure would need to contain the same members as struct element: the same three strings, the same position, and the same list for storing attributes. And we come back to what we call this thing. "Un-nestable element-like non-element thing" seems a little unwieldy. But do we really need 2 or 3 or 4 structures and 2 or 3 or 4 instances for these when one should suffice, just because XML doesn't have a proper term for the whole set?
-
Jeff Smith -
Nikolay Sivov