Hi folks,
While I looking for something _small_ to do, I discovered that listview.c had two of those HEAP_strdupXXX calls that Alexandre hates so much. I figured that's got to be a 2 minute job, but I was wrong -- it turns out that the listview control needed to get Unicode enabled.
So, after much work and no testing, here is a first try at doing that. The good news is that it seems it compiles. The bad news is that I have no idea how to test it. So any suggestions on how to test the listview control, or any help with the testing is greatly appreciated, as well as any comments on the patch itself.
Alexandre, why is it that you want so badly to get rid of the HEAP_strdup functions? I mean, in certain cases they are not needed, and they rather signal that the code needs fixing, but in other places they _are_ needed and they convey a heck of a lot better what we're trying to do then the strange WideCharXXX function calls.
-- Dimi
"Dimitrie O. Paun" dimi@cs.toronto.edu writes:
Alexandre, why is it that you want so badly to get rid of the HEAP_strdup functions? I mean, in certain cases they are not needed, and they rather signal that the code needs fixing, but in other places they _are_ needed and they convey a heck of a lot better what we're trying to do then the strange WideCharXXX function calls.
The reason is very simple: WideCharXXX are Windows API, HEAP_strdupA/W are not.
If you want an strdup to make the code more readable, you can declare a static one in the file where you need it; but of course this only makes sense if it is used a lot.
On Tue, 8 Jan 2002, Alexandre Julliard wrote:
The reason is very simple: WideCharXXX are Windows API, HEAP_strdupA/W are not.
I have supported your effort for getting rid of the HEAP_strdupA/W calls for this very reason: I strongly believe using the standard API is very important for too many reasons to list here.
However, looking over the code with critical eye, I realized that we are uglifing/obfuscating the code with this change, rather then cleaning it up. As a result, I've been kindda torn between the desire to use a standard API, and the aesthetics of the code base.
Now, thinking about the problem, I realized that the strdupA/W is not a standard Windows API because apps should not typically need it, while it is _clearly_ needed as an internal API for the Win32 API.
What would be wrong with:
LPSTR wine_strdupWtoA(LPCWSTR); LPWSTR wine_strdupAtoW(LPCSTR); void wine_strfreeA(LPSTR); void wine_strfreeW(LPWSTR) exported from, say, ntdll?
Besides, they can remain inlined (as they are now), so they don't really need to be exported from anywhere.
-- Dimi.
"Dimitrie O. Paun" dimi@cs.toronto.edu writes:
What would be wrong with:
LPSTR wine_strdupWtoA(LPCWSTR); LPWSTR wine_strdupAtoW(LPCSTR); void wine_strfreeA(LPSTR); void wine_strfreeW(LPWSTR) exported from, say, ntdll?
Besides, they can remain inlined (as they are now), so they don't really need to be exported from anywhere.
You can just as well put the inline functions directly into the C file, or in some dll private headers. The functions are only 3 lines long, it's no big deal to have a few duplicates. There is no reason to pollute the standard headers with that.
In many cases you could also use RtlCreateUnicodeStringFromAsciiz, which is an exported API and is probably more readable that MultiByteToWideChar etc.
On Tue, 8 Jan 2002, Alexandre Julliard wrote:
You can just as well put the inline functions directly into the C file, or in some dll private headers. The functions are only 3 lines long, it's no big deal to have a few duplicates. There is no reason to pollute the standard headers with that.
Excellent. I was not preaching header pollution :). But if we do that, why not have a wine/heap.h header with the said functions?
In many cases you could also use RtlCreateUnicodeStringFromAsciiz, which is an exported API and is probably more readable that MultiByteToWideChar etc.
Indeed. MultiByteToWideChar is just brutal IMO. But many times if we need to go W->A we also need to go A->W, and hence my proposal for symetrical functions.
-- Dimi.
"Dimitrie O. Paun" dimi@cs.toronto.edu writes:
Excellent. I was not preaching header pollution :). But if we do that, why not have a wine/heap.h header with the said functions?
That's just as much pollution. We must not introduce incompatibilities except where there's clearly no other possibility. In this case there are tons of other options.
Indeed. MultiByteToWideChar is just brutal IMO. But many times if we need to go W->A we also need to go A->W, and hence my proposal for symetrical functions.
The asymmetry could be considered a feature, since people should be encouraged to go A->W instead of W->A wherever possible.
On Tue, 8 Jan 2002, Alexandre Julliard wrote:
That's just as much pollution. We must not introduce incompatibilities except where there's clearly no other possibility. In this case there are tons of other options.
If you insist... :)
Indeed. MultiByteToWideChar is just brutal IMO. But many times if we need to go W->A we also need to go A->W, and hence my proposal for symetrical functions.
The asymmetry could be considered a feature, since people should be encouraged to go A->W instead of W->A wherever possible.
In theory, yes. But you know what Larry McVoy once said:
The difference between theory and practice is that in theory there is no difference, while in practice there is.
Applied to our case, if we have fooW and fooA, then typically fooA looks as such:
fooA() { strdupAtoW fooW strdupWtoA }
So, even if we do go only from A->W at the API level, the actual string conversions do go both ways.
-- Dimi.
"Dimitrie O. Paun" dimi@cs.toronto.edu writes:
Applied to our case, if we have fooW and fooA, then typically fooA looks as such:
fooA() { strdupAtoW fooW strdupWtoA }
So, even if we do go only from A->W at the API level, the actual string conversions do go both ways.
Very few functions would need something like that. The normal case would be:
fooA() { strdupAtoW fooW copy W back into A if needed }
The W->A case is not a strdup, it's a simple WideCharToMultiByte.
Probably at least 90% of the existing calls to HEAP_strdupWtoA are because of a W function calling a A function, which is precisely what should be discouraged.
On Tue, 8 Jan 2002, Alexandre Julliard wrote:
The W->A case is not a strdup, it's a simple WideCharToMultiByte.
Probably at least 90% of the existing calls to HEAP_strdupWtoA are because of a W function calling a A function, which is precisely what should be discouraged.
Good point. I'm convinced. But since we are on the subject, why did you prefer the use of the WideToXXX function instead of the RtlCreateUnicode...Asciiz?
-- Dimi.
"Dimitrie O. Paun" dimi@cs.toronto.edu writes:
Good point. I'm convinced. But since we are on the subject, why did you prefer the use of the WideToXXX function instead of the RtlCreateUnicode...Asciiz?
No real reason, mostly laziness. It was easier to simply copy the 3 lines from HEAP_strdup into the code than to change it to use the Rtl* functions. But if you feel like changing that you are most welcome to do it.
On Tue, 8 Jan 2002, Dimitrie O. Paun wrote: [...]
The bad news is that I have no idea how to test it. So any suggestions on how to test the listview control, or any help with the testing is greatly appreciated, as well as any comments on the patch itself.
I believe you will find the 'Common Control Spy Samples' very useful in testing common controls. You can download them from Microsoft's web site:
http://www.microsoft.com/msj/0998/code/controlspy.exe
You may also want to read the following article: "Control Spy Exposes the Clandestine Life of Windows Common Controls" * Part I http://www.microsoft.com/msj/0798/controlspy.htm * Part II http://www.microsoft.com/msj/0998/control/control.htm * Part III http://www.microsoft.com/msj/1298/controlspy3/controlspy3.htm
(from http://wine.codeweavers.com/bugzilla/show_bug.cgi?id=239)
-- Francois Gouget fgouget@free.fr http://fgouget.free.fr/ La terre est une bêta...
-
Alexandre Julliard -
Dimitrie O. Paun -
Francois Gouget