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

[Alexandre Julliard]

Robert Shearman <rob(a)codeweavers.com> writes:
> +                /* hexadecimal entity */
> +                while ((*p >= '0' && *p <= '9') || (*p >= 'a' && *p <= 'f') ||
> +                       (*p >= 'A' && *p <= 'F') || (*p == ';'))
> +                    p++;

You should exit the loop at the first ';'. Also you have to increment
p first.
> +            while (isalpha(*p))
> +                p++;

You can't use ASCII ctype functions on Unicode chars.
> +            for (i = 0; i < sizeof(char_refs)/sizeof(char_refs[0]); i++)
> +                if (p - start - 1 <= sizeof(char_refs[i].name))
> +                {
> +                    if (!strncmpW(char_refs[i].name, start + 1, p - start - 1))
> +                        break;
> +                }

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.

-- 
Alexandre Julliard
julliard(a)winehq.org

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

[Robert Shearman]

Alexandre Julliard wrote:
> Robert Shearman <rob(a)codeweavers.com> writes:
> > +                /* hexadecimal entity */
> > +                while ((*p >= '0' && *p <= '9') || (*p >= 'a' && *p <= 'f') ||
> > +                       (*p >= 'A' && *p <= 'F') || (*p == ';'))
> > +                    p++;
>
> You should exit the loop at the first ';'. Also you have to increment
> p first.
> > +            while (isalpha(*p))
> > +                p++;
>
> You can't use ASCII ctype functions on Unicode chars.

Thanks, I'll send an updated patch which should address these comments.
> > +            for (i = 0; i < sizeof(char_refs)/sizeof(char_refs[0]); i++)
> > +                if (p - start - 1 <= sizeof(char_refs[i].name))
> > +                {
> > +                    if (!strncmpW(char_refs[i].name, start + 1, p - start - 1))
> > +                        break;
> > +                }
>
> 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.

Again, I'm not sure what you mean here. I know (start + 1) -> p will not 
have any nul's in it, therefore strncmpW will return non-zero if 
char_refs[i].name has a nul in it before (p - start - 1) characters. I 
wrote a small test program to prove this:
    static const WCHAR str1[] = {'t','e','s','t',0};
    static const WCHAR str2[] = {'t','e','s','t','s','t','r','i','n','g',0};
    printf("strncmpW(str1, str2, sizeof(str2)/sizeof(str2[0])-1) = 
%d\n", strncmpW(str1, str2, sizeof(str2)/sizeof(str2[0])-1));
It outputs:
strncmpW(str1, str2, sizeof(str2)/sizeof(str2[0])-1) = -115

So I'm what other cases I need to handle where strncmpW succeeds when it 
shouldn't.

-- 
Rob Shearman