Re: kernel32: WideCharToMultiByte: return error on negative dest len (take 2)
"Dan Kegel" <dank(a)kegel.com> writes:
While investigating a crash in gs-auftrag in bug 9039, noticed WideCharToMultiByte wasn't quite conformant for negative destlen. Fixed, with tests. Passes for me in Wine and Win XP.
This would require reviewing all uses of WideCharToMultiByte in Wine, many places get this wrong. Unless you have an app that requires this fix I'd suggest to leave the test as todo_wine for now. -- Alexandre Julliard julliard(a)winehq.org
Alexandre Julliard wrote:
"Dan Kegel" <dank(a)kegel.com> writes:
While investigating a crash in gs-auftrag in bug 9039, noticed WideCharToMultiByte wasn't quite conformant for negative destlen. Fixed, with tests. Passes for me in Wine and Win XP.
This would require reviewing all uses of WideCharToMultiByte in Wine, many places get this wrong. Unless you have an app that requires this fix I'd suggest to leave the test as todo_wine for now. Would that be a good janitorial work to be done before Wine 1.0? I feel like I'm getting tired of translating ...
bye michael
Michael Stefaniuc <mstefani(a)redhat.com> writes:
Would that be a good janitorial work to be done before Wine 1.0? I feel like I'm getting tired of translating ...
Sure, if you feel like doing it go ahead. I'm not sure the fix will go in before 1.0, but already fixing as many callers as possible certainly can't hurt. -- Alexandre Julliard julliard(a)winehq.org
On Wed, Mar 26, 2008 at 4:04 AM, Michael Stefaniuc <mstefani(a)redhat.com> wrote:
This would require reviewing all uses of WideCharToMultiByte in Wine, many places get this wrong. Unless you have an app that requires this fix I'd suggest to leave the test as todo_wine for now.
Would that be a good janitorial work to be done before Wine 1.0? I feel like I'm getting tired of translating ...
Sure. A quick grep finds some obvious ones: $ find . -name '*.c' | xargs grep WideCharToMultiByte | grep ' -1, NULL, NULL)' ./wininet/urlcache.c: WideCharToMultiByte(CP_ACP, 0, lpszLocalFileName, -1, achFile, -1, NULL, NULL); ./mshtml/editor.c: WideCharToMultiByte(CP_ACP, 0, V_BSTR(in), -1, stra, -1, NULL, NULL); ./mshtml/persist.c: WideCharToMultiByte(CP_ACP, 0, headers, -1, data, -1, NULL, NULL); ./mshtml/install.c: WideCharToMultiByte(CP_ACP, 0, file_name, -1, file_name_a, -1, NULL, NULL); ./mshtml/nsio.c: WideCharToMultiByte(CP_ACP, 0, container->doc->mime, -1, This->content, -1, NULL, NULL); ./mshtml/navigate.c: WideCharToMultiByte(CP_ACP, 0, szStatusText, -1, This->nschannel->content, -1, NULL, NULL); ./advapi32/cred.c: string_len = WideCharToMultiByte(CP_ACP, 0, CredentialW->TargetName, -1, CredentialA->TargetName, -1, NULL, NULL); ./advapi32/cred.c: string_len = WideCharToMultiByte(CP_ACP, 0, CredentialW->Comment, -1, CredentialA->Comment, -1, NULL, NULL); ./advapi32/cred.c: string_len = WideCharToMultiByte(CP_ACP, 0, CredentialW->TargetAlias, -1, CredentialA->TargetAlias, -1, NULL, NULL); ./advapi32/cred.c: string_len = WideCharToMultiByte(CP_ACP, 0, CredentialW->UserName, -1, CredentialA->UserName, -1, NULL, NULL); ./urlmon/sec_mgr.c: WideCharToMultiByte(CP_ACP, 0, url, -1, (LPSTR)pbSecurityId, -1, NULL, NULL) (I peeked at that last one. The code is enforcing the length limit itself and then telling WideCharToMultiByte that there's no limit. May as well pass the proper limit to WideCharToMultiByte and let it do the length checking?) The first step, as Alexandre says, is to add the test with a todo_wine around the negative destlen test. I would have done it but I'm more than happy to let you run with it. I haven't quite figured out the fix for bug 9039, this is just something I noticed along the way. - Dan
Dan Kegel wrote:
On Wed, Mar 26, 2008 at 4:04 AM, Michael Stefaniuc <mstefani(a)redhat.com> wrote:
This would require reviewing all uses of WideCharToMultiByte in Wine, many places get this wrong. Unless you have an app that requires this fix I'd suggest to leave the test as todo_wine for now.
Would that be a good janitorial work to be done before Wine 1.0? I feel like I'm getting tired of translating ...
Sure. A quick grep finds some obvious ones:
$ find . -name '*.c' | xargs grep WideCharToMultiByte | grep ' -1, NULL, NULL)'
The first step, as Alexandre says, is to add the test with a todo_wine around the negative destlen test. I would have done it but I'm more than happy to let you run with it. Please go ahead and resubmit the patch with todo_wine around it. I don't want to claim ownership for the patch you wrote.
bye michael
On Wed, Mar 26, 2008 at 6:38 AM, Michael Stefaniuc <mstefani(a)redhat.com> wrote:
The first step, as Alexandre says, is to add the test with a todo_wine around the negative destlen test. I would have done it but I'm more than happy to let you run with it.
Please go ahead and resubmit the patch with todo_wine around it.
Done.
I don't want to claim ownership for the patch you wrote.
Don't worry, Alexandre usually gets that right.
"Dan Kegel" <dank(a)kegel.com> writes:
I don't want to claim ownership for the patch you wrote.
Don't worry, Alexandre usually gets that right.
Maybe, but I don't like it. I really prefer for people to send their own patches instead of having others do it for them. -- Alexandre Julliard julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
Dan Kegel -
Michael Stefaniuc