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

[Jeff Smith]

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.

Thanks,
Jeff