Re: [RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Detlef Riekenberg]

On Mi, 2007-04-04 at 10:36 +0900, Byeong-Sik Jeon wrote:
> - /* convert from WCHAR size to maximum required TCHAR size */
> - max_val_name_len *= sizeof(WCHAR) / sizeof(TCHAR);
> - valName = HeapAlloc(GetProcessHeap(), 0, max_val_name_len *
>
> sizeof(TCHAR));
>      valBuf = HeapAlloc(GetProcessHeap(), 0, max_val_size);
>      if (RegQueryValueEx(hKey, NULL, NULL, &valType, valBuf, &valSize)
> == ERROR_FILE_NOT_FOUND) {

This looks wrong to me:
TCHAR is WCHAR, when compiled with UNICODE, otherwise TCHAR is CHAR
The code does already handle the different character-size
in HeapAlloc for varName.

When the app must handle the case, that 
sizeof(string) can be > "strlen(string) + sizeof(0)",
then we should convert the app to use UNICODE everywhere.

For the last quoted line, valSize is not initialized!
(RegQueryInfoKey wrote the result to max_val_size)

Just checked Coverity: not detected

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Byeong-Sik Jeon]

Hi,

Currently regedit is not defined "UNICODE".

The point are "max_val_name_len", "valName", "valNameLen".
Not max_val_size, valSize.

RegQueryInfoKey set the "max_val_name_len" to the size of the longest
value name, in characters. This value is not required byte size.

N character string:
==> w/  UNICODE, N WHAR string.
==> w/o UNICODE, N ~ 2N unsigned char string.

Please, test the CJK locale. and See the "IDS_NEWKEY, IDS_NEWVALUE" in
Ja.rc or Ko.rc .
If we create the registry "value name" in the empty registry key,
we can't see any newly created reg value name.

Thanks.

Detlef Riekenberg wrote:
> On Mi, 2007-04-04 at 10:36 +0900, Byeong-Sik Jeon wrote:
> > - /* convert from WCHAR size to maximum required TCHAR size */
> > - max_val_name_len *= sizeof(WCHAR) / sizeof(TCHAR);
> > - valName = HeapAlloc(GetProcessHeap(), 0, max_val_name_len *
> >
> > sizeof(TCHAR));
> >      valBuf = HeapAlloc(GetProcessHeap(), 0, max_val_size);
> >      if (RegQueryValueEx(hKey, NULL, NULL, &valType, valBuf, &valSize)
> > == ERROR_FILE_NOT_FOUND) {
>
> This looks wrong to me:
> TCHAR is WCHAR, when compiled with UNICODE, otherwise TCHAR is CHAR
> The code does already handle the different character-size
> in HeapAlloc for varName.
>
> When the app must handle the case, that 
> sizeof(string) can be > "strlen(string) + sizeof(0)",
> then we should convert the app to use UNICODE everywhere.
>
> For the last quoted line, valSize is not initialized!
> (RegQueryInfoKey wrote the result to max_val_size)
>
> Just checked Coverity: not detected

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Bill Medland]

On Thu, 2007-05-04 at 02:18 +0900, Byeong-Sik Jeon wrote:
> Hi,
>
> Currently regedit is not defined "UNICODE".
>
> The point are "max_val_name_len", "valName", "valNameLen".
> Not max_val_size, valSize.
>
> RegQueryInfoKey set the "max_val_name_len" to the size of the longest
> value name, in characters. This value is not required byte size.

but the call to HeapAlloc multiplies by sizeof TCHAR already.
> N character string:
> ==> w/  UNICODE, N WHAR string.
> ==> w/o UNICODE, N ~ 2N unsigned char string.
>
> Please, test the CJK locale. and See the "IDS_NEWKEY, IDS_NEWVALUE" in
> Ja.rc or Ko.rc .
> If we create the registry "value name" in the empty registry key,
> we can't see any newly created reg value name.
>
> Thanks.
>
> Detlef Riekenberg wrote:
> > On Mi, 2007-04-04 at 10:36 +0900, Byeong-Sik Jeon wrote:
> > > - /* convert from WCHAR size to maximum required TCHAR size */
> > > - max_val_name_len *= sizeof(WCHAR) / sizeof(TCHAR);
> > > - valName = HeapAlloc(GetProcessHeap(), 0, max_val_name_len *
> > >
> > > sizeof(TCHAR));

