October 2011 - Wine-Devel - list.winehq.org

Re: [Wine] WineHQ database compromise

by Josh Juran

On Oct 11, 2011, at 12:13 PM, Jeremy White wrote: > What we know at this point that someone was able to obtain unauthorized > access to the phpmyadmin utility. We do not exactly how they obtained > access; it was either by compromising an admins credentials, or by > exploiting an unpatched vulnerability in phpmyadmin. Insecure HTTP access? > Unfortunately, the attackers were able to download the full login > database for both the appdb and bugzilla. This means that they have all > of those emails, as well as the passwords. The passwords are stored > encrypted, but with enough effort and depending on the quality of the > password, they can be cracked. > > This, I'm afraid, is a serious threat; it means that anyone who uses the > same email / password on other systems is now vulnerable to a malicious > attacker using that information to access their account. Since bugzilla passwords were sent in cleartext anyway, I sincerely hope none of them were otherwise valuable. (Remember FireSheep?) > We are going to be resetting every password and sending a private email > to every affected user. You might also consider expiring old login cookies. > This is again another reminder to never use a common username / password > pair. This web site provides further advice as well: > http://asiknews.wordpress.com/2011/03/02/best-practice-password-management-… Josh

Oct. 11, 2011

Re: WineHQ database compromise

by Per Johansson

On Tue, Oct 11, 2011 at 9:13 PM, Jeremy White <jwhite(a)codeweavers.com>wrote: > Hi, > > I am sad to say that there was a compromise of the WineHQ database system. > <snip> Hi, one question. I'm not worried about my current account, but I had an old email with an old password recorded in my keychain store. I tried that email at appdb.winehq.org but it said "user does not exist". Can I assume it was completely deleted? Regards, -- Per Johansson

Oct. 11, 2011

Re: [PATCH 1/7] msi/custom.c: unify thread handling of different custom action types (resend)

by Bernhard Loos

