"Jonathan Ernst" <Jonathan(a)ErnstFamily.ch> wrote:
With some help I'll be able to improve it (more unicodification, etc.) when I'll come back.
You can't really convert to unicode partially, you have to do it in one go. That not only will simplify things a lot, but also will save you a lot of time.
+ for (i=0; i < numentries; i++) + { + len = WideCharToMultiByte(CP_UNIXCP, 0, entries[i].descr, -1, NULL, 0, NULL, NULL); + descr = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR)); + WideCharToMultiByte(CP_UNIXCP, 0, entries[i].descr, -1, descr, len, NULL, NULL); + printf("%s|||%s\n", entries[i].key, descr);
There is no need to allocate len * sizeof(WCHAR) bytes, len works just fine.
+int wmain(int argc, char *argv[])
wmain takes 'WCHAR *argv[]' list. All remaining problems are caused by the fact that now argv[] array is passed in unicode but you still handle argv[] strings as ASCII and pass them around to internal functions which accept 'char *' not 'WCHAR *' strings. -- Dmitry.
Le lundi 28 février 2005 à 09:10 +0800, Dmitry Timoshkov a écrit :
"Jonathan Ernst" <Jonathan(a)ErnstFamily.ch> wrote:
With some help I'll be able to improve it (more unicodification, etc.) when I'll come back.
You can't really convert to unicode partially, you have to do it in one go. That not only will simplify things a lot, but also will save you a lot of time.
Ok, I will continue to work on it then. I'm sorry to require that much help, but my first idea was just to add internationalization to the uninstaller, then I was asked to move from window to dialog and then from ascii to unicode and that's another cup of tea ;-)
+ for (i=0; i < numentries; i++) + { + len = WideCharToMultiByte(CP_UNIXCP, 0, entries[i].descr, -1, NULL, 0, NULL, NULL); + descr = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR)); + WideCharToMultiByte(CP_UNIXCP, 0, entries[i].descr, -1, descr, len, NULL, NULL); + printf("%s|||%s\n", entries[i].key, descr);
There is no need to allocate len * sizeof(WCHAR) bytes, len works just fine.
Ok Fixed.
+int wmain(int argc, char *argv[])
wmain takes 'WCHAR *argv[]' list.
Fixed as well.
All remaining problems are caused by the fact that now argv[] array is passed in unicode but you still handle argv[] strings as ASCII and pass them around to internal functions which accept 'char *' not 'WCHAR *' strings.
I tried to fix every char->WCHAR I though was worth it; please have a second look at it. The problem now with unicode is that when I try to strstr my token with the element of the list (entries[i].descr) it works only with the first letter (before to use unicode it was working very well). You can see the same problem (one letter .descr) when using the unintaller with --list. The second problem is that since I use unicode for the argvs, whatever argv I give I get the --list branch. Thanks everyone for your help.
"Jonathan Ernst" <Jonathan(a)ErnstFamily.ch> wrote:
I tried to fix every char->WCHAR I though was worth it; please have a second look at it.
The problem now with unicode is that when I try to strstr my token with the element of the list (entries[i].descr) it works only with the first letter (before to use unicode it was working very well). You can see the same problem (one letter .descr) when using the unintaller with --list.
The second problem is that since I use unicode for the argvs, whatever argv I give I get the --list branch.
Your main mistake is that you are trying to handle ASCII strings with "old" APIs, you can't do that. Unicode requires a completely different set of APIs, and win32 provides one.
+ /* + TELL ME: IS IT STILL NEEDED WITH UNICODE ??? len = WideCharToMultiByte(CP_UNIXCP, 0, entries[i].descr, -1, NULL, 0, NULL, NULL);
As I have said it already, you need to convert unicode strings to the encoding of the underlying system before printing strings on console. CP_UNIXCP is supposed to do exactly that task.
+ descr = HeapAlloc(GetProcessHeap(), 0, len); + WideCharToMultiByte(CP_UNIXCP, 0, entries[i].descr, -1, descr, len, NULL, NULL); */ + printf("%s|||%s\n", entries[i].key, entries[i].descr); + /*HeapFree(GetProcessHeap(), 0, descr);*/
You have to HeapFree a HeapAlloc'ed memory block. It's better to use W versions of APIs even if it's not strictly required.
+ if (RegOpenKeyExA(HKEY_LOCAL_MACHINE, REGSTR_PATH_UNINSTALL, 0, KEY_READ, &hkeyUninst) != ERROR_SUCCESS)
RegOpenKeyExW in this case.
{ - MessageBox(0, "Uninstall registry key not available (yet), nothing to do !", appname, MB_OK); - return 0; + MessageBox(0, sRegistryKeyNotAvailable, sAppName, MB_OK);
MessageBoxW in this case.
if (!entries) - entries = HeapAlloc(GetProcessHeap(), 0, sizeof(uninst_entry)); + entries = HeapAlloc(GetProcessHeap(), 0, sizeof(uninst_entry));
Quite strange formatting.
strcpy(key_app, REGSTR_PATH_UNINSTALL); strcat(key_app, "\\"); p = key_app+strlen(REGSTR_PATH_UNINSTALL)+1;
You shouldn't use string APIs supposed to handle ASCII strings. Use lstrcpyW,lstrcatW and lstrlenW in this and all other cases. sizeOfSubKeyName = 255; - for ( i=0; - RegEnumKeyExA( hkeyUninst, i, subKeyName, &sizeOfSubKeyName, - NULL, NULL, NULL, NULL ) != ERROR_NO_MORE_ITEMS; - ++i ) + for (i=0; RegEnumKeyExA( hkeyUninst, i, subKeyName, &sizeOfSubKeyName, NULL, NULL, NULL, NULL ) != ERROR_NO_MORE_ITEMS; ++i) RegEnumKeyExW in this case.
+ static const WCHAR DisplayNameW[] = {'D','i','s','p','l','a','y','N','a','m','e',0}; + strcpy(p, subKeyName);
lstrcpyW
+ RegOpenKeyExA(HKEY_LOCAL_MACHINE, key_app, 0, KEY_READ, &hkeyApp);
RegOpenKeyExW
+ res = CreateProcess(NULL, entries[i].command, NULL, NULL, FALSE, 0, NULL, NULL, &si, &info);
CreateProcessW
+ prevsel = SendMessage(hList, LB_GETCURSEL, 0, 0);
SendMessageW and many,many,many others. Make sure to pass correct arguments to all the changed APIs. For better understanding of what is going on add the following as the very first line in main.c: #define __WINESRC__ -- Dmitry.
Le mardi 08 mars 2005 à 18:04 +0800, Dmitry Timoshkov a écrit : [...]
and many,many,many others. Make sure to pass correct arguments to all the changed APIs.
I fixed/changed many things regarding unicode. It seems that now both outputing to the console and case insensitive find as you type feature (re-)work. I fixed most warnings, but there are still 4 lines that I couldn't fix (173, 206, 215, 429). Can someone help me ? Last but not least, since these changes the uninstaller crashes (very) often. I have an unhandled exception when retrieving some apps from the uninstall key and I don't know what cause this: err:seh:EXC_DefaultHandling Unhandled exception code c0000005 flags 0 addr 0x77ecf5d4
For better understanding of what is going on add the following as the very first line in main.c:
#define __WINESRC__
When adding this line the compilation fails with this message: main.c:194: error: `STARTUPINFO' undeclared (first use in this function) Are some include(s) missing ? ... Do you think I'm near the target ?
"Jonathan Ernst" <Jonathan(a)ErnstFamily.ch> wrote:
I fixed/changed many things regarding unicode. It seems that now both outputing to the console and case insensitive find as you type feature (re-)work.
I fixed most warnings, but there are still 4 lines that I couldn't fix (173,
cast entries[numentries-1].command to LPBYTE as you do 3 lines earlier.
206,
wrong STARTUPINFO declaration, you need a unicode one with W suffix.
215,
you can't use sprintf to print to a WCHAR string, use win32 API wsprintfW instead.
429). Can someone help me ?
slen = strlenW(String), plen = strlenW(Pattern);
better use official API lstrlenW.
/* find start of pattern in string */ while (toupper(*start) != toupper(*Pattern))
You can't use toupper with unicode strings. I can't suggest a win32 equivalent from the top of my head, you need to find out.
pptr = (WCHAR *)Pattern;
There is no need for a cast here.
while (toupper(*sptr) == toupper(*pptr))
same as above.
Last but not least, since these changes the uninstaller crashes (very) often. I have an unhandled exception when retrieving some apps from the uninstall key and I don't know what cause this:
err:seh:EXC_DefaultHandling Unhandled exception code c0000005 flags 0 addr 0x77ecf5d4
Very likely a common error happening often in the process of unicodification: buffer overflow due to not a correct memory allocation, or a not correct initialization of a variable pointing to a buffer length (size of buffer in bytes vs. WCHARs).
For better understanding of what is going on add the following as the very first line in main.c:
#define __WINESRC__
When adding this line the compilation fails with this message:
main.c:194: error: `STARTUPINFO' undeclared (first use in this function)
Are some include(s) missing ?
That's the point. I'd prefer that you could figure out that on your own. Hint: look at the headers, and you will see that all those STARTUPINFO, lstrcpy, RegOpenKey are not the real API names, but just macros depending whether UNICODE is defined or not. In Wine using API macros is deprecated, personally I'd prefer to not use them in programs/* either.
Do you think I'm near the target ?
Definitely very close. -- Dmitry.
Le mardi 08 mars 2005 à 23:29 +0800, Dmitry Timoshkov a écrit :
"Jonathan Ernst" <Jonathan(a)ErnstFamily.ch> wrote:
[...]
Thanks, fixed.
You can't use toupper with unicode strings. I can't suggest a win32 equivalent from the top of my head, you need to find out.
It seems that CharUpperW works well. [...]
Last but not least, since these changes the uninstaller crashes (very) often. I have an unhandled exception when retrieving some apps from the uninstall key and I don't know what cause this:
err:seh:EXC_DefaultHandling Unhandled exception code c0000005 flags 0 addr 0x77ecf5d4
Very likely a common error happening often in the process of unicodification: buffer overflow due to not a correct memory allocation, or a not correct initialization of a variable pointing to a buffer length (size of buffer in bytes vs. WCHARs).
Thanks for the hint. I found the allocation problem. [...] Fixed Now everything works; thanks for your step by step help; I hope that what I'me learning now will let me make more additions to Wine ! But: It is even possible to uninstall more than one app at a time, but I wish that when an uninstaller has finished to uninstall an application the list is updated, but it's not. What's and where's a clean way to do it ? Please find attached main.c's diff. Jonathan P.S. Do I really have to split this new uninstaller in different patches as nearly everything changed ?
"Jonathan Ernst" <Jonathan(a)ErnstFamily.ch> wrote:
Thanks for the hint. I found the allocation problem.
Here is another one:
+ int len = GetWindowTextLengthW(GetDlgItem(hwnd, IDC_FILTER)); + if(len > 0) + { + sFilter = (WCHAR*)GlobalAlloc(GPTR, len + 1); + GetDlgItemTextW(hwnd, IDC_FILTER, sFilter, len + 1);
And this one:
+ /* Load MessageBox's strings */ + LoadStringW(hInst, IDS_APPNAME, sAppName, sizeof(sAppName)); + LoadStringW(hInst, IDS_ABOUTTITLE, sAboutTitle, sizeof(sAboutTitle)); + LoadStringW(hInst, IDS_ABOUT, sAbout, sizeof(sAbout)); + LoadStringW(hInst, IDS_REGISTRYKEYNOTAVAILABLE, sRegistryKeyNotAvailable, sizeof(sRegistryKeyNotAvailable)); + LoadStringW(hInst, IDS_UNINSTALLFAILED, sUninstallFailed, sizeof(sUninstallFailed));
LoadStringW takes number of WCHARs, not bytes. Everything else seems to be good enough.
P.S. Do I really have to split this new uninstaller in different patches as nearly everything changed ?
It's up to Alexandre to decide, just send your work to wine-patches. -- Dmitry.
P.S. Do I really have to split this new uninstaller in different patches as nearly everything changed ?
It's up to Alexandre to decide, just send your work to wine-patches.
I would suggest that you do split it into seperate patches. If one of your patches causes a regression, it's easier to find the regression if the changes are small. Secondly, it's easier to review and verify small patches. Thirdly, since small patches are easier to check, they're easier to get accepted. How about creating three seperate patches that: 1. Change to use a dialog 2. Change to use unicode 3. internationalize the code This seems slower, but it will actually be faster, since you'll only need to perfect one patch at a time. Mike
Hello, I noticed Alexandre committed patch #1/3 of the new uninstaller; thanks for that. Could you please tell me why #2 (find-as-you-type) and #3 (internationalization) stayed out ? Thanks a lot for your help. Jonathan
Jonathan Ernst <Jonathan(a)ErnstFamily.ch> writes:
Hello,
I noticed Alexandre committed patch #1/3 of the new uninstaller; thanks for that. Could you please tell me why #2 (find-as-you-type) and #3 (internationalization) stayed out ?
They don't apply cleanly, you have to resubmit them relative to CVS. -- Alexandre Julliard julliard(a)winehq.org
Just another bug report - supermaintainers can't delete postings like normal maintainers can. Thanks, Scott Ritchie
Le vendredi 11 mars 2005 à 01:12 -0800, Scott Ritchie a écrit :
Just another bug report - supermaintainers can't delete postings like normal maintainers can.
Thanks, Scott Ritchie
Are you sure ? I just tried it and it works on my local setup. I click on "Edit" and then I have a delete button I can click on. What error do you get when clicking on this button ? Thanks
On Fri, 2005-03-11 at 10:51 +0100, Jonathan Ernst wrote:
Le vendredi 11 mars 2005 à 01:12 -0800, Scott Ritchie a écrit :
Just another bug report - supermaintainers can't delete postings like normal maintainers can.
Thanks, Scott Ritchie
Are you sure ?
I just tried it and it works on my local setup. I click on "Edit" and then I have a delete button I can click on.
What error do you get when clicking on this button ?
Thanks
I'm talking about the posts by users on the version's page, not the notes and howtos and stuff. Thanks, Scott
Le vendredi 11 mars 2005 à 04:00 -0800, Scott Ritchie a écrit :
On Fri, 2005-03-11 at 10:51 +0100, Jonathan Ernst wrote:
Are you sure ?
I just tried it and it works on my local setup. I click on "Edit" and then I have a delete button I can click on.
What error do you get when clicking on this button ?
Thanks
I'm talking about the posts by users on the version's page, not the notes and howtos and stuff.
Thanks, Scott
That's what I'm talking about too. => what error do you get ? thanks.
On Fri, 2005-03-11 at 13:47 +0100, Jonathan Ernst wrote:
Le vendredi 11 mars 2005 à 04:00 -0800, Scott Ritchie a écrit :
On Fri, 2005-03-11 at 10:51 +0100, Jonathan Ernst wrote:
Are you sure ?
I just tried it and it works on my local setup. I click on "Edit" and then I have a delete button I can click on.
What error do you get when clicking on this button ?
Thanks
I'm talking about the posts by users on the version's page, not the notes and howtos and stuff.
Thanks, Scott
That's what I'm talking about too. => what error do you get ?
thanks.
I don't get a button at all, only "Post new" and "Reply to this" -Scott
participants (5)
-
Alexandre Julliard -
Dmitry Timoshkov -
Jonathan Ernst -
Mike McCormack -
Scott Ritchie