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

2008-04-17 Thread Hin-Tak Leung

--- On Wed, 16/4/08, Tomasz Jezierski - Tefnet [EMAIL PROTECTED] wrote:

snipped
 If you won't find entity in table and change ampersand
 to another char,
 you won't get infinite loop, but I'm not sure what
 will happen if you
 have amp; in help, that might trigger loop too. Do you
 know any free
 software in which I can craft such chm files?

I don't know about free, but the microsoft htmlhelp workshop is available 
for download... and you might be able to do something like that with it.

It is also a good source of information, BTW. Comes with an htmlhelp API 
reference. (I installed it under wine last week).

BTW, I keep forgetting - what is the policy about contributing code after
reading MSDN and official microsoft documentation available for download such 
as this? 

I think MSDN is okay, butactual source files (including include headers)
is not?


  ___ 
Yahoo! For Good helps you make a difference  

http://uk.promotions.yahoo.com/forgood/




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

2008-04-16 Thread Hin-Tak Leung



--- On Tue, 15/4/08, Tomasz Jezierski - Tefnet [EMAIL PROTECTED] wrote:
snipped
 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.

I have experience the infinite loop myself, but I haven't quite checked
where it is - it happens if the table is incomplete and the code
encounters an html entity that's not mentioned in the table. I encountered it 
when I tried to view some help pages containing german umlaute; 's
in another help file I happened to have around.
(gpstrack is about french accented characters.)

So in that sense, the patch is dangerous - infinite loop is far worse 
than not seeing a couple of help pages. I think it needs to do something 
sensible (e.g. don't translate) even if the table is incomplete.



  ___ 
Yahoo! For Good helps you make a difference  

http://uk.promotions.yahoo.com/forgood/




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

2008-04-16 Thread Tomasz Jezierski - Tefnet

Dnia 16-04-2008, śro o godzinie 00:49 +, Hin-Tak Leung pisze:
 
 
 --- On Tue, 15/4/08, Tomasz Jezierski - Tefnet [EMAIL PROTECTED] wrote:
 snipped
  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.
 
 I have experience the infinite loop myself, but I haven't quite checked
 where it is - it happens if the table is incomplete and the code
 encounters an html entity that's not mentioned in the table. I encountered it 
 when I tried to view some help pages containing german umlaute; 's
 in another help file I happened to have around.
 (gpstrack is about french accented characters.)
 
If you won't find entity in table and change ampersand to another char,
you won't get infinite loop, but I'm not sure what will happen if you
have amp; in help, that might trigger loop too. Do you know any free
software in which I can craft such chm files?








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

2008-04-15 Thread Tomasz Jezierski - Tefnet

Dnia 30-05-2007, śro o godzinie 14:41 +0200, Alexandre Julliard pisze:
 Robert Shearman [EMAIL PROTECTED] 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









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

2008-04-15 Thread Tomasz Jezierski - Tefnet
Dnia 30-05-2007, śro o godzinie 14:41 +0200, Alexandre Julliard pisze:
 Robert Shearman [EMAIL PROTECTED] writes:
 

Hi, I found this patch from May 2007 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









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

2007-05-30 Thread Alexandre Julliard
Robert Shearman [EMAIL PROTECTED] 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
[EMAIL PROTECTED]




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

2007-05-18 Thread Robert Shearman

Alexandre Julliard wrote:

Robert Shearman [EMAIL PROTECTED] 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





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

2007-05-01 Thread Alexandre Julliard
Robert Shearman [EMAIL PROTECTED] 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
[EMAIL PROTECTED]