See!
> > >  valBuf = HeapAlloc(GetProcessHeap(), 0, max_val_size);
> > >  if (RegQueryValueEx(hKey, NULL, NULL, &valType, valBuf, &valSize)
> > >
> > > == ERROR_FILE_NOT_FOUND) {
> >
> > This looks wrong to me:
> > TCHAR is WCHAR, when compiled with UNICODE, otherwise TCHAR is CHAR
> > The code does already handle the different character-size
> > in HeapAlloc for varName.
> >
> > When the app must handle the case, that 
> > sizeof(string) can be > "strlen(string) + sizeof(0)",
> > then we should convert the app to use UNICODE everywhere.
> >
> > For the last quoted line, valSize is not initialized!
> > (RegQueryInfoKey wrote the result to max_val_size)
> >
> > Just checked Coverity: not detected

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Byeong-Sik Jeon]

Bill Medland wrote:
> On Thu, 2007-05-04 at 02:18 +0900, Byeong-Sik Jeon wrote:
> > Hi,
> >
> > Currently regedit is not defined "UNICODE".
> >
> > The point are "max_val_name_len", "valName", "valNameLen".
> > Not max_val_size, valSize.
> >
> > RegQueryInfoKey set the "max_val_name_len" to the size of the longest
> > value name, in characters. This value is not required byte size.
>
> but the call to HeapAlloc multiplies by sizeof TCHAR already.
> > N character string:
> > ==> w/  UNICODE, N WHAR string.
> > ==> w/o UNICODE, N ~ 2N unsigned char string.
> >
> > Please, test the CJK locale. and See the "IDS_NEWKEY, IDS_NEWVALUE" in
> > Ja.rc or Ko.rc .
> > If we create the registry "value name" in the empty registry key,
> > we can't see any newly created reg value name.
> >
> > Thanks.
> >
> > Detlef Riekenberg wrote:
> > > On Mi, 2007-04-04 at 10:36 +0900, Byeong-Sik Jeon wrote:
> > > > - /* convert from WCHAR size to maximum required TCHAR size */
> > > > - max_val_name_len *= sizeof(WCHAR) / sizeof(TCHAR);
> > > > - valName = HeapAlloc(GetProcessHeap(), 0, max_val_name_len *
> > > >
> > > > sizeof(TCHAR));
>
> See!

Without UNICODE, sizeof(TCHAR) == 1. This is no effect. CJK multibyte
character is 2 byte size.

example:
string "새 값 #1" has 6 characters. It is  max_val_name_len.
w/  UNICODE, this string is 6 * sizeof(WCHAR) bytes.
w/o UNICODE, this string is 8 bytes.
             8 == space(1) * 2 + #(1) + 1(1) + 새(2) + 값(2)

If you can't non englidh character, please install following font:
"Arial Unicode"

I'm sorry my poor english. but it is true.
Thanks.

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Byeong-Sik Jeon]

Bill Medland wrote:
> On Thu, 2007-05-04 at 02:18 +0900, Byeong-Sik Jeon wrote:
> > Hi,
> >
> > Currently regedit is not defined "UNICODE".
> >
> > The point are "max_val_name_len", "valName", "valNameLen".
> > Not max_val_size, valSize.
> >
> > RegQueryInfoKey set the "max_val_name_len" to the size of the longest
> > value name, in characters. This value is not required byte size.
>
> but the call to HeapAlloc multiplies by sizeof TCHAR already.
> > N character string:
> > ==> w/  UNICODE, N WHAR string.
> > ==> w/o UNICODE, N ~ 2N unsigned char string.
> >
> > Please, test the CJK locale. and See the "IDS_NEWKEY, IDS_NEWVALUE" in
> > Ja.rc or Ko.rc .
> > If we create the registry "value name" in the empty registry key,
> > we can't see any newly created reg value name.
> >
> > Thanks.
> >
> > Detlef Riekenberg wrote:
> > > On Mi, 2007-04-04 at 10:36 +0900, Byeong-Sik Jeon wrote:
> > > > - /* convert from WCHAR size to maximum required TCHAR size */
> > > > - max_val_name_len *= sizeof(WCHAR) / sizeof(TCHAR);
> > > > - valName = HeapAlloc(GetProcessHeap(), 0, max_val_name_len *
> > > >
> > > > sizeof(TCHAR));
>
> See!

Without UNICODE, sizeof(TCHAR) == 1. This is no effect. CJK multibyte
character is 2 byte size.

example:
string "새 값 #1" has 6 characters. It is  max_val_name_len.
w/  UNICODE, this string is 6 * sizeof(WCHAR) bytes.
w/o UNICODE, this string is 8 bytes.
             8 == space(1) * 2 + #(1) + 1(1) + 새(2) + 값(2)

