Re: Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Shachar Shemesh]

+	   LPWSTR lpText = (LPWSTR)lParam;
+	   lpText[0] = '\0';

Shouldn't it be

+	   LPWSTR lpText = (LPWSTR)lParam;
+	   lpText[0] = L'\0';

?

Carl Sopchak wrote:
> I found this while trying to get Quicken 2000 Deluxe to run under wine.  Q2k
> was crashing when adding a new security (stock) if nothing was seleted in the
> Asset Class ListBox.  (It doesn't do this under "real Win NT"...)  It turns
> out that Q2k seems to keep processing the value returned by WM_GETTEXT
> regardless of the number of characters WM_GETTEXT returns.  Since wine's
> WM_GETTEXT was not returning a NULL string (='\0'), a subsequent strcpy (or
> some such) was choking because the string was not null terminated.
>
> The attached patch sets the return string to '\0' if LB_GETCURSEL returns
> LB_ERR.  (Although this behaviour is not specified on msdn.microsoft.com, I
> figure it can't hurt, since the program calling WM_GETTEXT should be
> expecting the buffer to get changed by the call...)  I don't know (and have
> not checked) to see if other *_GETTEXT message handlers need to do the
> same...
>
> One last point:  I'm fairly new at C coding.  If there's a better way to do
> what this patch does, feel free to improve upon it... <grin>
>
> Carl
> Index: controls/combo.c
>
> RCS file: /home/wine/wine/controls/combo.c,v
> retrieving revision 1.85
> diff -u -r1.85 combo.c
> --- controls/combo.c	10 Jul 2002 23:11:28 -0000	1.85
> +++ controls/combo.c	15 Jul 2002 17:02:47 -0000
> @@ -1603,6 +1603,15 @@
>       return (LRESULT)n;
>        }
>    }
> - /* LB_GETCURSEL returned LB_ERR - truncate string, return zero */
> - if (unicode)
> - {
> -
>   LPWSTR lpText = (LPWSTR)lParam;
> -
>   lpText[0] = '\0';
> - } else {
> -
>   LPSTR lpText = (LPSTR)lParam;
> -
>   lpText[0] = '\0';
> - }
> return 0;
>
> }

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Francois Gouget]

On Mon, 15 Jul 2002, Shachar Shemesh wrote:
> -
>   LPWSTR lpText = (LPWSTR)lParam;
> -
>   lpText[0] = '\0';
>
>
> Shouldn't it be
> -
>   LPWSTR lpText = (LPWSTR)lParam;
> -
>   lpText[0] = L'\0';

It does not matter. Both are equivalent to:
> -
>    lpText[0] = 0;

'\0' is a single byte 0 value (IOW a char) which is expanded to 16bits,
and assigned to the WCHAR. L'\0' is a 32bit 0 Unicode character (a
wchar_t) which is scaled down to 16bits and assigned to the WCHAR. And
the last one is a 32bit integer which too is scaled down to 16bits and
assigned to the WCHAR.

The thing is Unix C compilers don't have 16bit literal Unicode
characters (except in some cases g++ 3.x) so you cannot write code that
does not imvolve a conversion of some sort.

--
Francois Gouget         [e-mail]        http://fgouget.free.fr/
    I haven't lost my mind, it's backed up on tape around here somewhere...

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Shachar Shemesh]

Francois Gouget wrote:
> On Mon, 15 Jul 2002, Shachar Shemesh wrote:
> > -
> >   LPWSTR lpText = (LPWSTR)lParam;
> > -
> >   lpText[0] = '\0';
> >
> >
> > Shouldn't it be
> > -
> >   LPWSTR lpText = (LPWSTR)lParam;
> > -
> >   lpText[0] = L'\0';
>
> It does not matter. Both are equivalent to:
> > -
> >    lpText[0] = 0;
>
> '\0' is a single byte 0 value (IOW a char) which is expanded to 16bits,
> and assigned to the WCHAR. L'\0' is a 32bit 0 Unicode character (a
> wchar_t) which is scaled down to 16bits and assigned to the WCHAR. And
> the last one is a 32bit integer which too is scaled down to 16bits and
> assigned to the WCHAR.
>
> The thing is Unix C compilers don't have 16bit literal Unicode
> characters (except in some cases g++ 3.x) so you cannot write code that
> does not imvolve a conversion of some sort.

