[PATCH] xmllite: Avoid extra read from stream when identifying comment node.

Nikolay Sivov 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);
>>>            ptr++;
>>> +        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.
>
> Thanks,
> Jeff




More information about the wine-devel mailing list