Currently regedit compiled without UNICODE.
RegQueryInfoKey result: max_val_name_len ===> 6;
max_val_name_len++  ===> 7
max_val_name_len * sizeof(TCHAR) ==> 7 * 1
This is bad result. we need 9 or more.

I'm sorry my poor english. but it is true.
Thanks.

PS)
If you can't see non english character, please install font "Arial Unicode".

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Bill Medland]

On Thu, 2007-05-04 at 06:17 +0900, Byeong-Sik Jeon wrote:
> Bill Medland wrote:
> > On Thu, 2007-05-04 at 02:18 +0900, Byeong-Sik Jeon wrote:
> > > Hi,
> > >
> > > Currently regedit is not defined "UNICODE".
> > >
> > > The point are "max_val_name_len", "valName", "valNameLen".
> > > Not max_val_size, valSize.
> > >
> > > RegQueryInfoKey set the "max_val_name_len" to the size of the longest
> > > value name, in characters. This value is not required byte size.
> >
> > but the call to HeapAlloc multiplies by sizeof TCHAR already.
> > > N character string:
> > > ==> w/  UNICODE, N WHAR string.
> > > ==> w/o UNICODE, N ~ 2N unsigned char string.
> > >
> > > Please, test the CJK locale. and See the "IDS_NEWKEY, IDS_NEWVALUE" in
> > > Ja.rc or Ko.rc .
> > > If we create the registry "value name" in the empty registry key,
> > > we can't see any newly created reg value name.
> > >
> > > Thanks.
> > >
> > > Detlef Riekenberg wrote:
> > > > On Mi, 2007-04-04 at 10:36 +0900, Byeong-Sik Jeon wrote:
> > > > > - /* convert from WCHAR size to maximum required TCHAR size */
> > > > > - max_val_name_len *= sizeof(WCHAR) / sizeof(TCHAR);
> > > > > - valName = HeapAlloc(GetProcessHeap(), 0, max_val_name_len *
> > > > >
> > > > > sizeof(TCHAR));
> >
> > See!
>
> Without UNICODE, sizeof(TCHAR) == 1. This is no effect. CJK multibyte
> character is 2 byte size.

Yes, but you are getting confused by the term 'character'.  It is being
used in two different senses.  You are using the wrong method to solve
the problem.  You are fixing it where it is not broken; it is (probably)
broken somewhere else.

AFAIK The value returned by RegQueryInfoKey is (supposed to be) the
number of TCHARS required to hold the name.  MSDN means TCHARS when it
says 'character' for this function.  It does not matter that it actually
requires two (char) TCHARS to hold a (language) character.  After all in
UTF-8 it could require even more than two.
> example:
> string "새 값 #1" has 6 characters. It is  max_val_name_len.
> w/  UNICODE, this string is 6 * sizeof(WCHAR) bytes.
> w/o UNICODE, this string is 8 bytes.
>              8 == space(1) * 2 + #(1) + 1(1) + 새(2) + 값(2)
>
> Currently regedit compiled without UNICODE.
> RegQueryInfoKey result: max_val_name_len ===> 6;
> max_val_name_len++  ===> 7
> max_val_name_len * sizeof(TCHAR) ==> 7 * 1
> This is bad result. we need 9 or more.

So what you have to investigate is why RegQueryInfoKey is returning a
number that is too small.  It should return the number of TCHARS, not
the number of language characters (I believe)
> I'm sorry my poor english. but it is true.
> Thanks.
>
> PS)
> If you can't see non english character, please install font "Arial Unicode".

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Byeong-Sik Jeon]

Bill Medland wrote:
> On Thu, 2007-05-04 at 06:17 +0900, Byeong-Sik Jeon wrote:
> > Without UNICODE, sizeof(TCHAR) == 1. This is no effect. CJK multibyte
> > character is 2 byte size.
>
> Yes, but you are getting confused by the term 'character'.  It is being
> used in two different senses.  You are using the wrong method to solve
> the problem.  You are fixing it where it is not broken; it is (probably)
> broken somewhere else.
>
> AFAIK The value returned by RegQueryInfoKey is (supposed to be) the
> number of TCHARS required to hold the name.  MSDN means TCHARS when it
> says 'character' for this function.  It does not matter that it actually
> requires two (char) TCHARS to hold a (language) character.  After all in
> UTF-8 it could require even more than two.
> > example:
> > string "새 값 #1" has 6 characters. It is  max_val_name_len.
> > w/  UNICODE, this string is 6 * sizeof(WCHAR) bytes.
> > w/o UNICODE, this string is 8 bytes.
> >              8 == space(1) * 2 + #(1) + 1(1) + 새(2) + 값(2)
> >
> > Currently regedit compiled without UNICODE.
> > RegQueryInfoKey result: max_val_name_len ===> 6;
> > max_val_name_len++  ===> 7
> > max_val_name_len * sizeof(TCHAR) ==> 7 * 1
> > This is bad result. we need 9 or more.
>
> So what you have to investigate is why RegQueryInfoKey is returning a
> number that is too small.  It should return the number of TCHARS, not
> the number of language characters (I believe)