On Tue, Oct 11, 2011 at 2:29 PM, Hans Leidekker <hans(a)codeweavers.com> wrote: > On Tue, 2011-10-11 at 13:20 +0200, Bernhard Loos wrote: > +static HRESULT get_action_info( msi_custom_action_info *info, INT *type, > + BSTR *source, BSTR *target, BSTR *name, > + IWineMsiRemotePackage **package ) > { > - IClassFactory *cf = NULL; > - IWineMsiRemoteCustomAction *rca = NULL; > - HRESULT r; > - > - r = DllGetClassObject( &CLSID_WineMsiRemoteCustomAction, > - &IID_IClassFactory, (LPVOID *)&cf ); > - if (FAILED(r)) > - { > - ERR("failed to get IClassFactory interface\n"); > - return ERROR_FUNCTION_FAILED; > - } > - > - r = IClassFactory_CreateInstance( cf, NULL, &IID_IWineMsiRemoteCustomAction, (LPVOID *)&rca ); > - if (FAILED(r)) > - { > - ERR("failed to get IWineMsiRemoteCustomAction interface\n"); > - return ERROR_FUNCTION_FAILED; > - } > - > - r = IWineMsiRemoteCustomAction_GetActionInfo( rca, guid, type, handle, dll, funcname, package ); > - IWineMsiRemoteCustomAction_Release( rca ); > - if (FAILED(r)) > - { > - ERR("GetActionInfo failed\n"); > - return ERROR_FUNCTION_FAILED; > + IWineMsiRemotePackage *rp; > + HRESULT res; > + *source = *target = *name = NULL; > + > + res = create_msi_remote_package( NULL, (void **) &rp ); > + if (FAILED( res )) > + return res; > + res = IWineMsiRemotePackage_SetMsiHandle( rp, alloc_msihandle( &info->package->hdr ) ); > > You should check for failure from alloc_msihandle. In my defence, it currently also doesn't get checked, but I will add something. > + if (FAILED( res )) > + goto error; > + > + *source = SysAllocString( info->source ); > + *target = SysAllocString( info->target ); > + *name = SysAllocString( info->action ); > + if (!*source || !*target || !*name) { > + res = E_OUTOFMEMORY; > + goto error; > } > - > + > + *package = rp; > + *type = info->type; > + > return ERROR_SUCCESS; > > This should become S_OK. You are mixing up return types in more places.

Oct. 11, 2011

Re: [PATCH 5/7] msi/custom.c: get rid of MSIRUNNINGACTION

by Bernhard Loos

On Tue, Oct 11, 2011 at 3:22 PM, Hans Leidekker <hans(a)codeweavers.com> wrote: > >> -static UINT HANDLE_CustomType50(MSIPACKAGE *package, LPCWSTR source, >> +static msi_custom_action_info *HANDLE_CustomType50(MSIPACKAGE *package, LPCWSTR source, >> LPCWSTR target, const INT type, LPCWSTR action) >> { >> WCHAR *exe, *arg; >> msi_custom_action_info *info; >> >> - if (!(exe = msi_dup_property( package->db, source ))) return ERROR_SUCCESS; >> + if (!(exe = msi_dup_property( package->db, source ))) return NULL; > > Are you sure this should be an error? Native might ignore a missing property. Well, it can't continue with executing the custom action, so it fails (tried this). I will try to add a test for it. >> -static UINT HANDLE_CustomType53_54(MSIPACKAGE *package, LPCWSTR source, >> +static msi_custom_action_info *HANDLE_CustomType53_54(MSIPACKAGE *package, LPCWSTR source, >> LPCWSTR target, const INT type, LPCWSTR action) >> { >> msi_custom_action_info *info; >> WCHAR *prop; >> - UINT r; >> >> TRACE("%s %s\n", debugstr_w(source), debugstr_w(target)); >> >> prop = msi_dup_property( package->db, source ); >> - if (!prop) return ERROR_SUCCESS; >> + if (!prop) return NULL; > > Same here. I never got script actions working (is there any documentation on this?!?), but I would be really surprised, if it silently drops the action and continues with the installation.

Oct. 11, 2011

Re: WineHQ database compromise

by Nicolas Le Cam

2011/10/11 Jerome Leclanche <adys.wh(a)gmail.com>: > Thank you so much for letting the users know so early on. > > Bugzilla/forum passwords should probably be reset as well for appdb > users, there's no doubt most people share passwords with the appdb. > > On Tue, Oct 11, 2011 at 8:13 PM, Jeremy White <jwhite(a)codeweavers.com> wrote: >> Hi, >> >> I am sad to say that there was a compromise of the WineHQ database system. >> >> What we know at this point that someone was able to obtain unauthorized >> access to the phpmyadmin utility. We do not exactly how they obtained >> access; it was either by compromising an admins credentials, or by >> exploiting an unpatched vulnerability in phpmyadmin. >> >> We had reluctantly provided access to phpmyadmin to the appdb developers >> (it is a very handy tool, and something they very much wanted). But it >> is a prime target for hackers, and apparently our best efforts at >> obscuring it and patching it were not sufficient. >> >> So we have removed all access to phpmyadmin from the outside world. >> >> We do not believe the attackers obtained any other form of access to the >> system. >> >> On the one hand, we saw no evidence of harm to any database. We saw no >> evidence of any attempt to change the database (and candidly, using the >> real appdb or bugzilla is the easy way to change the database). >> >> Unfortunately, the attackers were able to download the full login >> database for both the appdb and bugzilla. This means that they have all >> of those emails, as well as the passwords. The passwords are stored >> encrypted, but with enough effort and depending on the quality of the >> password, they can be cracked. >> >> This, I'm afraid, is a serious threat; it means that anyone who uses the >> same email / password on other systems is now vulnerable to a malicious >> attacker using that information to access their account. >> >> We are going to be resetting every password and sending a private email >> to every affected user. >> >> This is again another reminder to never use a common username / password >> pair. This web site provides further advice as well: >> http://asiknews.wordpress.com/2011/03/02/best-practice-password-management-… >> >> I am very sad to have to report this. We have so many challenges in our >> world today that this is a particularly painful form of salt for our wounds. >> >> However, I think it is urgent for everyone to know what happened. >> >> Cheers, >> >> Jeremy >> >> >> > > > Thanks for the early notice ! Testbot passwords should also be reset as it seems it doesn't allow password reset / change ATM. (At least I wasn't able to find that possibility) -- Nicolas Le Cam

Oct. 11, 2011

Re: WineHQ database compromise

by Jerome Leclanche

On Tue, Oct 11, 2011 at 8:46 PM, Jerome Leclanche <adys.wh(a)gmail.com> wrote: > Thank you so much for letting the users know so early on. > > Bugzilla/forum passwords should probably be reset as well for appdb > users, there's no doubt most people share passwords with the appdb. > > On Tue, Oct 11, 2011 at 8:13 PM, Jeremy White <jwhite(a)codeweavers.com> wrote: >> Hi, >> >> I am sad to say that there was a compromise of the WineHQ database system. >> >> What we know at this point that someone was able to obtain unauthorized >> access to the phpmyadmin utility. We do not exactly how they obtained >> access; it was either by compromising an admins credentials, or by >> exploiting an unpatched vulnerability in phpmyadmin. >> >> We had reluctantly provided access to phpmyadmin to the appdb developers >> (it is a very handy tool, and something they very much wanted). But it >> is a prime target for hackers, and apparently our best efforts at >> obscuring it and patching it were not sufficient. >> >> So we have removed all access to phpmyadmin from the outside world. >> >> We do not believe the attackers obtained any other form of access to the >> system. >> >> On the one hand, we saw no evidence of harm to any database. We saw no >> evidence of any attempt to change the database (and candidly, using the >> real appdb or bugzilla is the easy way to change the database). >> >> Unfortunately, the attackers were able to download the full login >> database for both the appdb and bugzilla. This means that they have all >> of those emails, as well as the passwords. The passwords are stored >> encrypted, but with enough effort and depending on the quality of the >> password, they can be cracked. >> >> This, I'm afraid, is a serious threat; it means that anyone who uses the >> same email / password on other systems is now vulnerable to a malicious >> attacker using that information to access their account. >> >> We are going to be resetting every password and sending a private email >> to every affected user. >> >> This is again another reminder to never use a common username / password >> pair. This web site provides further advice as well: >> http://asiknews.wordpress.com/2011/03/02/best-practice-password-management-… >> >> I am very sad to have to report this. We have so many challenges in our >> world today that this is a particularly painful form of salt for our wounds. >> >> However, I think it is urgent for everyone to know what happened. >> >> Cheers, >> >> Jeremy >> >> >> > Nevermind... had not received the other emails yet. Best of luck sorting it all out. JL

Oct. 11, 2011

Re: WineHQ database compromise

by Maarten Lankhorst

Hey everyone, On 10/11/2011 09:13 PM, Jeremy White wrote: > Hi, > > I am sad to say that there was a compromise of the WineHQ database system. > > What we know at this point that someone was able to obtain unauthorized > access to the phpmyadmin utility. We do not exactly how they obtained > access; it was either by compromising an admins credentials, or by > exploiting an unpatched vulnerability in phpmyadmin. > > We had reluctantly provided access to phpmyadmin to the appdb developers > (it is a very handy tool, and something they very much wanted). But it > is a prime target for hackers, and apparently our best efforts at > obscuring it and patching it were not sufficient. > > So we have removed all access to phpmyadmin from the outside world. > > We do not believe the attackers obtained any other form of access to the > system. > > On the one hand, we saw no evidence of harm to any database. We saw no > evidence of any attempt to change the database (and candidly, using the > real appdb or bugzilla is the easy way to change the database). > > Unfortunately, the attackers were able to download the full login > database for both the appdb and bugzilla. This means that they have all > of those emails, as well as the passwords. The passwords are stored > encrypted, but with enough effort and depending on the quality of the > password, they can be cracked. > > This, I'm afraid, is a serious threat; it means that anyone who uses the > same email / password on other systems is now vulnerable to a malicious > attacker using that information to access their account. > You may also want to change your testbot password if you re-used your password.. https://testbot.winehq.org/ForgotPassword.pl Cheers, Maarten

Oct. 11, 2011

Re: WineHQ database compromise

by Jerome Leclanche

Thank you so much for letting the users know so early on. Bugzilla/forum passwords should probably be reset as well for appdb users, there's no doubt most people share passwords with the appdb. On Tue, Oct 11, 2011 at 8:13 PM, Jeremy White <jwhite(a)codeweavers.com> wrote: > Hi, > > I am sad to say that there was a compromise of the WineHQ database system. > > What we know at this point that someone was able to obtain unauthorized > access to the phpmyadmin utility. We do not exactly how they obtained > access; it was either by compromising an admins credentials, or by > exploiting an unpatched vulnerability in phpmyadmin. > > We had reluctantly provided access to phpmyadmin to the appdb developers > (it is a very handy tool, and something they very much wanted). But it > is a prime target for hackers, and apparently our best efforts at > obscuring it and patching it were not sufficient. > > So we have removed all access to phpmyadmin from the outside world. > > We do not believe the attackers obtained any other form of access to the > system. > > On the one hand, we saw no evidence of harm to any database. We saw no > evidence of any attempt to change the database (and candidly, using the > real appdb or bugzilla is the easy way to change the database). > > Unfortunately, the attackers were able to download the full login > database for both the appdb and bugzilla. This means that they have all > of those emails, as well as the passwords. The passwords are stored > encrypted, but with enough effort and depending on the quality of the > password, they can be cracked. > > This, I'm afraid, is a serious threat; it means that anyone who uses the > same email / password on other systems is now vulnerable to a malicious > attacker using that information to access their account. > > We are going to be resetting every password and sending a private email > to every affected user. > > This is again another reminder to never use a common username / password > pair. This web site provides further advice as well: > http://asiknews.wordpress.com/2011/03/02/best-practice-password-management-… > > I am very sad to have to report this. We have so many challenges in our > world today that this is a particularly painful form of salt for our wounds. > > However, I think it is urgent for everyone to know what happened. > > Cheers, > > Jeremy > > >

Oct. 11, 2011

WineHQ database compromise

by Jeremy White

Hi, I am sad to say that there was a compromise of the WineHQ database system. What we know at this point that someone was able to obtain unauthorized access to the phpmyadmin utility. We do not exactly how they obtained access; it was either by compromising an admins credentials, or by exploiting an unpatched vulnerability in phpmyadmin. We had reluctantly provided access to phpmyadmin to the appdb developers (it is a very handy tool, and something they very much wanted). But it is a prime target for hackers, and apparently our best efforts at obscuring it and patching it were not sufficient. So we have removed all access to phpmyadmin from the outside world. We do not believe the attackers obtained any other form of access to the system. On the one hand, we saw no evidence of harm to any database. We saw no evidence of any attempt to change the database (and candidly, using the real appdb or bugzilla is the easy way to change the database). Unfortunately, the attackers were able to download the full login database for both the appdb and bugzilla. This means that they have all of those emails, as well as the passwords. The passwords are stored encrypted, but with enough effort and depending on the quality of the password, they can be cracked. This, I'm afraid, is a serious threat; it means that anyone who uses the same email / password on other systems is now vulnerable to a malicious attacker using that information to access their account. We are going to be resetting every password and sending a private email to every affected user. This is again another reminder to never use a common username / password pair. This web site provides further advice as well: http://asiknews.wordpress.com/2011/03/02/best-practice-password-management-… I am very sad to have to report this. We have so many challenges in our world today that this is a particularly painful form of salt for our wounds. However, I think it is urgent for everyone to know what happened. Cheers, Jeremy

Oct. 11, 2011

Re: [PATCH 1/2] ntdll: windows refuses to load an image; if sections overlap the header size as given in the SizeOfHeaders field

by Stefan Dösinger

On Tuesday 11 October 2011 18:15:29 Dan Kegel wrote: > On Tue, Oct 11, 2011 at 9:10 AM, Bernhard Loos > > <bernhardloos(a)googlemail.com> wrote: > > It gets used in patch 2/2, that's why I overlooked it. Should I still > > resubmit it, as it would be only broken for a single revision? > > It would be nice; that way the tree is a bit easier to bisect, etc, > but mostly that way your patch series will actually be tested by the > buildbot. I think Alexandre builds with -Werror, so he wouldn't apply your patches anyway.

Oct. 11, 2011

Re: [3/3] ddraw: Keep track of interface attached by AddAttachedSurface and detach correct interface when parent is released. (try 2)

by Alexandre Julliard

Octavian Voicu <octavian.voicu(a)gmail.com> writes: > Fixes a bug that causes a stencil buffer leak in the game The Longest Journey. > Problem can be seen in the console output log attached to bug #11819. > This patch doesn't fix the original bug, but now game exits cleanly. > > The root of the problem is that when releasing a surface, its attached > surfaces are detached using IDirectDraw7_DeleteAttachedSurface, regardless > of what interface was originally attached. It doesn't work here: ../../../tools/runtest -q -P wine -M ddraw.dll -T ../../.. -p ddraw_test.exe.so dsurface.c && touch dsurface.ok dsurface.c:1603: Test failed: DeleteAttachedSurface returned 887601cc dsurface.c:3534: Tests skipped: IDirectDraw7_CreateSurface failed, hr = 0x88760091, format D3DFMT_X8B8G8R8 dsurface.c:3534: Tests skipped: IDirectDraw7_CreateSurface failed, hr = 0x88760091, format D3DFMT_X8B8G8R8 dsurface.c:3534: Tests skipped: IDirectDraw7_CreateSurface failed, hr = 0x88760091, format D3DFMT_A2R10G10B10 dsurface.c:3534: Tests skipped: IDirectDraw7_CreateSurface failed, hr = 0x88760091, format D3DFMT_A8L8 make[1]: *** [dsurface.ok] Error 1 -- Alexandre Julliard julliard(a)winehq.org

Oct. 11, 2011

Re: [PATCH 1/2] ntdll: windows refuses to load an image; if sections overlap the header size as given in the SizeOfHeaders field

by Dan Kegel

Incidentally, the reason no automatic email was sent was that gcc still has those nonascii characters in its output, even though I set LANG=C. I'm now adding more logging to see what's up. On Tue, Oct 11, 2011 at 9:06 AM, Dan Kegel <dank(a)kegel.com> wrote: > Hi Bernhard, > Build fails here: > > loader.c: In function ‘test_section_mapping’: > loader.c:706:9: error: unused variable ‘i’ [-Werror=unused-variable] > cc1: all warnings being treated as errors > make[1]: *** [loader.o] Error 1 > make[1]: Leaving directory > `/home/bob/wineslave.dir/sandbox/slave/runtests-default/build/dlls/kernel32/tests' > make: *** [dlls/kernel32/tests] Error 2 > > http://buildbot.kegel.com:8010/builders/runtests-default/builds/159 > > Can you fix and resubmit the series? >

Oct. 11, 2011

Re: [PATCH 1/2] ntdll: windows refuses to load an image; if sections overlap the header size as given in the SizeOfHeaders field

by Dan Kegel

On Tue, Oct 11, 2011 at 9:10 AM, Bernhard Loos <bernhardloos(a)googlemail.com> wrote: > It gets used in patch 2/2, that's why I overlooked it. Should I still > resubmit it, as it would be only broken for a single revision? It would be nice; that way the tree is a bit easier to bisect, etc, but mostly that way your patch series will actually be tested by the buildbot.

Oct. 11, 2011

Re: [PATCH 1/2] ntdll: windows refuses to load an image; if sections overlap the header size as given in the SizeOfHeaders field

by Bernhard Loos

On Tue, Oct 11, 2011 at 6:06 PM, Dan Kegel <dank(a)kegel.com> wrote: > Hi Bernhard, > Build fails here: > > loader.c: In function ‘test_section_mapping’: > loader.c:706:9: error: unused variable ‘i’ [-Werror=unused-variable] > cc1: all warnings being treated as errors > make[1]: *** [loader.o] Error 1 > make[1]: Leaving directory > `/home/bob/wineslave.dir/sandbox/slave/runtests-default/build/dlls/kernel32/tests' > make: *** [dlls/kernel32/tests] Error 2 > > http://buildbot.kegel.com:8010/builders/runtests-default/builds/159 > > Can you fix and resubmit the series? It gets used in patch 2/2, that's why I overlooked it. Should I still resubmit it, as it would be only broken for a single revision?

Oct. 11, 2011

re: [PATCH 1/2] ntdll: windows refuses to load an image; if sections overlap the header size as given in the SizeOfHeaders field

by Dan Kegel

Hi Bernhard, Build fails here: loader.c: In function ‘test_section_mapping’: loader.c:706:9: error: unused variable ‘i’ [-Werror=unused-variable] cc1: all warnings being treated as errors make[1]: *** [loader.o] Error 1 make[1]: Leaving directory `/home/bob/wineslave.dir/sandbox/slave/runtests-default/build/dlls/kernel32/tests' make: *** [dlls/kernel32/tests] Error 2 http://buildbot.kegel.com:8010/builders/runtests-default/builds/159 Can you fix and resubmit the series?

Oct. 11, 2011

Re: [PATCH 4/4] kernel32, ntdll: implement most of GetNamedPipeHandleState

by Bernhard Loos

On Tue, Oct 11, 2011 at 5:12 PM, Juan Lang <juan.lang(a)gmail.com> wrote: > Hi Bernhard, > > nit: > - todo_wine > - ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, > + todo_wine ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, > > This change is a no-op. > --Juan > I think I removed it first but had to re-add it later.

Oct. 11, 2011

Re: [PATCH 4/4] kernel32, ntdll: implement most of GetNamedPipeHandleState

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=14813 Your paranoid android. === WNT4WSSP6 (32 bit pipe) === pipe.c:1559: Test failed: expected ERROR_CANNOT_IMPERSONATE, got 203 === W2KPROSP4 (32 bit pipe) === pipe.c:1559: Test failed: expected ERROR_CANNOT_IMPERSONATE, got 997

Oct. 11, 2011

Re: [PATCH 4/4] kernel32, ntdll: implement most of GetNamedPipeHandleState

by Juan Lang

Hi Bernhard, nit: - todo_wine - ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, + todo_wine ok(!ret && GetLastError() == ERROR_INVALID_PARAMETER, This change is a no-op. --Juan

Oct. 11, 2011

Re: [PATCH 5/7] msi/custom.c: get rid of MSIRUNNINGACTION

by Hans Leidekker

> -static UINT HANDLE_CustomType50(MSIPACKAGE *package, LPCWSTR source, > +static msi_custom_action_info *HANDLE_CustomType50(MSIPACKAGE *package, LPCWSTR source, > LPCWSTR target, const INT type, LPCWSTR action) > { > WCHAR *exe, *arg; > msi_custom_action_info *info; > > - if (!(exe = msi_dup_property( package->db, source ))) return ERROR_SUCCESS; > + if (!(exe = msi_dup_property( package->db, source ))) return NULL; Are you sure this should be an error? Native might ignore a missing property. > -static UINT HANDLE_CustomType53_54(MSIPACKAGE *package, LPCWSTR source, > +static msi_custom_action_info *HANDLE_CustomType53_54(MSIPACKAGE *package, LPCWSTR source, > LPCWSTR target, const INT type, LPCWSTR action) > { > msi_custom_action_info *info; > WCHAR *prop; > - UINT r; > > TRACE("%s %s\n", debugstr_w(source), debugstr_w(target)); > > prop = msi_dup_property( package->db, source ); > - if (!prop) return ERROR_SUCCESS; > + if (!prop) return NULL; Same here.

Oct. 11, 2011

Re: [PATCH 3/3] vbscript: Added Hex function tests

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=14804 Your paranoid android. === WNT4WSSP6 (32 bit) === No test summary line found === W2KPROSP4 (32 bit) === No test summary line found === WXPPROSP3 (32 bit) === No test summary line found === W2K3R2SESP2 (32 bit) === No test summary line found === WVISTAADM (32 bit) === No test summary line found === W2K8SE (32 bit) === No test summary line found === W7PRO (32 bit) === No test summary line found === W7PROX64 (32 bit) === No test summary line found === TEST64_W7SP1 (32 bit) === No test summary line found === W7PROX64 (64 bit) === No test summary line found === TEST64_W7SP1 (64 bit) === No test summary line found

Oct. 11, 2011

Re: [PATCH 1/7] msi/custom.c: unify thread handling of different custom action types (resend)

by Hans Leidekker

On Tue, 2011-10-11 at 13:20 +0200, Bernhard Loos wrote: +static HRESULT get_action_info( msi_custom_action_info *info, INT *type, + BSTR *source, BSTR *target, BSTR *name, + IWineMsiRemotePackage **package ) { - IClassFactory *cf = NULL; - IWineMsiRemoteCustomAction *rca = NULL; - HRESULT r; - - r = DllGetClassObject( &CLSID_WineMsiRemoteCustomAction, - &IID_IClassFactory, (LPVOID *)&cf ); - if (FAILED(r)) - { - ERR("failed to get IClassFactory interface\n"); - return ERROR_FUNCTION_FAILED; - } - - r = IClassFactory_CreateInstance( cf, NULL, &IID_IWineMsiRemoteCustomAction, (LPVOID *)&rca ); - if (FAILED(r)) - { - ERR("failed to get IWineMsiRemoteCustomAction interface\n"); - return ERROR_FUNCTION_FAILED; - } - - r = IWineMsiRemoteCustomAction_GetActionInfo( rca, guid, type, handle, dll, funcname, package ); - IWineMsiRemoteCustomAction_Release( rca ); - if (FAILED(r)) - { - ERR("GetActionInfo failed\n"); - return ERROR_FUNCTION_FAILED; + IWineMsiRemotePackage *rp; + HRESULT res; + *source = *target = *name = NULL; + + res = create_msi_remote_package( NULL, (void **) &rp ); + if (FAILED( res )) + return res; + res = IWineMsiRemotePackage_SetMsiHandle( rp, alloc_msihandle( &info->package->hdr ) ); You should check for failure from alloc_msihandle. + if (FAILED( res )) + goto error; + + *source = SysAllocString( info->source ); + *target = SysAllocString( info->target ); + *name = SysAllocString( info->action ); + if (!*source || !*target || !*name) { + res = E_OUTOFMEMORY; + goto error; } - + + *package = rp; + *type = info->type; + return ERROR_SUCCESS; This should become S_OK. You are mixing up return types in more places.

Oct. 11, 2011

Re: winex11.drv/keyboard: MAPVK_VK_TO_CHAR correction (Bug 28168)

by Frédéric Delanoy

On Mon, Oct 10, 2011 at 22:46, <sebastian.riemer(a)gmx.de> wrote: > --- > dlls/winex11.drv/keyboard.c | 563 ++++++++++++++++++++++++++++++++++++++----- > 1 files changed, 499 insertions(+), 64 deletions(-) Please follow patch submission rules from http://wiki.winehq.org/SubmittingPatches

Oct. 10, 2011

winex11.drv/keyboard: MAPVK_VK_TO_CHAR direct mapping

by Sebastian Riemer

Hi developers, I've just sent my first patch I would like to see in mainline. I did quite some efforts to verify that my patch is correct by doing extensive compares with Windows, the wine before and the patched wine as well as a special testing version of wine. I want to fix this bug http://bugs.winehq.org/show_bug.cgi?id=28168 and I've attached my verification data there, because this is difficult to integrate into the test suite. Could someone please look at my files and help me with some tips and advice or want to become my mentor? I'm really tired of just writing test data into the AppDB and only little changes over the time for the applications I maintain. So, I want to improve wine for the applications I like. Cheers, Sebastian

Oct. 10, 2011

Re: mscoree: Implement ICorDebug SetManagedHandler (try 3)

by Vincent Povirk

Can we have a WARN message in case of .NET 1 applications? On Mon, Oct 10, 2011 at 2:36 PM, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > On 11/10/2011 2:07 AM, Vincent Povirk wrote: >> >> Should SetManagedHandler really fail if the given object doesn't >> support ICorDebugManagedCallback2 ? > > Yes, this is a requirement for debugging .NET 2 applications. > > Best Regards > Alistair Leslie-Hughes >

Oct. 10, 2011

Re: mscoree: Implement ICorDebug SetManagedHandler (try 3)

by Alistair Leslie-Hughes

On 11/10/2011 2:07 AM, Vincent Povirk wrote: > Should SetManagedHandler really fail if the given object doesn't > support ICorDebugManagedCallback2 ? Yes, this is a requirement for debugging .NET 2 applications. Best Regards Alistair Leslie-Hughes

Oct. 10, 2011

[RFC] Use ICU in wine ?

by Rafał Mużyło

On Mon, Oct 10, 2011 at 07:47:32PM +0200, Shachar Shemesh wrote: > ICU is impossible to dynamically link with, and it's size is quite huge > if statically linked. > Huh ? /usr/lib/libicudata.so.48 /usr/lib/libiculx.so.48 /usr/lib/libicutest.so.48 /usr/lib/libicui18n.so.48 /usr/lib/libicutu.so.48 /usr/lib/libicuio.so.48 /usr/lib/libicuuc.so.48 /usr/lib/libicule.so.48 Care to elaborate ? Yes, it has an annoying habit of ABI breaks, but so does i.e. poppler.

Oct. 10, 2011

Re: [RFC] Use ICU in wine ?

by André Hentschel

Am 10.10.2011 17:33, schrieb Rafał Mużyło: > Anyway, if DUCET and Windows default differ is dumping that default even > allowed ? I don't think so... I once was hacking on our collation stuff too to fix http://bugs.winehq.org/show_bug.cgi?id=5163 but i guess dumping the windows stuff wouldn't even have helped me. -- Best Regards, André Hentschel

Oct. 10, 2011

Re: [RFC] Use ICU in wine ?

by Shachar Shemesh

On 10/10/2011 02:48 AM, Rafał Mużyło wrote: > Right now, wine claims to use DUCET data for lingustic sorting, but by > section 1.9.2 of that document (as of version 6.0.0), uses it in a wrong > way. The result of it are bugs such as #10767 and #9583. > > A possible way around it would beby using ICU to get language specific > tailoring and applying some of wine-specific for the parts addressed by > #10767. I will not go into the part of the discussion that has to do with whether ICU will or will not resolve the issues. I will point out that Wine used to have a soft dependency on ICU (introduced, for my sins, by me) for the sake of BiDi processing. Everyone involved, myself included, were all too happy to see it go. ICU is impossible to dynamically link with, and it's size is quite huge if statically linked. The good news is that ICU's license seems to be compatible with Wine's LGPL, so if needed, the relevant part can be copied into Wine. Shachar -- Shachar Shemesh Lingnu Open Source Consulting Ltd. http://www.lingnu.com

Oct. 10, 2011

[RFC] Use ICU in wine ?

by Rafał Mużyło

On Mon, Oct 10, 2011 at 04:16:26PM +0900, Dmitry Timoshkov wrote (and contradicted himself): > What's DUCET and where do you see Wine does claim to use it? > DUCET: Default Unicode Collation Element Table http://bugs.winehq.org/show_bug.cgi?id=10767#c1 > > but by > > section 1.9.2 of that document (as of version 6.0.0), uses it in a wrong > > way. > http://www.unicode.org/reports/tr10/#Non-Goals, point 6 "...DUCET does not and cannot actually provide linguistically correct sorting for every language without further tailoring...." > Could you please be slightly more specific? > > Looks like that you don't understand what those bugs are about. > Is the above link specific enough ? Anyway, if DUCET and Windows default differ is dumping that default even allowed ? Besides, the only proper way to fix bug #9583 is to implement some kind of language tailoring.

Oct. 10, 2011

Re: mscoree: Implement ICorDebug SetManagedHandler (try 3)

by Vincent Povirk

Should SetManagedHandler really fail if the given object doesn't support ICorDebugManagedCallback2 ? You've got a stray whitespace change in RuntimeHost_Destroy. Otherwise, this looks good to me. On Mon, Oct 10, 2011 at 5:12 AM, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > Hi, > > Only Release the interfaces after we have a valid callback. > Allow multiple Interfaces per runtime. > > > Changelog: > mscoree: Implement ICorDebug SetManagedHandler > > > Best Regards > Alistair Leslie-Hughes > > > > > >

Oct. 10, 2011

DEVELOPMENTAL: use uniscribe in the edit control

by Aric Stewart

This patch is experimental but people had some interest in testing it and I would love some more testing. What is currently known not to be working: * Tabs * Font Fallback * Complex composed glyphs are not breaking or highlighting properly. Testing is appreceated. Send me crashes or incorrect behavior. It should work with both native and builtin usp10. The patch will be cleaned and broken up before being send it this is just for testing. thanks! -aric --- dlls/user32/edit.c | 495 +++++++++++++++++++++++++++------------------------- 1 files changed, 256 insertions(+), 239 deletions(-)

Oct. 10, 2011

[RFC] Use ICU in wine ?

by Rafał Mużyło

On Mon, Oct 10, 2011 at 04:16:26PM +0900, Dmitry Timoshkov wrote (and contradicted himself): > What's DUCET and where do you see Wine does claim to use it? > DUCET: Default Unicode Collation Element Table http://bugs.winehq.org/show_bug.cgi?id=10767#c1 > > but by > > section 1.9.2 of that document (as of version 6.0.0), uses it in a wrong > > way. > http://www.unicode.org/reports/tr10/#Non-Goals, point 6 "...DUCET does not and cannot actually provide linguistically correct sorting for every language without further tailoring...." > Could you please be slightly more specific? > > Looks like that you don't understand what those bugs are about. > Is the above link specific enough ?

Oct. 10, 2011

Re: [1/5] d3d8: Never announce support for palettized textures

by Stefan Dösinger

On Monday 10 October 2011 10:20:15 Henri Verbeet wrote: > It's not really clear to me that refusing to create P8 surfaces in > d3d8/9 really gains us much, or that it's really a prerequisite for > the other patches. I guess you've got a point here, if we just allow the surfaces we'll end up with the same behavior as windows for probably the same reason - functionality for ddraw is exposed via d3d8/9, but because d3d8/9 don't have P8 primaries it is useless. I'll still return D3DERR_INVALIDCALL from the palette setters/getters to remove the d3d8/9 palette API from wined3d, which simplifies the code there. We could implement them via the ddraw palette API, but without test cases showing how they're supposed to work it would be a shot in the dark and most likely dead code.

Oct. 10, 2011

Re: [ 3/3] ddraw: Keep track of interface attached by AddAttachedSurface and detach correct interface when parent is released.

by Octavian Voicu

On Mon, Oct 10, 2011 at 10:04 AM, Henri Verbeet <hverbeet(a)gmail.com> wrote: > Can't you just handle this in ddraw_surface_delete_attached_surface()? > I.e., replace the Release calls in > ddraw_surface?_DeleteAttachedSurface() with a > IUnknown_Release(Surf->attached_iface) in > ddraw_surface_delete_attached_surface(), and then just call > ddraw_surface_delete_attached_surface() where you're using > delete_attached_surface_helper() now. This is much clever! I'll resend the patches, thanks. Octavian

Oct. 10, 2011

Re: [1/5] d3d8: Never announce support for palettized textures

by Henri Verbeet

On 9 October 2011 17:31, Stefan Dösinger <stefan(a)codeweavers.com> wrote: >> - I can't say I find e.g. sprinkling d3dformat_is_palettized() over >> the code particularly elegant. > I suppose I could check for it in IDirect3DDevice8Impl_CreateSurface(and the > d3d9 equivalent), if all calls that create surfaces run through it. > >> - Do you have tests for the D3DERR_INVALIDCALL returns? > I tested that and Windows returns D3D_OK. Windows also allows creating P8 > offscreen plain surfaces, and it allows StretchRect operations between those > surfaces. It doesn't allow StretchRect operations between P8 and non- > Palettized surfaces. In the end I could not find any way to make use of those > surfaces I could create. > > I prefer to return INVALIDCALL over the half-baked implementation Windows has > to find applications that try to use P8 surfaces in a way I may have missed. > It's not really clear to me that refusing to create P8 surfaces in d3d8/9 really gains us much, or that it's really a prerequisite for the other patches. If you want to go that way though, you should probably just map D3DFORMAT_P8 to something that's not WINED3DFMT_P8_UINT, perhaps WINED3DFMT_UNSUPPORTED, and just let wined3d fail resource creation.

Oct. 10, 2011

Re: [RFC] Use ICU in wine ?

by Dmitry Timoshkov

Rafał Mużyło <galtgendo(a)o2.pl> wrote: > Right now, wine claims to use DUCET data for lingustic sorting, What's DUCET and where do you see Wine does claim to use it? > but by > section 1.9.2 of that document (as of version 6.0.0), uses it in a wrong > way. Could you please be slightly more specific? > The result of it are bugs such as #10767 and #9583. Looks like that you don't understand what those bugs are about. > A possible way around it would beby using ICU to get language specific > tailoring and applying some of wine-specific for the parts addressed by > #10767. Once you uderstand the problems you may see that adding ICU to Wine depencies will create much more problems than it's supposedly will solve. -- Dmitry.

Oct. 10, 2011

Re: [ 3/3] ddraw: Keep track of interface attached by AddAttachedSurface and detach correct interface when parent is released.

by Henri Verbeet

On 10 October 2011 08:07, Octavian Voicu <octavian.voicu(a)gmail.com> wrote: > +static HRESULT delete_attached_surface_helper(IDirectDrawSurfaceImpl *root, IDirectDrawSurfaceImpl *detach) > +{ > + if (detach->attached_iface == &detach->IDirectDrawSurface4_iface) > + return IDirectDrawSurface4_DeleteAttachedSurface(&root->IDirectDrawSurface4_iface, 0, > + &detach->IDirectDrawSurface4_iface); > + else if (detach->attached_iface == &detach->IDirectDrawSurface3_iface) > + return IDirectDrawSurface3_DeleteAttachedSurface(&root->IDirectDrawSurface3_iface, 0, > + &detach->IDirectDrawSurface3_iface); > + else if (detach->attached_iface == &detach->IDirectDrawSurface2_iface) > + return IDirectDrawSurface2_DeleteAttachedSurface(&root->IDirectDrawSurface2_iface, 0, > + &detach->IDirectDrawSurface2_iface); > + else if (detach->attached_iface == &detach->IDirectDrawSurface_iface) > + return IDirectDrawSurface_DeleteAttachedSurface(&root->IDirectDrawSurface_iface, 0, > + &detach->IDirectDrawSurface_iface); > + else if (detach->attached_iface != &detach->IDirectDrawSurface7_iface) > + FIXME("attached_iface %p is invalid for surface %p\n", detach->attached_iface, detach); > + > + return IDirectDrawSurface7_DeleteAttachedSurface(&root->IDirectDrawSurface7_iface, 0, > + &detach->IDirectDrawSurface7_iface); > +} > + Can't you just handle this in ddraw_surface_delete_attached_surface()? I.e., replace the Release calls in ddraw_surface?_DeleteAttachedSurface() with a IUnknown_Release(Surf->attached_iface) in ddraw_surface_delete_attached_surface(), and then just call ddraw_surface_delete_attached_surface() where you're using delete_attached_surface_helper() now.

Oct. 10, 2011

Re: [PATCH] ntdll: deal with images which override the section table during mapping

by Marcus Meissner

On Mon, Oct 10, 2011 at 11:36:33AM +0200, Bernhard Loos wrote: > --- > dlls/ntdll/virtual.c | 38 ++++++++++++++++++++++++++++++++++---- > 1 files changed, 34 insertions(+), 4 deletions(-) I think this needs tests, the algorithms are very fragile. Ciao, Marcus > diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c > index 5e69eb9..7841d9c 100644 > --- a/dlls/ntdll/virtual.c > +++ b/dlls/ntdll/virtual.c > @@ -1114,6 +1114,7 @@ static NTSTATUS map_image( HANDLE hmapping, int fd, char *base, SIZE_T total_siz > IMAGE_DOS_HEADER *dos; > IMAGE_NT_HEADERS *nt; > IMAGE_SECTION_HEADER *sec; > + IMAGE_SECTION_HEADER *section_table = NULL; > IMAGE_DATA_DIRECTORY *imports; > NTSTATUS status = STATUS_CONFLICTING_ADDRESSES; > int i; > @@ -1123,6 +1124,7 @@ static NTSTATUS map_image( HANDLE hmapping, int fd, char *base, SIZE_T total_siz > struct file_view *view = NULL; > char *ptr, *header_end; > INT_PTR delta = 0; > + DWORD SizeOfHeaders; > > /* zero-map the whole range */ > > @@ -1158,8 +1160,11 @@ static NTSTATUS map_image( HANDLE hmapping, int fd, char *base, SIZE_T total_siz > header_end = ptr + ROUND_SIZE( 0, header_size ); > memset( ptr + header_size, 0, header_end - (ptr + header_size) ); > if ((char *)(nt + 1) > header_end) goto error; > - sec = (IMAGE_SECTION_HEADER*)((char*)&nt->OptionalHeader+nt->FileHeader.SizeOfOptionalHeader); > - if ((char *)(sec + nt->FileHeader.NumberOfSections) > header_end) goto error; > + section_table = (IMAGE_SECTION_HEADER*)((char*)&nt->OptionalHeader+nt->FileHeader.SizeOfOptionalHeader); > + /* make sure we mapped enough memory */ > + if ((char *)(section_table + nt->FileHeader.NumberOfSections) > header_end) goto error; > + /* windows is only interested in the size as given by OptionalHeader.SizeOfHeaders */ > + SizeOfHeaders = ROUND_SIZE( 0, nt->OptionalHeader.SizeOfHeaders ); > > imports = nt->OptionalHeader.DataDirectory + IMAGE_DIRECTORY_ENTRY_IMPORT; > if (!imports->Size || !imports->VirtualAddress) imports = NULL; > @@ -1180,7 +1185,7 @@ static NTSTATUS map_image( HANDLE hmapping, int fd, char *base, SIZE_T total_siz > if (nt->OptionalHeader.FileAlignment != nt->OptionalHeader.SectionAlignment) goto error; > for (i = 0; i < nt->FileHeader.NumberOfSections; i++) > { > - if (sec[i].VirtualAddress != sec[i].PointerToRawData) > + if (section_table[i].VirtualAddress != section_table[i].PointerToRawData) > goto error; /* Windows refuses to load in that case too */ > } > > @@ -1192,6 +1197,24 @@ static NTSTATUS map_image( HANDLE hmapping, int fd, char *base, SIZE_T total_siz > goto done; > } > > + if ((char *)(section_table + nt->FileHeader.NumberOfSections) > ptr + SizeOfHeaders) > + { > + /* part of the section table is outside of SizeOfHeaders > + * at this point, it's likely that a section will get mapped over > + * the section table (Borderlands does this) > + * Not sure how it works on Windows, but we make a copy of the > + * table at this point */ > + sec = RtlAllocateHeap( virtual_heap, 0, sizeof(*sec) * nt->FileHeader.NumberOfSections); > + if (!sec) > + { > + status = STATUS_NO_MEMORY; > + goto error; > + } > + memcpy( sec, section_table, sizeof(*sec) * nt->FileHeader.NumberOfSections); > + section_table = sec; > + } > + else > + sec = section_table; > > /* map all the sections */ > > @@ -1200,6 +1223,9 @@ static NTSTATUS map_image( HANDLE hmapping, int fd, char *base, SIZE_T total_siz > static const SIZE_T sector_align = 0x1ff; > SIZE_T map_size, file_start, file_size, end; > > + if (sec->VirtualAddress < SizeOfHeaders) > + goto error; > + > if (!sec->Misc.VirtualSize) > map_size = ROUND_SIZE( 0, sec->SizeOfRawData ); > else > @@ -1329,7 +1355,7 @@ static NTSTATUS map_image( HANDLE hmapping, int fd, char *base, SIZE_T total_siz > > VIRTUAL_SetProt( view, ptr, ROUND_SIZE( 0, header_size ), VPROT_COMMITTED | VPROT_READ ); > > - sec = (IMAGE_SECTION_HEADER*)((char *)&nt->OptionalHeader+nt->FileHeader.SizeOfOptionalHeader); > + sec = section_table; > for (i = 0; i < nt->FileHeader.NumberOfSections; i++, sec++) > { > SIZE_T size; > @@ -1361,6 +1387,8 @@ static NTSTATUS map_image( HANDLE hmapping, int fd, char *base, SIZE_T total_siz > } > > done: > + if (section_table && (char *)section_table != (char *)&nt->OptionalHeader+nt->FileHeader.SizeOfOptionalHeader) > + RtlFreeHeap( virtual_heap, 0, section_table ); > view->mapping = dup_mapping; > server_leave_uninterrupted_section( &csVirtual, &sigset ); > > @@ -1373,6 +1401,8 @@ static NTSTATUS map_image( HANDLE hmapping, int fd, char *base, SIZE_T total_siz > > error: > if (view) delete_view( view ); > + if (section_table && (char *)section_table != (char *)&nt->OptionalHeader+nt->FileHeader.SizeOfOptionalHeader) > + RtlFreeHeap( virtual_heap, 0, section_table ); > server_leave_uninterrupted_section( &csVirtual, &sigset ); > if (dup_mapping) NtClose( dup_mapping ); > return status; >

Oct. 10, 2011

[RFC] Use ICU in wine ?

by Rafał Mużyło

Right now, wine claims to use DUCET data for lingustic sorting, but by section 1.9.2 of that document (as of version 6.0.0), uses it in a wrong way. The result of it are bugs such as #10767 and #9583. A possible way around it would beby using ICU to get language specific tailoring and applying some of wine-specific for the parts addressed by #10767. In a way, wine already *indirectly* depends on icu on many distros (a potential libxml2 dep) - they were quite a few patches already addressing build failures caused by this in recent past. I've cooked up a patch, that's unfortunately isn't working properly yet, it's more of a draft on where to go, if such road were to be taken. Main issue with this patch ain't it's not ignoring the proper list of symbols, as this would be easy to fix, if not for one problem: the part that I put inside 'if (0)' block. For some reason (threading, perhaps ?), if I pass a non-zero lenght string to ucol_openRules, wineboot hangs. Oddly, if the string lenght is 0, the hang doesn't happen. I can't really tell, what going wrong, as the almost same code works in native linux testcase. Also, while in this patch UCOL_ALTERNATE_HANDLING would have been used for NORM_IGNORESYMBOLS, it would be much better to manipulate some of the settings by a custom rule and VariableTop value, but to do that, first such rule would have to be passed and that odd hang prevents it at the moment. It would also be nice, if it was possible to initialize the collator only once for an app instance - after all, in Windows locale change requires a reboot, so while (AFAIK) one wineserver instance can have apps in different locales running, locale can't change for an already running app. So if you have any idea of why exactly the hang happens and how to get around it or know any *technical* reasons of why ICU couldn't be used in wine, CC me with comments.

Oct. 10, 2011

Re: [PATCH] server: if a debugger is attached to a process, child processes shouldn't get debugged (resend)

by Bernhard Loos

On Sat, Oct 8, 2011 at 10:05 AM, David Laight <david(a)l8s.co.uk> wrote: > On Fri, Oct 07, 2011 at 08:32:27PM +0200, Marcus Meissner wrote: >> On Fri, Oct 07, 2011 at 11:44:24AM +0900, Dmitry Timoshkov wrote: >> > Bernhard Loos <bernhardloos(a)googlemail.com> wrote: >> > >> > > + int debug_childs:1; /* also debug all child processes */ >> > >> > 'debug_children' would be a better name. >> >> also >> unsigned int foo:1; >> please. (int foo:1 works, but is semidefined only ;) > > Why the bitfield anyway? > Unless you are allocating a lot of copies of the structure > it is likely to generate more code than the saved memory. Actually, the generated code is pretty much exactly the same size or even smaller for one function. Also, it makes it clear that this is a boolean flag. > David > > -- > David Laight: david(a)l8s.co.uk >

Oct. 9, 2011

Re: The major mmdevapi and winmm audio bugs

by Austin English

On Fri, Oct 7, 2011 at 09:24, <Joerg-Cyril.Hoehle(a)t-systems.com> wrote: > Hi, > > Bugzilla admins, please set the 1.4 milestone on all bugs mentioned here. > > I'm not yet looking at dsound issues since I first want a correct > foundation upon which to build to upper layers. > > o #28039 IAudioClock_GetPosition must ignore underruns (MacOS) > Here I've no idea how to solve this. My initial query remained unanswered: > http://lists.apple.com/archives/coreaudio-api//2011/Sep/msg00024.html > Ken? Andrew's yesterday patch (using GetCurrentPadding) is an interim > work-around. GetPosition (speaker) is unrelated to padding which is > solely about buffering. > > o #28093 GetCurrentPadding must slowly decrease (MacOS) > may cause an app to play audio using a single buffer, causing > underruns and audio drop outs. > > o #28056 Problems with sound under FreeBSD > which became "blocking calls with OSS" > recently addressed by Andrew. > > o snd_pcm_drop causes incorrect GetPosition after mmdevapi Stop > which I believe to be the cause of: > #27901 winmm tests timing out waiting for hevent after waveOutRestart > May get solved by Andrew's work in: > #28517 Pikachu Volleyball stops playing sound effects > > Every snd_pcm_drop and reset must be inspected with care w.r.t. its > effect on GetPosition. Wine must ensure that after playing ends, > GetPosition == sum of written samples for both mmdevapi and winmm. > > o #27937 winmm kept busy playing silence after play finishes > IIRC because winmm writes silence. Once mmdevapi handles underruns > correctly, my patch "winmm: Never write silence, mmdevapi must handle > underruns." will become applicable: > http://www.winehq.org/pipermail/wine-patches/2011-September/107108.html > > Playing silence causes: > #28027 μ-law rendering with pauses and repetitions (MacOS) > as well as presumably: > #28413 Sound play in games and programs causes brief "pauses" > at the time dsound used winmm prior to 1.3.29. > > o #28047 IAudioClock_GetPosition must not depend on buffering (ALSA/Pulse) > ALSA must derive GetPosition from snd_pcm_delay, not snd_pcm_avail_update > > > There are other issues not associated with a bug entry. I'm off for > vacation, but here's a snippet from my TODO list: > - OSS GetPosition from DSP_GETODELAY > - ALSA+OSS: ReleaseBuffer: check method ordering and max size > - GetStreamLatency = Xms + SHAREDMODE:period > Stream latency is a static property of the audio graph's components. > - wineoss GetStreamLatency must be constant, and SNDCTL_DSP_GETODELAY > is only usable once running > - Check WHDR_BEGIN/ENDLOOP code and write tests > - WINMM_GetPosition is simply written_frames, not even minus padding > (as the old alsa driver did), later use mmdevapi:GetPosition. > - winmm PushData without IAC_Reset or last_clock_pos=0 in case of underrun > - Investigate OSS4 underrun behaviour > - Align wineOSS on newest ALSA code improvements > - Submit CoreAudio memory leak fix to #28023; Write error handling patch > - Check ALSA laptop suspend w.r.t. GetPosition > - Add run-time consistency checks in renderer and > abort audio stream (GetData) upon trouble instead of hanging. > > and finally: > > - Test compatibility with native DSound and file bug if unusable. > There are several reports in bugzilla mentioning that this work-around > ceased to work since 1.3.25. Loss of interoperability is not acceptable. > > Regards, > Jörg Höhle Done. In the future, please post full urls, makes my job easier :). I've also given you bug editing/confirming privileges in bugzilla. Cheers, Austin

Oct. 9, 2011

Re: [PATCH] server: Use syscall(2) instead of inline assembly on Mac OS, too.

by Charles Davis

On Oct 9, 2011, at 1:42 PM, David Laight wrote: > On Sat, Oct 08, 2011 at 07:48:40PM +0200, Alexandre Julliard wrote: >> Charles Davis <cdavis(a)mymail.mines.edu> writes: >> >>> @@ -268,9 +256,9 @@ int send_thread_signal( struct thread *thread, int sig ) >>> if (!mach_port_extract_right( process_port, thread->unix_tid, >>> MACH_MSG_TYPE_COPY_SEND, &port, &type )) >>> { >>> - if ((ret = pthread_kill_syscall( port, sig )) < 0) >>> + if ((ret = syscall( SYS___pthread_kill, port, sig )) != 0) >>> { >>> - errno = -ret; >>> + errno = ret; >> >> syscall is supposed to take care of errno. > > I'm also not at all sure of the portability of using syscall() in > application code. > > What happens when you try to run the above on anything other than Linux? > (Eg aone of the BSDs) That's just it. None of that code is portable. It was designed specifically to work on Darwin (Mac OS X/iOS). (It was even worse before. It used x86 assembly to make the syscall, so it would only work on an Intel Mac--and only in 32-bit mode at that. So if anything, this is making it *more* portable.) It doesn't get compiled anywhere else anyway. For this particular file, the only other system I would be worried about is the HURD (the only other OS I know of that's based on Mach), which hardly anyone uses in practice anyway. Also, if you'll look at Wine's development history, you'll notice similar patches by Maarten Lankhorst and, yes, by AJ himself, for Linux. Chip

Oct. 9, 2011

Re: [PATCH] server: Use syscall(2) instead of inline assembly on Mac OS, too.

by David Laight

On Sat, Oct 08, 2011 at 07:48:40PM +0200, Alexandre Julliard wrote: > Charles Davis <cdavis(a)mymail.mines.edu> writes: > > > @@ -268,9 +256,9 @@ int send_thread_signal( struct thread *thread, int sig ) > > if (!mach_port_extract_right( process_port, thread->unix_tid, > > MACH_MSG_TYPE_COPY_SEND, &port, &type )) > > { > > - if ((ret = pthread_kill_syscall( port, sig )) < 0) > > + if ((ret = syscall( SYS___pthread_kill, port, sig )) != 0) > > { > > - errno = -ret; > > + errno = ret; > > syscall is supposed to take care of errno. I'm also not at all sure of the portability of using syscall() in application code. What happens when you try to run the above on anything other than Linux? (Eg aone of the BSDs) David -- David Laight: david(a)l8s.co.uk

Oct. 9, 2011

valgrind and buildbot

by Dan Kegel

As always, http://buildbot.kegel.com is just now starting to look stable (kind of like fusion power is always nearly practical). So I'm starting to look at adding valgrind to the buildbot. I won't check for memory leaks at all, since those are too numerous to count. And I'll only valgrind the tests immediately related to each proposed patch. First, though, I need a complete suppressions file for all existing wine valgrind warnings. That's going to take a while to put together, since some of the warnings don't show up on every run. And before I bother to automate anything, I'm taking a little detour, looking at the existing warnings, and trying to see if we can make a little progress on some of them. The script http://code.google.com/p/winezeug/source/browse/trunk/buildbot/runval.sh is how I'm starting to do a manual sweep for valgrind errors. I'm going DLL by DLL, starting with advapi32, looking carefully at each valgrind error. For each error that looks spurious or expected, I will add a suppression to the existing global suppressions file, http://code.google.com/p/winezeug/source/browse/trunk/valgrind/valgrind-sup… For each error that looks real, I'll try to understand it and fix it if the error is not deep, or file a wine bug and add a matching suppression to http://code.google.com/p/winezeug/source/browse/trunk/buildbot/valgrind-bla… When bugs get fixed, I'll remove the matching suppressions from that file. This morning I looked at a grand total of one valgrind warning (in advapi32/security.c), and filed http://bugs.winehq.org/show_bug.cgi?id=28628 for it. I did glance at a couple more; advapi32/crypt.c has a bunch of expected errors (which I'll probably add to valgrind-suppressions), and atl/atl_ax has a bunch of what look like real errors (which I'll probably file a bug for and add to valgrind-blacklist). It'll probably take quite a while to catalog the baseline errors and get a complete blacklist. If it takes too long (more than a few weeks), I'll chuck it and do a dumb blanket suppression of all existing warnings.

Oct. 9, 2011

Re: [1/5] d3d8: Never announce support for palettized textures

by Stefan Dösinger

On Sunday 09 October 2011 16:40:11 Henri Verbeet wrote: > I thought you asked me not to look at these patches yet because you > still had some issues to fix. Regardless, I gave this a cursory look: Yeah, those were the ones that implemented blitting in a different way in wined3d and had issues with palette changes and SFLAG_CONVERTED. The patches I sent to wine-devel have this fixed, and the patches I sent to wine-patches aren't affected by this. > - I can't say I find e.g. sprinkling d3dformat_is_palettized() over > the code particularly elegant. I suppose I could check for it in IDirect3DDevice8Impl_CreateSurface(and the d3d9 equivalent), if all calls that create surfaces run through it. > - Do you have tests for the D3DERR_INVALIDCALL returns? I tested that and Windows returns D3D_OK. Windows also allows creating P8 offscreen plain surfaces, and it allows StretchRect operations between those surfaces. It doesn't allow StretchRect operations between P8 and non- Palettized surfaces. In the end I could not find any way to make use of those surfaces I could create. I prefer to return INVALIDCALL over the half-baked implementation Windows has to find applications that try to use P8 surfaces in a way I may have missed. > - In patch 5 you remove wined3d_device_get_current_texture_palette() > from the spec but not from the public header. Oops, thanks for catching that.

Oct. 9, 2011

Re: [1/5] d3d8: Never announce support for palettized textures

by Henri Verbeet

On 9 October 2011 15:21, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > This is the first set of my P8 patches, they remove dead code that is left over > from d3d8/9 style palettized texture support. See > http://www.winehq.org/pipermail/wine-devel/2011-October/092744.html for the > entire patchset. > I thought you asked me not to look at these patches yet because you still had some issues to fix. Regardless, I gave this a cursory look: - I can't say I find e.g. sprinkling d3dformat_is_palettized() over the code particularly elegant. - Do you have tests for the D3DERR_INVALIDCALL returns? - In patch 5 you remove wined3d_device_get_current_texture_palette() from the spec but not from the public header.

Oct. 9, 2011

Palettized ddraw surface patches

by Stefan Dösinger

Hi, I'll start sending my patches for blitting palettized surfaces with opengl. There are quite a few patches and I won't send them all in one, so this mail shows where I am headed. Palettized surfaces are in a bad shape right now, both in terms of code quality and functionality. My patches will not fix all issues because this would require a shadow frontbuffer in wined3d, and Henri and I agreed that this should wait until after Wine 1.4. However, my patches fix a number of issues and eliminate a lot of dead code and hacks. Other work that needs to be done, but is separate from those patches: *) Emulate color keying in the ARBfp fragment pipeline. That way the upload- time color keying conversion can be limited to situations where shaders are not available and the alpha = 0.0 hack in patch 20 removed. *) Finish the blit infrastructure cleanup. Mainly, get rid of the hardcoded arbfp_blit backend in wined3d_surface_blt, get rid of arbfp_blit_surface and the remains of IWineD3DSurfaceImpl_BltOverride. *) Investigate if there's any point in P8 blitting with ATIfs and nvrc. The fixed function pipeline replacements would profit from proper 3D color keying, but that only works if we have blitters that can blit without color keying during texture upload. *) Make overlays work properly again. The overlay tests work, but the d3d7 sdk samples refuse to run for some reason. With the GL ddraw renderer as default we can also look into games like Settlers 3 that use overlays to draw the mouse pointer. @Henri and Matteo: The difference between this patchset and the one I sent you a few days ago is that my old patchset made onscreen surfaces as a whole a special case, while this one only has special handling for blits to onscreen surfaces. The new way needs less infrastructure changes, works better with palette changes and is a lot closer to how things should work with a shadow frontbuffer in wined3d. Stefan

Oct. 9, 2011

Re: The major mmdevapi and winmm audio bugs

by Ken Thomases

On Oct 7, 2011, at 9:24 AM, Joerg-Cyril.Hoehle(a)t-systems.com wrote: > o #28039 IAudioClock_GetPosition must ignore underruns (MacOS) > Here I've no idea how to solve this. My initial query remained unanswered: > http://lists.apple.com/archives/coreaudio-api//2011/Sep/msg00024.html > Ken? Andrew's yesterday patch (using GetCurrentPadding) is an interim > work-around. GetPosition (speaker) is unrelated to padding which is > solely about buffering. It is expected that the queue's time keeps incrementing even when no buffers are enqueued. For example, see <https://developer.apple.com/library/ios/#qa/qa1718/_index.html>. Here's one possible solution: Enqueue the buffers with AudioQueueEnqueueBufferWithParameters() to get back the actual start time (in the queue's time frame) at which each buffer is scheduled to start. Then, compare AudioQueueGetCurrentTime() with the scheduled start time of the enqueued buffer(s) to see how far into which buffer the queue has played. The code will have to track the position (in the desired sense) separately from the queue time. There will be a scalar value containing the sum of the lengths of all buffers known to have been completely played. There will also be a list of time ranges for those buffers which have been queued but are not known to have been completely played yet. For each buffer, the time range consists of its scheduled start time and its length. At certain points -- ca_out_buffer_cb() and GetPosition() -- AudioQueueGetCurrentTime() will be called. The buffer list will be scanned. All the buffers whose end time (i.e. start time + length) is less than the queue time will be removed from the list, with their lengths added into the sum. That's all that's necessary for ca_out_buffer_cb() and is only done in that function to make sure the buffer list doesn't keep growing if GetPosition() is rarely or never called. For GetPosition(), the queue time will be compared to the first buffer that remains in the list, if any. If there are no buffers or the queue time is before the start time of the first buffer, then GetPosition() returns the sum. Otherwise, it will return the sum plus the queue time minus the buffer start time. AudioClient_Reset() will clear the buffer list (and the sum?). There's another approach that came to mind, but I don't think it's feasible: only run the queue when there are actually buffers to play. Don't start the queue until a buffer has been enqueued and stop it when there are no more ready. Basically, during the callback when the queue is requesting another buffer, if there's nothing to enqueue, call AudioQueueStop(..., FALSE). You may also need to use AudioQueueAddPropertyListener() to observe kAudioQueueProperty_IsRunning to learn when it has actually stopped, since the queue's time will reset to 0 at that point. During each period of the queue running, its time will reflect actual queued samples being played. Timeline discontinuities occur when the sample rate changes, the output device is switched, the DSP has to be reset, or that sort of thing, I believe. They don't have anything to do with underruns of the kind you're considering. Regards, Ken

Oct. 9, 2011

Re: inetmib1: Avoid idempotent operation in findSupportedQuery function (Clang)

by Octavian Voicu

2011/10/2 Frédéric Delanoy <frederic.delanoy(a)gmail.com> > - for (i = (indexLow + indexHigh) / 2; !impl && indexLow <= indexHigh; > - i = (indexLow + indexHigh) / 2) > + for (i = indexHigh / 2; !impl && indexLow <= indexHigh; i = (indexLow + indexHigh) / 2) How about following code: while (!impl && indexLow <= indexHigh) { /* ... */ i = (indexLow + indexHigh) / 2; /* ... */ } Octavian

Oct. 9, 2011

Re: jscript: Mark some fall-throughs in switch statements

by Andrew Talbot

> Isn't it that way just to save the use of an extra return S_OK? > Instead a break could be used too because that function returns S_OK > by default. > > http://source.winehq.org/source/dlls/jscript/regexp.c#L4025 > > Best regards, > Bruno A fall-through would indeed be lazy and not in the spirit of the fact that DISPATCH_PROPERTYGET and DISPATCH_PROPERTYPUT are kind of the opposite of each other. It either requires a break in both places (for consistency) to the common "return S_OK" or two immediate such returns, instead. -- Andy.

Oct. 8, 2011