Not trying to be a hard head here, and I'm not even remotely trying to 
suggest that this particular line of code is a crucial point. It's just 
that my understanding of this project and the way code should be with 
it, all WINE DLLs are actually WineLib code. All WineLib code should 
duel compile on Windows and Unix. In theory, there should not be a 
problem compiling our sources of USER.DLL on Visual C, and running it on 
Windows instead of the native DLL (far fetched example, I know).

If VC does have a "one correct way", while gcc doesn't, shouldn't we 
prefer the VC correct one?

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Francois Gouget]

On Mon, 15 Jul 2002, Shachar Shemesh wrote:
[...]
> -
>   LPWSTR lpText = (LPWSTR)lParam;
> -
>   lpText[0] = '\0';

[...]
> -
>   lpText[0] = L'\0';

[...]
> -
>    lpText[0] = 0;

[...]
> Not trying to be a hard head here, and I'm not even remotely trying to
> suggest that this particular line of code is a crucial point. It's just
> that my understanding of this project and the way code should be with
> it, all WINE DLLs are actually WineLib code. All WineLib code should
> duel compile on Windows and Unix. In theory, there should not be a
> problem compiling our sources of USER.DLL on Visual C, and running it on
> Windows instead of the native DLL (far fetched example, I know).
>
> If VC does have a "one correct way", while gcc doesn't, shouldn't we
> prefer the VC correct one?

That's a good point. All three ways would work in Visual C++ but as you
pointed out, L'\0' would be the most correct one.

So it would make sense to prefer it when assigning to WCHARs (too bad we
cannot do the same for strings as this would not work in gcc).