1. MSDN means TCHARS when it says 'character' for this function.
   ==> No.
2. why RegQueryInfoKey is returning a number that is too small
   ==> No. Currently RegQueryInfoKey returns the right values.

In SBCS or w/ UNICODE case, you are right.
But this case is DBCS. Some characters in a DBCS have a single byte code
value and some have a double byte code value. <== from MSDN

MSDN use "in characters", "in bytes", "in TCHARS", "numbers of wide
characters" representation. it's different.

Did you test the "regedit" in the Japanese or Korean locale? This is
really bug.
If we create the registry value name in the empty registry key,
we can't see any newly created reg value name ( because of RegEnumValue
function failed).

Of cause, we can make another solution to fix this problem.
We can check the return value of RegEnumValue function. but the patch
will complicate more. We just need more big buffer.

another soultion:
 * we can change the "IDS_NEWKEY, IDS_NEWVALUE"  of resource file.
 * define the UNICODE
 but these sulution don't fix the regedit's bug. just bug hide...

Thanks.

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Byeong-Sik Jeon]

Byeong-Sik Jeon wrote:
> Bill Medland wrote:
> > On Thu, 2007-05-04 at 06:17 +0900, Byeong-Sik Jeon wrote:
> > > Without UNICODE, sizeof(TCHAR) == 1. This is no effect. CJK multibyte
> > > character is 2 byte size.
> >
> > Yes, but you are getting confused by the term 'character'.  It is being
> > used in two different senses.  You are using the wrong method to solve
> > the problem.  You are fixing it where it is not broken; it is (probably)
> > broken somewhere else.
> >
> > AFAIK The value returned by RegQueryInfoKey is (supposed to be) the
> > number of TCHARS required to hold the name.  MSDN means TCHARS when it
> > says 'character' for this function.  It does not matter that it actually
> > requires two (char) TCHARS to hold a (language) character.  After all in
> > UTF-8 it could require even more than two.
> > > example:
> > > string "새 값 #1" has 6 characters. It is  max_val_name_len.
> > > w/  UNICODE, this string is 6 * sizeof(WCHAR) bytes.
> > > w/o UNICODE, this string is 8 bytes.
> > >              8 == space(1) * 2 + #(1) + 1(1) + 새(2) + 값(2)
> > >
> > > Currently regedit compiled without UNICODE.
> > > RegQueryInfoKey result: max_val_name_len ===> 6;
> > > max_val_name_len++  ===> 7
> > > max_val_name_len * sizeof(TCHAR) ==> 7 * 1
> > > This is bad result. we need 9 or more.
> >
> > So what you have to investigate is why RegQueryInfoKey is returning a
> > number that is too small.  It should return the number of TCHARS, not
> > the number of language characters (I believe)
> - MSDN means TCHARS when it says 'character' for this function.
> ==> No.
> - why RegQueryInfoKey is returning a number that is too small
> ==> No. Currently RegQueryInfoKey returns the right values.

I'm sorry.
I'm not yet test the MS-Windows's RegQueryInfoKey function.

I'm sorry again.
Thank you.

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Byeong-Sik Jeon]

Hi,

MS-Windows's RegQueryKeyInfo function test result:

1. MSDN means TCHARS when it says 'character' for this function.
    ==> No.
2. why RegQueryInfoKey is returning a number that is too small
    ==> No. Currently Wine's RegQueryInfoKey set the right values.

Thank you.

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Robert Shearman]

Byeong-Sik Jeon wrote:
> Hi,
>
> MS-Windows's RegQueryKeyInfo function test result:
> - MSDN means TCHARS when it says 'character' for this function.
>  ==> No.
> - why RegQueryInfoKey is returning a number that is too small
>  ==> No. Currently Wine's RegQueryInfoKey set the right values.

Sorry, but I think you are wrong. Every Windows function (except for 
CharNext and CharPrev) that mentions "character" actually means TCHARs.

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Byeong-Sik Jeon]

