[PATCH] xmllite: Avoid extra read from stream when identifying comment node.
nsivov at codeweavers.com
Wed Nov 20 09:44:12 CST 2019
On 11/20/19 6:23 PM, Jeff Smith wrote:
> On Wed, Nov 20, 2019 at 4:32 AM Nikolay Sivov <nsivov at codeweavers.com> wrote:
>> On 11/19/19 4:40 AM, Jeff Smith wrote:
>>> Signed-off-by: Jeff Smith <whydoubt at gmail.com>
>>> dlls/xmllite/reader.c | 5 +++--
>>> dlls/xmllite/tests/reader.c | 1 -
>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>> diff --git a/dlls/xmllite/reader.c b/dlls/xmllite/reader.c
>>> index eddc4d8eec..5299871136 100644
>>> --- a/dlls/xmllite/reader.c
>>> +++ b/dlls/xmllite/reader.c
>>> @@ -1459,7 +1459,7 @@ static HRESULT reader_parse_comment(xmlreader *reader)
>>> reader_init_strvalue(start, reader_get_cur(reader)-start, &value);
>>> TRACE("%s\n", debug_strval(reader, &value));
>>> - /* skip rest of markup '->' */
>>> + /* skip rest of markup '-->' */
>>> reader_skipn(reader, 3);
>> That's obviously correct.
>>> reader_set_strvalue(reader, StringValue_Value, &value);
>>> @@ -1472,8 +1472,9 @@ static HRESULT reader_parse_comment(xmlreader *reader)
>>> - reader_skipn(reader, 1);
>>> + if (*ptr)
>>> + reader_skipn(reader, 1);
>> I don't think it makes sense to change that just to get expected call
>> sequence. Admittedly test itself is questionable.
> Hi Nikolay,
> It appears that determining the value of the test is key here.
> It does seem to be bordering on 'implementation detail' territory.
> I think either the test should be removed, or the code should be fixed
> to pass the test.
> If the test is removed, there is no need for my patch.
> Otherwise, the test should pass, and I will defend my fix if need be.
I wouldn't mind if this test function was removed.
More information about the wine-devel