[PATCH 0/3] MR1199: Implement a few APIs
Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199
From: David Kahurani <k.kahurani(a)gmail.com> Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/xmllite/tests/writer.c | 79 +++++++++++++++++++++++++++++++++++++ dlls/xmllite/writer.c | 70 ++++++++++++++++++++------------ 2 files changed, 124 insertions(+), 25 deletions(-) diff --git a/dlls/xmllite/tests/writer.c b/dlls/xmllite/tests/writer.c index c4ab079373c..f26228327b0 100644 --- a/dlls/xmllite/tests/writer.c +++ b/dlls/xmllite/tests/writer.c @@ -1486,6 +1486,15 @@ static void test_writer_state(void) check_writer_state(writer, WR_E_INVALIDACTION); IStream_Release(stream); + /* WriteChars */ + stream = writer_set_output(writer); + + hr = IXmlWriter_WriteChars(writer, L"a", 1); + ok(hr == WR_E_INVALIDACTION, "Unexpected hr %#lx.\n", hr); + + check_writer_state(writer, WR_E_INVALIDACTION); + + IStream_Release(stream); IXmlWriter_Release(writer); } @@ -2090,6 +2099,75 @@ static void test_WriteString(void) IStream_Release(stream); } +static void test_WriteChars(void) +{ + IXmlWriter *writer; + IStream *stream; + HRESULT hr; + + hr = CreateXmlWriter(&IID_IXmlWriter, (void**)&writer, NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + stream = writer_set_output(writer); + + writer_set_property(writer, XmlWriterProperty_OmitXmlDeclaration); + + hr = IXmlWriter_WriteStartElement(writer, NULL, L"chars", NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + /* Writes the whole string but returns an error, strange but true */ + hr = IXmlWriter_WriteChars(writer, L"<chars>", 20); + ok(hr == WC_E_XMLCHARACTER, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteChars(writer, L"<chars>", 7); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_Flush(writer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + CHECK_OUTPUT(stream, + "<chars><chars><chars>"); + IStream_Release(stream); + + stream = writer_set_output(writer); + + hr = IXmlWriter_WriteStartElement(writer, NULL, L"chars", NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteChars(writer, NULL, 0); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteFullEndElement(writer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_Flush(writer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + CHECK_OUTPUT(stream, + "<chars></chars>"); + IStream_Release(stream); + + stream = writer_set_output(writer); + + hr = IXmlWriter_WriteStartElement(writer, NULL, L"chars", NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteChars(writer, L"", 0); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteEndElement(writer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_Flush(writer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + CHECK_OUTPUT(stream, + "<chars />"); + + IXmlWriter_Release(writer); + IStream_Release(stream); +} + static void test_WriteDocType(void) { static const struct @@ -2617,6 +2695,7 @@ START_TEST(writer) test_WriteAttributeString(); test_WriteFullEndElement(); test_WriteCharEntity(); + test_WriteChars(); test_WriteString(); test_WriteDocType(); test_WriteWhitespace(); diff --git a/dlls/xmllite/writer.c b/dlls/xmllite/writer.c index 065716f7e8c..bd5c649b94b 100644 --- a/dlls/xmllite/writer.c +++ b/dlls/xmllite/writer.c @@ -1104,28 +1104,6 @@ static HRESULT WINAPI xmlwriter_WriteCharEntity(IXmlWriter *iface, WCHAR ch) return S_OK; } -static HRESULT WINAPI xmlwriter_WriteChars(IXmlWriter *iface, const WCHAR *pwch, UINT cwch) -{ - xmlwriter *This = impl_from_IXmlWriter(iface); - - FIXME("%p %s %d\n", This, wine_dbgstr_w(pwch), cwch); - - switch (This->state) - { - case XmlWriterState_Initial: - return E_UNEXPECTED; - case XmlWriterState_InvalidEncoding: - return MX_E_ENCODING; - case XmlWriterState_DocClosed: - return WR_E_INVALIDACTION; - default: - ; - } - - return E_NOTIMPL; -} - - static HRESULT WINAPI xmlwriter_WriteComment(IXmlWriter *iface, LPCWSTR comment) { xmlwriter *This = impl_from_IXmlWriter(iface); @@ -1834,9 +1812,9 @@ static HRESULT WINAPI xmlwriter_WriteStartElement(IXmlWriter *iface, LPCWSTR pre return S_OK; } -static void write_escaped_string(xmlwriter *writer, const WCHAR *string) +static int write_escaped_string(xmlwriter *writer, const WCHAR *string, int length) { - while (*string) + while (*string && length) { switch (*string) { @@ -1854,7 +1832,10 @@ static void write_escaped_string(xmlwriter *writer, const WCHAR *string) } string++; + length--; } + + return length; } static HRESULT WINAPI xmlwriter_WriteString(IXmlWriter *iface, const WCHAR *string) @@ -1884,10 +1865,49 @@ static HRESULT WINAPI xmlwriter_WriteString(IXmlWriter *iface, const WCHAR *stri } This->textnode = 1; - write_escaped_string(This, string); + write_escaped_string(This, string, lstrlenW(string)); return S_OK; } +static HRESULT WINAPI xmlwriter_WriteChars(IXmlWriter *iface, const WCHAR *pwch, UINT cwch) +{ + xmlwriter *This = impl_from_IXmlWriter(iface); + int not_written = 0; + + TRACE("%p %s %d\n", This, wine_dbgstr_w(pwch), cwch); + + if ((pwch == NULL && cwch != 0)) + return WC_E_XMLCHARACTER; + else if ((pwch == NULL && cwch == 0)) + return S_OK; + else if ((is_empty_string(pwch) && cwch == 0)) + return S_OK; + + switch (This->state) + { + case XmlWriterState_Initial: + return E_UNEXPECTED; + case XmlWriterState_InvalidEncoding: + return MX_E_ENCODING; + case XmlWriterState_ElemStarted: + writer_close_starttag(This); + break; + case XmlWriterState_Ready: + case XmlWriterState_DocClosed: + This->state = XmlWriterState_DocClosed; + return WR_E_INVALIDACTION; + default: + ; + } + + This->textnode = 1; + not_written = write_escaped_string(This, pwch, cwch); + if (not_written) + return WC_E_XMLCHARACTER; + else + return S_OK; +} + static HRESULT WINAPI xmlwriter_WriteSurrogateCharEntity(IXmlWriter *iface, WCHAR wchLow, WCHAR wchHigh) { xmlwriter *This = impl_from_IXmlWriter(iface); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1199
From: David Kahurani <k.kahurani(a)gmail.com> Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/xmllite/tests/writer.c | 49 +++++++++++++++++++++++++++++++++++++ dlls/xmllite/writer.c | 25 +++++++++++++++++-- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/dlls/xmllite/tests/writer.c b/dlls/xmllite/tests/writer.c index f26228327b0..9a767dc4cc8 100644 --- a/dlls/xmllite/tests/writer.c +++ b/dlls/xmllite/tests/writer.c @@ -1892,6 +1892,54 @@ static void test_WriteCharEntity(void) IStream_Release(stream); } +static void test_WriteRawChars(void) +{ + IXmlWriter *writer; + IStream *stream; + HRESULT hr; + + hr = CreateXmlWriter(&IID_IXmlWriter, (void**)&writer, NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + stream = writer_set_output(writer); + + writer_set_property(writer, XmlWriterProperty_OmitXmlDeclaration); + + hr = IXmlWriter_WriteRawChars(writer, NULL, 0); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteRawChars(writer, L"", 0); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteStartElement(writer, NULL, L"sub", NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteRawChars(writer, L"<rawChars>", 5); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_Flush(writer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + CHECK_OUTPUT(stream, "<sub><rawC"); + IStream_Release(stream); + + stream = writer_set_output(writer); + + hr = IXmlWriter_WriteStartElement(writer, NULL, L"sub", NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteRawChars(writer, L"<;;>", 10); + ok(hr == WC_E_XMLCHARACTER, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_Flush(writer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + CHECK_OUTPUT(stream, "<sub><;;>"); + + IXmlWriter_Release(writer); + IStream_Release(stream); +} + static void test_WriteString(void) { IXmlWriter *writer; @@ -2696,6 +2744,7 @@ START_TEST(writer) test_WriteFullEndElement(); test_WriteCharEntity(); test_WriteChars(); + test_WriteRawChars(); test_WriteString(); test_WriteDocType(); test_WriteWhitespace(); diff --git a/dlls/xmllite/writer.c b/dlls/xmllite/writer.c index bd5c649b94b..35a388ea276 100644 --- a/dlls/xmllite/writer.c +++ b/dlls/xmllite/writer.c @@ -1698,8 +1698,19 @@ static HRESULT WINAPI xmlwriter_WriteRaw(IXmlWriter *iface, LPCWSTR data) static HRESULT WINAPI xmlwriter_WriteRawChars(IXmlWriter *iface, const WCHAR *pwch, UINT cwch) { xmlwriter *This = impl_from_IXmlWriter(iface); + HRESULT hr = S_OK; + + TRACE("%p %s %d\n", This, wine_dbgstr_w(pwch), cwch); + + if ((pwch == NULL && cwch != 0)) + return WC_E_XMLCHARACTER; + else if ((pwch == NULL && cwch == 0)) + return S_OK; + else if ((is_empty_string(pwch) && cwch == 0)) + return S_OK; - FIXME("%p %s %d\n", This, wine_dbgstr_w(pwch), cwch); + if ((pwch && cwch > (lstrlenW(pwch)))) + hr = WC_E_XMLCHARACTER; switch (This->state) { @@ -1709,11 +1720,21 @@ static HRESULT WINAPI xmlwriter_WriteRawChars(IXmlWriter *iface, const WCHAR *p return MX_E_ENCODING; case XmlWriterState_DocClosed: return WR_E_INVALIDACTION; + case XmlWriterState_Ready: + write_xmldecl(This, XmlStandalone_Omit); + break; + case XmlWriterState_ElemStarted: + writer_close_starttag(This); default: ; } - return E_NOTIMPL; + if (hr != S_OK) + write_output_buffer(This->output, pwch, -1); + else + write_output_buffer(This->output, pwch, cwch); + + return hr; } static HRESULT WINAPI xmlwriter_WriteStartDocument(IXmlWriter *iface, XmlStandalone standalone) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1199
From: David Kahurani <k.kahurani(a)gmail.com> Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/xmllite/tests/writer.c | 56 +++++++++++++++++++++++++++++++++++-- dlls/xmllite/writer.c | 31 ++++++++++++++++++-- 2 files changed, 82 insertions(+), 5 deletions(-) diff --git a/dlls/xmllite/tests/writer.c b/dlls/xmllite/tests/writer.c index 9a767dc4cc8..4f80a1ad886 100644 --- a/dlls/xmllite/tests/writer.c +++ b/dlls/xmllite/tests/writer.c @@ -138,6 +138,8 @@ static void check_writer_state(IXmlWriter *writer, HRESULT exp_hr) { IXmlReader *reader; HRESULT hr; + WCHAR low = 0xdcef; + WCHAR high = 0xdaff; hr = CreateXmlReader(&IID_IXmlReader, (void **)&reader, NULL); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -217,7 +219,8 @@ static void check_writer_state(IXmlWriter *writer, HRESULT exp_hr) hr = IXmlWriter_WriteString(writer, L"a"); ok(hr == exp_hr, "Unexpected hr %#lx, expected %#lx.\n", hr, exp_hr); - /* FIXME: add WriteSurrogateCharEntity */ + hr = IXmlWriter_WriteSurrogateCharEntity(writer, low, high); + ok(hr == exp_hr, "Unexpected hr %#lx, expected %#lx.\n", hr, exp_hr); hr = IXmlWriter_WriteWhitespace(writer, L" "); ok(hr == exp_hr, "Unexpected hr %#lx, expected %#lx.\n", hr, exp_hr); @@ -372,6 +375,8 @@ static void test_invalid_output_encoding(IXmlWriter *writer, IUnknown *output) { IXmlReader *reader; HRESULT hr; + WCHAR low = 0xdcef; + WCHAR high = 0xdaff; hr = CreateXmlReader(&IID_IXmlReader, (void **)&reader, NULL); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); @@ -448,7 +453,8 @@ static void test_invalid_output_encoding(IXmlWriter *writer, IUnknown *output) hr = IXmlWriter_WriteString(writer, L"a"); ok(hr == MX_E_ENCODING, "Unexpected hr %#lx.\n", hr); - /* TODO: WriteSurrogateCharEntity */ + hr = IXmlWriter_WriteSurrogateCharEntity(writer, low, high); + ok(hr == MX_E_ENCODING, "Unexpected hr %#lx.\n", hr); hr = IXmlWriter_WriteWhitespace(writer, L" "); ok(hr == MX_E_ENCODING, "Unexpected hr %#lx.\n", hr); @@ -1940,6 +1946,50 @@ static void test_WriteRawChars(void) IStream_Release(stream); } +static void test_WriteSurrogateCharEntity(void) +{ + IXmlWriter *writer; + IStream *stream; + HRESULT hr; + WCHAR low = 0xdcef; + WCHAR high = 0xdaff; + + hr = CreateXmlWriter(&IID_IXmlWriter, (void**)&writer, NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + stream = writer_set_output(writer); + + writer_set_property(writer, XmlWriterProperty_OmitXmlDeclaration); + + hr = IXmlWriter_WriteStartDocument(writer, XmlStandalone_Omit); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteStartElement(writer, NULL, L"root", NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteSurrogateCharEntity(writer, high, low); + ok(hr == WC_E_XMLCHARACTER, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_Flush(writer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + CHECK_OUTPUT(stream, "<root"); + + hr = IXmlWriter_WriteSurrogateCharEntity(writer, low, high); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_WriteFullEndElement(writer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IXmlWriter_Flush(writer); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + CHECK_OUTPUT(stream, "<root></root>"); + + IXmlWriter_Release(writer); + IStream_Release(stream); +} + static void test_WriteString(void) { IXmlWriter *writer; @@ -2163,7 +2213,6 @@ static void test_WriteChars(void) hr = IXmlWriter_WriteStartElement(writer, NULL, L"chars", NULL); ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - /* Writes the whole string but returns an error, strange but true */ hr = IXmlWriter_WriteChars(writer, L"<chars>", 20); ok(hr == WC_E_XMLCHARACTER, "Unexpected hr %#lx.\n", hr); @@ -2745,6 +2794,7 @@ START_TEST(writer) test_WriteCharEntity(); test_WriteChars(); test_WriteRawChars(); + test_WriteSurrogateCharEntity(); test_WriteString(); test_WriteDocType(); test_WriteWhitespace(); diff --git a/dlls/xmllite/writer.c b/dlls/xmllite/writer.c index 35a388ea276..a184d2c71e2 100644 --- a/dlls/xmllite/writer.c +++ b/dlls/xmllite/writer.c @@ -1932,10 +1932,37 @@ static HRESULT WINAPI xmlwriter_WriteChars(IXmlWriter *iface, const WCHAR *pwch, static HRESULT WINAPI xmlwriter_WriteSurrogateCharEntity(IXmlWriter *iface, WCHAR wchLow, WCHAR wchHigh) { xmlwriter *This = impl_from_IXmlWriter(iface); + int codepoint; + WCHAR bufW[16]; - FIXME("%p %d %d\n", This, wchLow, wchHigh); + TRACE("%p %d %d\n", This, wchLow, wchHigh); - return E_NOTIMPL; + if (!(wchLow >= 0xdc00 && wchLow <= 0xdfff)) + return WC_E_XMLCHARACTER; + + if (!(wchHigh <= 0xdbff && wchHigh >= 0xd800)) + return WC_E_XMLCHARACTER; + + switch(This->state) + { + case XmlWriterState_Initial: + return E_UNEXPECTED; + case XmlWriterState_InvalidEncoding: + return MX_E_ENCODING; + case XmlWriterState_ElemStarted: + writer_close_starttag(This); + break; + case XmlWriterState_DocClosed: + return WR_E_INVALIDACTION; + default: + ; + } + + codepoint = ((wchHigh - 0xd800) * 0x400) + (wchLow - 0xdc00) + 0x10000; + swprintf(bufW, ARRAY_SIZE(bufW), L"&#x%X;", codepoint); + write_output_buffer(This->output, bufW, -1); + + return S_OK; } static HRESULT WINAPI xmlwriter_WriteWhitespace(IXmlWriter *iface, LPCWSTR text) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1199
Nikolay Sivov (@nsivov) commented about dlls/xmllite/tests/writer.c:
+ IStream *stream; + HRESULT hr; + + hr = CreateXmlWriter(&IID_IXmlWriter, (void**)&writer, NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + stream = writer_set_output(writer); + + writer_set_property(writer, XmlWriterProperty_OmitXmlDeclaration); + + hr = IXmlWriter_WriteStartElement(writer, NULL, L"chars", NULL); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + /* Writes the whole string but returns an error, strange but true */ + hr = IXmlWriter_WriteChars(writer, L"<chars>", 20); + ok(hr == WC_E_XMLCHARACTER, "Unexpected hr %#lx.\n", hr); I don't think it's strange. It simply checks for valid characters in given input, following https://www.w3.org/TR/xml/#charsets:
`| [2] | Char | ::= | #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF] |` First character to fail this grammar is \0 in your input. The fact that it does not check everything before writing is probably to improve performance. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_12648
I think we should have more descriptive MR titles than "Implement a few APIs", even "xmllite: Implement a few APIs" would have been much better. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_12649
Nikolay Sivov (@nsivov) commented about dlls/xmllite/writer.c:
+ return MX_E_ENCODING; + case XmlWriterState_ElemStarted: + writer_close_starttag(This); + break; + case XmlWriterState_Ready: + case XmlWriterState_DocClosed: + This->state = XmlWriterState_DocClosed; + return WR_E_INVALIDACTION; + default: + ; + } + + This->textnode = 1; + not_written = write_escaped_string(This, pwch, cwch); + if (not_written) + return WC_E_XMLCHARACTER; I think this is misleading. Not written does not imply "invalid character" condition.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_12650
Nikolay Sivov (@nsivov) commented about dlls/xmllite/writer.c:
}
This->textnode = 1; - write_escaped_string(This, string); + write_escaped_string(This, string, lstrlenW(string));
I think we talked about this at some point. There is no need to get a string length that you don't have to. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_12652
Nikolay Sivov (@nsivov) commented about dlls/xmllite/writer.c:
+ if ((pwch == NULL && cwch != 0)) + return WC_E_XMLCHARACTER; + else if ((pwch == NULL && cwch == 0)) + return S_OK; + else if ((is_empty_string(pwch) && cwch == 0)) + return S_OK; + + switch (This->state) + { + case XmlWriterState_Initial: + return E_UNEXPECTED; + case XmlWriterState_InvalidEncoding: + return MX_E_ENCODING; + case XmlWriterState_ElemStarted: + writer_close_starttag(This); + break; I don't think we have a test for this part.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_12654
Nikolay Sivov (@nsivov) commented about dlls/xmllite/writer.c:
+ case XmlWriterState_Initial: + return E_UNEXPECTED; + case XmlWriterState_InvalidEncoding: + return MX_E_ENCODING; + case XmlWriterState_ElemStarted: + writer_close_starttag(This); + break; + case XmlWriterState_DocClosed: + return WR_E_INVALIDACTION; + default: + ; + } + + codepoint = ((wchHigh - 0xd800) * 0x400) + (wchLow - 0xdc00) + 0x10000; + swprintf(bufW, ARRAY_SIZE(bufW), L"&#x%X;", codepoint); + write_output_buffer(This->output, bufW, -1); What happens if you pass two surrogate halves to WriteString/WriteChars/WriteRawChars? Or even something like WriteChars(0xab, 1). It's possible they all share same hex escaping, and WriteSurrogateCharEntity() is not unique in that. Also, checking explicitly ranges here could as well be misplaced. I suspect WriteString() could validate that as well.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_12656
Nikolay Sivov (@nsivov) commented about dlls/xmllite/writer.c:
}
This->textnode = 1; - write_escaped_string(This, string); + write_escaped_string(This, string, lstrlenW(string)); return S_OK; }
+static HRESULT WINAPI xmlwriter_WriteChars(IXmlWriter *iface, const WCHAR *pwch, UINT cwch) +{ + xmlwriter *This = impl_from_IXmlWriter(iface); + int not_written = 0; + + TRACE("%p %s %d\n", This, wine_dbgstr_w(pwch), cwch);
For new methods let's use "xmlwriter *writer", and commas in a trace message. For string tracing this should be using debugstr_wn(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_12657
On Tue Nov 1 14:57:41 2022 +0000, Nikolay Sivov wrote:
What happens if you pass two surrogate halves to WriteString/WriteChars/WriteRawChars? Or even something like WriteChars(0xab, 1). It's possible they all share same hex escaping, and WriteSurrogateCharEntity() is not unique in that. Also, checking explicitly ranges here could as well be misplaced. I suspect WriteString() could validate that as well. I'm not sure what you mean by this part. I mean, yes, WriteString/WriteChars/WriteRawChars all reject surrogate characters, whether passed in as one or as a pair. They all do so with WR_E_INVALIDSURROGATEPAIR so yes something needs to be done there. WriteChars(0xab, 1) will fail because WriteChars expects a string and so do the other relatives. I fail to understand the part about hex escaping because it doesn't look to me like these functions ever get to see the hex passed in the user, they only see Unicode characters...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_13118
On Tue Nov 1 14:37:26 2022 +0000, Nikolay Sivov wrote:
I think this is misleading. Not written does not imply "invalid character" condition. Well, this is based on the assumption that the string is null-terminated which means we hit a null character at the end which of course, it not a valid character.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_13119
On Fri Nov 4 07:10:33 2022 +0000, David Kahurani wrote:
I'm not sure what you mean by this part. I mean, yes, WriteString/WriteChars/WriteRawChars all reject surrogate characters, whether passed in as one or as a pair. They all do so with WR_E_INVALIDSURROGATEPAIR so yes something needs to be done there. WriteChars(0xab, 1) will fail because WriteChars expects a string and so do the other relatives. I fail to understand the part about hex escaping because it doesn't look to me like these functions ever get to see the hex passed in the user, they only see Unicode characters... You can only pass surrogates there as a pair of consecutive characters. What do you mean by 0xab example? I don't get it. Regarding escaping, I mean you're using `&#x%X;` char entity just in this one method, but what happens if you do WriteChars(0x100, 1)? Shouldn't it be using same approach there?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_13149
On Fri Nov 4 07:11:29 2022 +0000, David Kahurani wrote:
Well, this is based on the assumption that the string is null-terminated which means we hit a null character at the end which of course, it not a valid character. From this calling site it's not clear at all. Maybe helper itself could return this code. Anyway, it seems to me that some more testing is necessary to figure out what's common between those 3 or 4 methods that write plain text data.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_13152
On Fri Nov 4 12:09:20 2022 +0000, Nikolay Sivov wrote:
You can only pass surrogates there as a pair of consecutive characters. What do you mean by 0xab example? I don't get it. Regarding escaping, I mean you're using `&#x%X;` char entity just in this one method, but what happens if you do WriteChars(0x100, 1)? Shouldn't it be using same approach there? Okay - I mostly understood everything now with the exception of the WriteChars example. Maybe I am just missing something but is WriteChars(0x100, 1) expected to compile? Do you by any chance mean WriteChars(L"\u0100", 1)? The prototype of this function being
``` HRESULT WriteChars (const WCHAR * characters, UINT count); ``` The API call will not compile... Is there something I am missing here? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_13260
On Sat Nov 5 10:17:27 2022 +0000, David Kahurani wrote:
Okay - I mostly understood everything now with the exception of the WriteChars example. Maybe I am just missing something but is WriteChars(0x100, 1) expected to compile? Do you by any chance mean WriteChars(L"\u0100", 1)? The prototype of this function being ``` HRESULT WriteChars (const WCHAR * characters, UINT count); ``` The API call will not compile... Is there something I am missing here? Of course. I don't mean that you should pass 0x100 pointer there.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_13333
On Sun Nov 6 11:44:54 2022 +0000, Nikolay Sivov wrote:
Of course. I don't mean that you should pass 0x100 pointer there. Okay - to answer your original question, that gives 'Ā' which I believe is the Unicode value for 0x100...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_13336
On Sun Nov 6 13:11:38 2022 +0000, David Kahurani wrote:
Okay - to answer your original question, that gives 'Ā' which I believe is the Unicode value for 0x100... The question was if it's written as is, or in an escaped form.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_13604
On Tue Nov 8 12:31:58 2022 +0000, Nikolay Sivov wrote:
The question was if it's written as is, or in an escaped form. It is not escaped. Or, to put it more simply because I don't know whether my testing is sufficient - I don't see any other characters other than the manually written value.
I'm also confused as to why you would anticipate that the value is escaped as, as only <functionname>Entity APIs seem to escape values. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_13731
On Tue Nov 8 17:18:12 2022 +0000, David Kahurani wrote:
It is not escaped. Or, to put it more simply because I don't know whether my testing is sufficient - I don't see any other characters other than the manually written value. I'm also confused as to why you would anticipate that the value is escaped as, as only <functionname>Entity APIs seem to escape values. Though, I think it will be necessary to escape the Unicode values into a string as `L"\\U%08x"` to accommodate surrogates. Maybe this is what you were driving at.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_13814
On Wed Nov 9 09:27:18 2022 +0000, David Kahurani wrote:
Though, I think it will be necessary to escape the Unicode values into a string as `L"\\U%08x"` to accommodate surrogates. Maybe this is what you were driving at. Edit: I don't think is this going to work as I had assumed. Silly me. Regarding WriteChars()/WriteString() if there is really no difference between them, we just need some tests to show that. Including tests with surrogates ranges that are rejected for some reason by these functions.
If something is unclear, please let me know. All the cleanups and changes that I mentioned already, I still would like to see in a next revision. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_13955
On Tue Nov 1 14:59:21 2022 +0000, Nikolay Sivov wrote:
For new methods let's use "xmlwriter *writer", and commas in a trace message. For string tracing this should be using debugstr_wn(). If we use debugstr_wn we will be forced to pass -1 as the length of the string because the user might request a size larger than the length of the string...should we still use debugstr_wn?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_16486
On Sat Nov 19 11:32:16 2022 +0000, David Kahurani wrote:
If we use debugstr_wn we will be forced to pass -1 as the length of the string because the user might request a size larger than the length of the string...should we still use debugstr_wn? We usually rely on correct input in such cases.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1199#note_16490
participants (4)
-
David Kahurani -
David Kahurani (＠kahurani)
-
Nikolay Sivov (＠nsivov)
-
Torge Matthies (＠tmatthies)