--
Francois Gouget         [e-mail]        http://fgouget.free.fr/
May your Tongue stick to the Roof of your Mouth with the Force of a Thousand Caramels.
`

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Uwe Bonnes]

> > > > > "Shachar" == Shachar Shemesh  writes:

Shachar> Not trying to be a hard head here, and I'm not even remotely
    Shachar> trying to suggest that this particular line of code is a
    Shachar> crucial point. It's just that my understanding of this project
    Shachar> and the way code should be with it, all WINE DLLs are actually
    Shachar> WineLib code. All WineLib code should duel compile on Windows
    Shachar> and Unix. In theory, there should not be a problem compiling
    Shachar> our sources of USER.DLL on Visual C, and running it on Windows
    Shachar> instead of the native DLL (far fetched example, I know).

Shachar> If VC does have a "one correct way", while gcc doesn't,
    Shachar> shouldn't we prefer the VC correct one?

With VC++ you either compile an ASCII or an Unicode version. In this case
you know whether to treat a char or string as ascii or unicode. In Wine you
have both ASCII and Unicode in one executable/library. Here you have to be
explicit what you mean.

Bye

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[David Laight]

> If VC does have a "one correct way", while gcc doesn't, shouldn't we 
> prefer the VC correct one?

No - you should the one that is in the C standard.

In this case '0' should be ok under ALL circumstances.
(Unless M$ have completely stuffed their compiler)

David

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Dmitry Timoshkov]

"Shachar Shemesh"  wrote:
> - LPWSTR lpText = (LPWSTR)lParam;
> - lpText[0] = '\0';
>
> Shouldn't it be
> - LPWSTR lpText = (LPWSTR)lParam;
> - lpText[0] = L'\0';
>
> ?

Due to a difference between *nix and win32 unicode character sizes
(4 vs. 2 bytes), personally I prefer to avoid such an ambiguity and
simply use plain 0 instead. But '\0' will work just fine.

IMO Wine should not use L prefix at all and explicitly convert ANSI
to unicode when it's needed or use explicit unicode character constants:
static const WCHAR fooW[] = {'f','o','o',0};
but never L"foo".

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Francois Gouget]

On Tue, 16 Jul 2002, Dmitry Timoshkov wrote:
[...]
> IMO Wine should not use L prefix at all and explicitly convert ANSI
> to unicode when it's needed or use explicit unicode character constants:
> static const WCHAR fooW[] = {'f','o','o',0};
> but never L"foo".

For character constants, both 'x' and L'x' with work. So it is basically
a matter of style/taste.

But for Unicode strings, neither "foo" nor L"foo"  work: one results in
a string with 1 byte character and the other in a string with 4 byte
characters the compiler will not perform any conversion.
So for Unicode string literals, the only option is as you have shown
above:

static const WCHAR fooW[] = {'f','o','o',0};

Clumsy but this is the only way.

(for Winelib we play tricks with WINE_UNICODE_TEXT but these are not
perfect and must not be used in Wine. That's why they are in an #ifndef
__WINE__ anyway)

--
Francois Gouget         [e-mail]        http://fgouget.free.fr/
                              145 = 1! + 4! + 5!

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Shachar Shemesh]

Francois Gouget wrote:
> Clumsy but this is the only way.
>
> (for Winelib we play tricks with WINE_UNICODE_TEXT but these are not
> perfect and must not be used in Wine. That's why they are in an #ifndef
> __WINE__ anyway)

didn't you just spot a serious problem with winelib?

As far as I can see, the only solution will be to formally seperate wine 
from winelib in aims. Wine should support UTF-16, as it handles binaries 
that use UTF-16. Winelib should support UTF-32, as that's what the 
compilers support. We do nasty workarounds for the wine sources, but do 
not require nasty workarounds from the winelib users.

Then again, rebutting my own claims, this will break binary 
compatibility between data files generated with winelib applications and 
windows applications.

Then again, rebutting my own rebuttal, binary compatiblity will not 
happen without explicit winelib users effort anyways, because of 
endianity issues.

So, after having proven my own claims null and invalid, what are we left 
with?

Shachar

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Dmitry Timoshkov]

"Shachar Shemesh"  wrote:
> As far as I can see, the only solution will be to formally seperate wine 
> from winelib in aims. Wine should support UTF-16, as it handles binaries 
> that use UTF-16. Winelib should support UTF-32, as that's what the 
> compilers support. We do nasty workarounds for the wine sources, but do 
> not require nasty workarounds from the winelib users.

Personally I do not consider using explicit unicode strings such as 'f','o',o',0
as nasty workarounds. It's a quite acceptable approach.

For the record: until the very recent time Unicode standard had only 65536
defined characters, i.e. UTF16 covered most of the applications requirements.
Nowadays it has slightly more, but they are not likely to be used too wide,
and there are surrogate pairs for 16-bit unicode.

And what's wrong with using Wine internal unicode library and huge number
of Win32 APIs dealing with wide (W) characters? As far as I understand it,
if Wine would wait till Linux community creates a working unicode implementation
we would still had nothing working at hands. Especially taking into account
a more wide spreading idea of using UTF8 (i.e. 8-bit unicode) instead of
16 or 32 bit version. It's ugly IMO: and performance is the very first argument.

Not to sound offensively, but I don't know (at least) any of the Linux
libraries (except glibc) that has (only) unicode APIs, or require usage of 32-bit
unicode from client applications, or has a slight idea about code pages.
It seems that everyone invents its own wheel, even XFree86 guys invent their
own internal unicode support...

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Francois Gouget]

On Tue, 16 Jul 2002, Dmitry Timoshkov wrote:
[...]
> Personally I do not consider using explicit unicode strings such as 'f','o',o',0
> as nasty workarounds. It's a quite acceptable approach.

'S','u','r','e',' ','i','t',''','s',' ','t','h','e','
','o','n','l','y',' ','u','s','a','b','l','e','
','a','p','p','r','o','a','c','h','.',
'B','u','t',' ','I',' ','s','t','i','l','l',' ','t','h','i','n','k',' ',
'w','r','i','t','i','n','g',' ','s','t','r','i','n','g','s',' ',
'l','i','k','e',' ','t','h','i','s',' ','i','s',' ',
'a','w','f','u','l','.',
'H','e','n','c','e',' ','t','h','e',' ','t','e','r','m',' ',
'k','l','u','d','g','e','.',
';','-',')'

--
Francois Gouget         [e-mail]        http://fgouget.free.fr/
      Any sufficiently advanced bug is indistinguishable from a feature.
                            -- from some indian guy

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Dmitry Timoshkov]

"Francois Gouget"  wrote:
> 'H','e','n','c','e',' ','t','h','e',' ','t','e','r','m',' ',
> 'k','l','u','d','g','e','.',
> ';','-',')'

Especially taking into account that it's very unlikely that
anybody will write such strings using anything except plain
ASCII.

UNICODE_STRING fooW;
char foo[] = "This is my answer in English. It's not a kluge."
             "Это мой ответ на русском. Не такой уж и ужасный :-)";

RtlCreateUnicodeStringFromAsciiz(&fooW, foo);
CreateFileW(fooW.Buffer, ....);

--
Dmitry.

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Francois Gouget]

On Wed, 17 Jul 2002, Dmitry Timoshkov wrote:
> "Francois Gouget"  wrote:
> > 'H','e','n','c','e',' ','t','h','e',' ','t','e','r','m',' ',
> > 'k','l','u','d','g','e','.',
> > ';','-',')'
>
> Especially taking into account that it's very unlikely that
> anybody will write such strings using anything except plain
> ASCII.
>
> UNICODE_STRING fooW;
> char foo[] = "This is my answer in English. It's not a kluge."
>              "üÔÏ ÍÏÊ ÏÔ×ÅÔ ÎÁ ÒÕÓÓËÏÍ. îÅ ÔÁËÏÊ ÕÖ É ÕÖÁÓÎÙÊ :-)";
>
> RtlCreateUnicodeStringFromAsciiz(&fooW, foo);

Yes but that involves a runtime conversion :-(

--
Francois Gouget         [e-mail]        http://fgouget.free.fr/
                  Dieu dit: "M-x Lumière". Et la lumière fut.

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Shachar Shemesh]

Ok. At least have a small util to convert a string to this format.

Sh.

Francois Gouget wrote:
> On Wed, 17 Jul 2002, Dmitry Timoshkov wrote:
> > "Francois Gouget"  wrote:
> > > 'H','e','n','c','e',' ','t','h','e',' ','t','e','r','m',' ',
> > > 'k','l','u','d','g','e','.',
> > > ';','-',')'
> >
> > Especially taking into account that it's very unlikely that
> > anybody will write such strings using anything except plain
> > ASCII.
> >
> > UNICODE_STRING fooW;
> > char foo[] = "This is my answer in English. It's not a kluge."
> >             "üÔÏ ÍÏÊ ÏÔ×ÅÔ ÎÁ ÒÕÓÓËÏÍ. îÅ ÔÁËÏÊ ÕÖ É ÕÖÁÓÎÙÊ :-)";
> >
> > RtlCreateUnicodeStringFromAsciiz(&fooW, foo);
>
> Yes but that involves a runtime conversion :-(
>
> --
> Francois Gouget         [e-mail]        http://fgouget.free.fr/
>                  Dieu dit: "M-x Lumière". Et la lumière fut.

Bug Fix: WM_GETTEXT should return NULL string if LB_GETCURSEL is LB_ERR

[Francois Gouget]

On Tue, 16 Jul 2002, Shachar Shemesh wrote:
> Francois Gouget wrote:
> > Clumsy but this is the only way.
> >
> > (for Winelib we play tricks with WINE_UNICODE_TEXT but these are not
> > perfect and must not be used in Wine. That's why they are in an #ifndef
> > __WINE__ anyway)
>
> didn't you just spot a serious problem with winelib?

Oh, no. I've known and dealt with this issue quite some time ago.

To summarise the situation:

* gcc 2.95: does not support 16bit Unicode at all
 * gcc 3.x: has a -fshort-wchar option that defines wchar_t as an
unsigned short. Unfortunately I am not sure all distributions of gcc 3.x
include this option.

For Wine:
 * it should have some degree of portability across compilers.
 * right now it has to support gcc 2.95
 * using the 'array of chars' approach is acceptable since we write the
code

For Winelib:
 * requiring that Winelib users modify their application to use 32bit
unicode characters is not an option. This is fine for one page
applications but not for applications with hundreds of thousands of
lines of code. Btw, this would require more work than just modifying the
literal string declarations.`
 * it is reasonable to require gcc/g++ 3.x for porting, especially since
it will help a lot for many other C++ features
 * thus we can strongly recommend the use of -fshort-wchar
 * if a Winelib user cannot/does not want to use gcc/g++ 3.x, then we
still have a fallback mechanism that, while it won't work in 100% cases,
will still cover most of the situations. That mechanism uses the _TEXT
macro (which all well written Windows applications should use) to
convert a 32bit Unicode string to a 16bit Unicode string in place at
runtime (which means you have to use -fwritable-strings). See
WINE_UNICODE_REWRITE and the block that follows in 'include/winnt.h'.

--
Francois Gouget         [e-mail]        http://fgouget.free.fr/
  Good judgment comes from experience, and experience comes from bad judgment
                               -- Barry LePatner