Robert Shearman wrote:
> Byeong-Sik Jeon wrote:
> > Hi,
> >
> > MS-Windows's RegQueryKeyInfo function test result:
> > - MSDN means TCHARS when it says 'character' for this function.
> >  ==> No.
> > - why RegQueryInfoKey is returning a number that is too small
> >  ==> No. Currently Wine's RegQueryInfoKey set the right values.
>
> Sorry, but I think you are wrong. Every Windows function (except for 
> CharNext and CharPrev) that mentions "character" actually means TCHARs.

Hi,

This is different situation.
Please see the RegQueryInfoKey{A|W}, NtQueryKey, wineserver's reg source
code. 
RegQueryInfoKey{A|W} can't process the string.

Thank you.

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Byeong-Sik Jeon]

Robert Shearman wrote:
> Byeong-Sik Jeon wrote:
> > Hi,
> >
> > MS-Windows's RegQueryKeyInfo function test result:
> > - MSDN means TCHARS when it says 'character' for this function.
> >  ==> No.
> > - why RegQueryInfoKey is returning a number that is too small
> >  ==> No. Currently Wine's RegQueryInfoKey set the right values.
>
> Sorry, but I think you are wrong. Every Windows function (except for 
> CharNext and CharPrev) that mentions "character" actually means TCHARs.

Thank you. you are right.

1. MSDN means TCHARS when it says 'in characters'
   ==> Yes. I'm sorry to everyboby.

2. MSDN means TCHARS when it says 'in characters' for RegQueryInfoKey's
lpcMaxValueNameLen parameter.
   ==> No, I think this is missed MSDN documentation.
       RegQueryInfoKey don't process the 'registry valuename string'

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Bill Medland]

On Thu, 2007-05-04 at 12:03 +0900, Byeong-Sik Jeon wrote:
> Hi,
>
> MS-Windows's RegQueryKeyInfo function test result:
> - MSDN means TCHARS when it says 'character' for this function.
>  ==> No.
> - why RegQueryInfoKey is returning a number that is too small
>  ==> No. Currently Wine's RegQueryInfoKey set the right values.
>
> Thank you.

Wow!!

I confess that I have not tested this.  I think it is sufficiently
counterintuitive that it would be useful if you could submit a test that
would demonstrate it, e.g. put a non-SBCS string into the registry in
Unicode, query and get it back out, demonstrating that the size returned
is the number of lexical characters (plus 1 in Win95/98/me?).

And if this is the case then your original fix is still incorrect; we
would need to multiply the size not by sizeof(TCHAR) but by the maximum
number of bytes required to hold a lexical character (which is
presumably going to have to handle UTF-8).

Bill

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Byeong-Sik Jeon]

Bill Medland wrote:
> I confess that I have not tested this.  I think it is sufficiently
> counterintuitive that it would be useful if you could submit a test that
> would demonstrate it, e.g. put a non-SBCS string into the registry in
> Unicode, query and get it back out, demonstrating that the size returned
> is the number of lexical characters (plus 1 in Win95/98/me?).
>
> And if this is the case then your original fix is still incorrect; we
> would need to multiply the size not by sizeof(TCHAR) but by the maximum
> number of bytes required to hold a lexical character (which is
> presumably going to have to handle UTF-8).

Hi,

I cancel this patch. Please see the "Alexandre Julliard"'s comments.
The good solution of this problem is "#define UNICODE".

Thank you.

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Alexandre Julliard]

Byeong-Sik Jeon  writes:
> another soultion:
> - we can change the "IDS_NEWKEY, IDS_NEWVALUE"  of resource file.
> - define the UNICODE
>
> but these sulution don't fix the regedit's bug. just bug hide...

What we should do for regedit, and actually for all our programs, is
to convert them to be explicitly Unicode throughout, and build them
with #define WINE_NO_UNICODE_MACROS to enforce that there are no
TCHARs anywhere. That would be a nice janitorial task...

[RESEND] regedit: Convert from WCHAR size to maximum required TCHAR size.

[Byeong-Sik Jeon]

Alexandre Julliard wrote:
> Byeong-Sik Jeon  writes:
> > another soultion:
> > - we can change the "IDS_NEWKEY, IDS_NEWVALUE"  of resource file.
> > - define the UNICODE
> >
> > but these sulution don't fix the regedit's bug. just bug hide...
>
> What we should do for regedit, and actually for all our programs, is
> to convert them to be explicitly Unicode throughout, and build them
> with #define WINE_NO_UNICODE_MACROS to enforce that there are no
> TCHARs anywhere. That would be a nice janitorial task...

Ah!!!  It's a great point.
I checked TCHAR, UNICODE, WINE_NO_UNICODE_MACROS, __WINESRC__, ... in
header files. Thank you.

First step for regedit:
regedit must to be build with UNICODE. And, regedit need many fix.

Thanks to all...