Re: hhctrl.ocx: Parse HTML entities in the table of contents.

[Alexandre Julliard]

Robert Shearman <rob(a)codeweavers.com> writes:
> +            p++;
> +            if ((*p == 'X') || (*p == 'x'))
> +            {
> +                /* hexadecimal entity */
> +                while ((*p >= '0' && *p <= '9') || (*p >= 'a' && *p <= 'f') ||
> +                       (*p >= 'A' && *p <= 'F'))
> +                    p++;
> +                ch = strtolW(start + 2, NULL, 16);
> +            }

This is still broken.
> +            if (p - start - 1 <= sizeof(char_refs[0].name))
> +            {
> +                for (i = 0; i < sizeof(char_refs)/sizeof(char_refs[0]); i++)
> +                    if (!strncmpW(char_refs[i].name, start + 1, p - start - 1))
> +                        break;
> +            }

This is still broken too.

-- 
Alexandre Julliard
julliard(a)winehq.org

hhctrl.ocx: Parse HTML entities in the table of contents.

[Tomasz Jezierski - Tefnet]

Dnia 30-05-2007, śro o godzinie 14:41 +0200, Alexandre Julliard pisze:
> Robert Shearman <rob(a)codeweavers.com> writes:

Hi, I found this patch from May which never got into wine.
http://www.winehq.org/pipermail/wine-patches/2007-May/039722.html

Could you explain me what is still wrong with it? I would like to fix
it.
> > +            p++;
> > +            if ((*p == 'X') || (*p == 'x'))
> > +            {
> > +                /* hexadecimal entity */
> > +                while ((*p >= '0' && *p <= '9') || (*p >= 'a' && *p <= 'f') ||
> > +                       (*p >= 'A' && *p <= 'F'))
> > +                    p++;
> > +                ch = strtolW(start + 2, NULL, 16);
> > +            }
>
> This is still broken.
> You said earlier:
> "You should exit the loop at the first ';'. Also you have to increment
> p first."

It looks like Robert did what you said?
> > +            if (p - start - 1 <= sizeof(char_refs[0].name))
> > +            {
> > +                for (i = 0; i < sizeof(char_refs)/sizeof(char_refs[0]); i++)
> > +                    if (!strncmpW(char_refs[i].name, start + 1, p - start - 1))
> > +                        break;
> > +            }
>
> This is still broken too.

You said earlier:
"The sizeof check doesn't make much sense, especially inside the
loop. What you need is to check that you reached the end of the string
if the strncmpW succeeded."

You mean to check if *p == 0 ?



While reading this patch I found another bug:

+            if (i == sizeof(char_refs)/sizeof(char_refs[0]))
+            {
+                FIXME("character entity %s not found\n", debugstr_wn(start + 1, p - start - 1));
+                continue;
+            }

I think *start should be changed to another char in that case, because if we will not change it, we will fall into infinite loop like it happens with help file in GPSTRACK from bug #6801.

Moreover we need full entity table, one of those mentioned by Hin-Tak Leung:
http://bugs.winehq.org/show_bug.cgi?id=6801#c28