Re: [MSVCRT] implement _mbsbtype

[Juan Lang]

Raphael wrote:
> - if (!*str) { /** TODO: check *str validity */
> - return -1; /** _MBC_ILLEGAL */
> - }
> - if (start == str && MSVCRT_isleadbyte(*str)) {
> - return 1; /** _MBC_LEAD */
> - }
> - if (start == str && MSVCRT_isleadbyte(str[-1])) {
> - return 2; /**_MBC_TRAIL */
> - }
> -
> - return 0; /** _MBC_SINGLE */

Cool.  How about patching mbctype.h with these constants too?

--Juan

__________________________________ 
Do you Yahoo!? 
Yahoo! Small Business - Try our new resources site!
http://smallbusiness.yahoo.com/resources/

[MSVCRT] implement _mbsbtype

[Raphael]

On Tuesday 15 March 2005 00:43, Juan Lang wrote:
> Raphael wrote:
> > - if (!*str) { /** TODO: check *str validity */
> > - return -1; /** _MBC_ILLEGAL */
> > - }
> > - if (start == str && MSVCRT_isleadbyte(*str)) {
> > - return 1; /** _MBC_LEAD */
> > - }
> > - if (start == str && MSVCRT_isleadbyte(str[-1])) {
> > - return 2; /**_MBC_TRAIL */
> > - }
> > -
> > - return 0; /** _MBC_SINGLE */
>
> Cool.  How about patching mbctype.h with these constants too?
>
> --Juan

Try 2 :)

Changelog:
  - implement _mbsbtype (bug 1831)

This One without tests improvements as they failed on wine (as wine don't seems to support chineese) :(

the test patch for people who want to test:
Index: dlls/msvcrt/tests/string.c
===================================================================
RCS file: /home/wine/wine/dlls/msvcrt/tests/string.c,v
retrieving revision 1.2
diff -u -r1.2 string.c
--- dlls/msvcrt/tests/string.c  4 May 2004 04:13:06 -0000       1.2
+++ dlls/msvcrt/tests/string.c  5 Apr 2005 20:33:36 -0000
@@ -22,6 +22,8 @@
 #include "winbase.h"
 #include <string.h>
 #include <stdlib.h>
+#include "mbctype.h"
+#include "mbstring.h"

static void* (*pmemcpy)(void *, const void *, size_t n);
 static int* (*pmemcmp)(void *, const void *, size_t n);
@@ -34,6 +36,10 @@
 {
     void *mem;
     static const char xilstring[]="c:/xilinx";
+
+    static const char s_chin[] = "\326\354"; /* ZHU1 for mbcs tests */
+    static const char s_jap[] = "A\x9A\x8B\xE0\xEF\xF0xXyYzZ";
+
     int nLen=strlen(xilstring);
     HMODULE hMsvcrt = LoadLibraryA("msvcrt.dll");
     ok(hMsvcrt != 0, "LoadLibraryA failed\n");
@@ -46,6 +52,24 @@
     ok(mem != NULL, "memory not allocated for size 0\n");
     strcpy((char*)mem,xilstring);
     pmemcpy((char*)mem+5, mem,nLen+1);
-    ok(pmemcmp((char*)mem+5,xilstring, nLen) == 0,
-       "Got result %s\n",(char*)mem+5);
+    ok(pmemcmp((char*)mem+5,xilstring, nLen) == 0, "Got result %s\n",(char*)mem+5);
+
+    /**
+     * http://www.geocities.com/yongweiwu/multibyte.htm
+     * http://home.a03.itscom.net/tsuzu/programing/tips06_2.htm
+     */
+
+    _setmbcp(936); /* Japanese */
+    ok( 0 == _ismbblead(s_jap[1]), "0 == _ismbblead(s_jap[1]), was %d\n",  _ismbblead(s_jap[1]) );
+
+    _setmbcp(932); /* Chinese GBK */
+    ok( 4 == _ismbblead(s_chin[1]),     "4 == _ismbblead(s_chin[1]), was %d\n",  _ismbblead(s_chin[1]) );
+    ok( 0 == _ismbslead(s_chin, &s_chin[1]), "0 == _ismbslead(s_chin, &s_chin[1]), was %d\n", _ismbslead(s_chin, &s_chin[1]));
+
+    ok( 2 == strlen(s_chin),  "2 == strlen(s_chin), was %d\n", strlen(s_chin));
+    ok( 1 == _mbslen(s_chin), "1 == _mbslen(s_chin), was %d\n", _mbslen(s_chin));
+
+    ok( _MBC_LEAD  == _mbsbtype(s_chin, 0), "_MBC_LEAD  == _mbsbtype(s_chin, 0), was %d\n", _mbsbtype(s_chin, 0) );
+    ok( _MBC_TRAIL == _mbsbtype(s_chin, 1), "_MBC_TRAIL == _mbsbtype(s_chin, 1), was %d\n", _mbsbtype(s_chin, 1) );
+
 }

Regards,
Raphael

[MSVCRT] implement _mbsbtype

[Juan Lang]

Hi Raphael, looking better.  One more point below:

--- Raphael  wrote:
> > Index: dlls/msvcrt/mbcs.c
>
> ===================================================================
> RCS file: /home/wine/wine/dlls/msvcrt/mbcs.c,v
> retrieving revision 1.30
> diff -u -r1.30 mbcs.c
> --- dlls/msvcrt/mbcs.c	25 Feb 2005 14:07:57 -0000	1.30
> +++ dlls/msvcrt/mbcs.c	5 Apr 2005 20:29:51 -0000
> @@ -1143,4 +1144,37 @@
>          str += (MSVCRT_isleadbyte(*str)?2:1);
>      }
>      return NULL;
> +}
> -
>
> +/*********************************************************************
> -
> -
> _mbsbtype (MSVCRT.@)
> - */
>
> +int _mbsbtype(const unsigned char* mbstr, size_t count) {
> - const unsigned char* str;
> - const unsigned char* start = mbstr;
> -
> - str = mbstr + count;
> -
> - /** from _ismbslead */
> - if (MSVCRT___mb_cur_max > 1)
> - {
> - while (start < str) {
> -
>  if (!*start) {
> - return _MBC_ILLEGAL;
> -
>  }
> -
>  start += MSVCRT_isleadbyte(*str) ? 2 : 1;
> - }
> -
> - }
> - if (!*str) { /** TODO: check *str validity */
> - return _MBC_ILLEGAL;
> - }
> - if (start == str && MSVCRT_isleadbyte(*str)) {
> - return _MBC_LEAD;
> - }
> - if (start == str && MSVCRT_isleadbyte(str[-1])) {
> - return _MBC_TRAIL;
> - }

For this last comparison, it's possible that you're indexing before the
array if count is 0.  Also, it seems more natural to check if (start ==
str && !MSVCRT_isleadbyte(*str)) return _MBC_SINGLE, but that would imply
that _MBC_TRAIL is never returned.  A test case would satisfy my
curiosity, at least.

--Juan

__________________________________ 
Yahoo! Messenger 
Show us what our next emoticon should look like. Join the fun. 
http://www.advision.webevents.yahoo.com/emoticontest

[MSVCRT] implement _mbsbtype

[Troy Rollo]

This does not look right:
> +int _mbsbtype(const unsigned char* mbstr, size_t count) {
> - const unsigned char* str;
> - const unsigned char* start = mbstr;
> -
> - str = mbstr + count;
> -
> - /** from _ismbslead */
> - if (MSVCRT___mb_cur_max > 1)
> - {
> - while (start < str) {
> -
>  if (!*start) {
> - return _MBC_ILLEGAL;
> -
>  }
> -
>  start += MSVCRT_isleadbyte(*str) ? 2 : 1;

This should probably be "start += MSVCRT_isleadbyte(*start) ? 2 : 1", BUT...
> - }
> -
> - }
> - if (!*str) { /** TODO: check *str validity */
> - return _MBC_ILLEGAL;
> - }
> - if (start == str && MSVCRT_isleadbyte(*str)) {
> - return _MBC_LEAD;
> - }

This is not safe, because values used for a lead byte can also be used for a 
trailing byte - indeed with your loop (corrected as above), it seems that 
"start" could never be pointing to a trailing byte as you skip over trailing 
bytes.
> - if (start == str && MSVCRT_isleadbyte(str[-1])) {
> - return _MBC_TRAIL;
> - }
> -
> - return _MBC_SINGLE;

Try this:

if (MSVCRT___mb_cur_max > 1)
  {
    while (start < str) {
      if (!*start) {
 return _MBC_ILLEGAL;
      }
      if (MSVCRT_isleadbyte(*start))
      {
        if (str == start)
          return _MBC_LEAD;
        else if (str == start + 1)
          return _MBC_TRAIL;
        start += 2;
      }
      else
      {
        if (str == start)
          return _MBC_SINGLE;
        ++start;
      }
    }
  }
  return _MBC_ILLEGAL;

The tests should probably test for this - of course the test will only work if 
the test knows which code page is set.

[MSVCRT] implement _mbsbtype

[Troy Rollo]

On Wed, 6 Apr 2005 08:58, Troy Rollo wrote:
> while (start < str) {

This should have been "while (start <= str)