August 2007 - Wine-Devel - list.winehq.org

Re: user32 - set_active_window uses SendMessage instead of PostMessage for WM_ACTIVATEAPP messages.

by Vitaliy Margolen

Peter Dons Tychsen wrote: > Hello Wine. > > On original Windows, the messages WM_ACTIVATEAPP are sent using > PostMessage() and not SendMessage() like in the current Wine > implementation. > > I have thoroughly verified that this is the case on Windows-XP (that it > uses PostMessage()). > > This error can lead to fatal focusing problems, as some programs call > more focus related functions when they receive this message. This leads > to recursive calls into set_active_calls. It does not crash, but leads > to unexpected results such as lost focus for mouse and keyboard. > > This fixes the focus for the game "Wild Metal" if you switch focus from > the game to the desktop, and then back again. > > http://www.rockstargames.com/classics/ > > Cheers, > > /Pedro > > This patch makes 45 msg.c tests in user32 fail for me. Could you double check your findings? Also when you say "thoroughly verified" have you used Wine tests or your own program? Vitaliy

Aug. 2, 2007

[PATCH 2/5] urlmon: Use BSCF_ values passed to report_data to keep track of download state.

by Misha Koshelev

> Hi Misha, > > Misha Koshelev wrote: > > This simplifies the code a bit by getting rid of an enum and a corresponding variable in the IBinding > > implementation, as we can successfully use the bscf values that are passed to report_data to get all > > the information. > > > > Additionally, the original implementation was somewhat incorrect at least for the case of the Http protocol > > as far as sending OnStopBinding > > This patch is already accepted, but unfortunately it's wrong. Tests > changed in the attached patch succeed on native urlmon.dll and fail on > current Wine. Probably only BSCF_LASTDATANOTIFICATION flag should be > used. Other OnProgress callbacks should be called depending on the real > binding state, not the one reported by protocol handler. > > Jacek >

Aug. 2, 2007

Re: Patch: VarWeekdayName

by James Hawkins

On 8/2/07, Jiří Paleček <jpalecek(a)web.de> wrote: > On Fri, 03 Aug 2007 01:43:05 +0200, James Hawkins <truiken(a)gmail.com> > wrote: > > > On 8/2/07, Jiří Paleček <jpalecek(a)web.de> wrote: > >> On Thu, 02 Aug 2007 16:06:44 +0200, Juan Lang <juan.lang(a)gmail.com> > >> wrote: > >> > >> >> Thank you. Hope it will be better this time. Also, I'll send a test > >> >> if this is OK. > >> > > >> > This one looks pretty good to me, but I have one error and a few nits > >> > to correct: > >> > > >> > The error: > >> > + { > >> > + ERR("GetLocaleInfo of 0x%x failed in 2nd stage?!\n", > >> localeValue); > >> > + SysFreeString(*pbstrOut); > >> > > >> > You should set *pbstrOut to NULL in this case. > >> > >> OK. However, note that this was copy&pasted, so there are more instances > >> of this error in wine. It could be somehow automated (like having a > >> macro > >> for these disposing functions which will also set the pointer to NULL). > >> > >> > The nits: > >> > You have a couple small errors in the comments: > >> > - You misspell iWeekday > >> > - The comment for iWeekday in the params section implies 0 is allowed, > >> > but the first check in the function disallows it. > >> > >> This is the problem the misspelled comment is about, MSDN also suggest > >> 0 for weekday is OK, however, disassemble showed it isn't on Win XP. > >> > > > > Are you disassembling native windows binaries? > > Yes, is it a problem? > Yes it's not allowed in this project. Assuming you only disassembled oleaut32.dll, you will unfortunately not be able to contribute to that dll anymore for the Wine project. -- James Hawkins

Aug. 2, 2007

Re: Patch: VarWeekdayName

by Jiří Paleček

On Fri, 03 Aug 2007 01:43:05 +0200, James Hawkins <truiken(a)gmail.com> wrote: > On 8/2/07, Jiří Paleček <jpalecek(a)web.de> wrote: >> On Thu, 02 Aug 2007 16:06:44 +0200, Juan Lang <juan.lang(a)gmail.com> >> wrote: >> >> >> Thank you. Hope it will be better this time. Also, I'll send a test >> >> if this is OK. >> > >> > This one looks pretty good to me, but I have one error and a few nits >> > to correct: >> > >> > The error: >> > + { >> > + ERR("GetLocaleInfo of 0x%x failed in 2nd stage?!\n", >> localeValue); >> > + SysFreeString(*pbstrOut); >> > >> > You should set *pbstrOut to NULL in this case. >> >> OK. However, note that this was copy&pasted, so there are more instances >> of this error in wine. It could be somehow automated (like having a >> macro >> for these disposing functions which will also set the pointer to NULL). >> >> > The nits: >> > You have a couple small errors in the comments: >> > - You misspell iWeekday >> > - The comment for iWeekday in the params section implies 0 is allowed, >> > but the first check in the function disallows it. >> >> This is the problem the misspelled comment is about, MSDN also suggest >> 0 for weekday is OK, however, disassemble showed it isn't on Win XP. >> > > Are you disassembling native windows binaries? Yes, is it a problem? Jiri Palecek

Aug. 2, 2007

Re: Patch: VarWeekdayName

by James Hawkins

On 8/2/07, Jiří Paleček <jpalecek(a)web.de> wrote: > On Thu, 02 Aug 2007 16:06:44 +0200, Juan Lang <juan.lang(a)gmail.com> wrote: > > >> Thank you. Hope it will be better this time. Also, I'll send a test > >> if this is OK. > > > > This one looks pretty good to me, but I have one error and a few nits > > to correct: > > > > The error: > > + { > > + ERR("GetLocaleInfo of 0x%x failed in 2nd stage?!\n", localeValue); > > + SysFreeString(*pbstrOut); > > > > You should set *pbstrOut to NULL in this case. > > OK. However, note that this was copy&pasted, so there are more instances > of this error in wine. It could be somehow automated (like having a macro > for these disposing functions which will also set the pointer to NULL). > > > The nits: > > You have a couple small errors in the comments: > > - You misspell iWeekday > > - The comment for iWeekday in the params section implies 0 is allowed, > > but the first check in the function disallows it. > > This is the problem the misspelled comment is about, MSDN also suggest > 0 for weekday is OK, however, disassemble showed it isn't on Win XP. > Are you disassembling native windows binaries? -- James Hawkins

Aug. 2, 2007

Re: Patch: VarWeekdayName

by Jiří Paleček

On Thu, 02 Aug 2007 16:06:44 +0200, Juan Lang <juan.lang(a)gmail.com> wrote: >> Thank you. Hope it will be better this time. Also, I'll send a test >> if this is OK. > > This one looks pretty good to me, but I have one error and a few nits > to correct: > > The error: > + { > + ERR("GetLocaleInfo of 0x%x failed in 2nd stage?!\n", localeValue); > + SysFreeString(*pbstrOut); > > You should set *pbstrOut to NULL in this case. OK. However, note that this was copy&pasted, so there are more instances of this error in wine. It could be somehow automated (like having a macro for these disposing functions which will also set the pointer to NULL). > The nits: > You have a couple small errors in the comments: > - You misspell iWeekday > - The comment for iWeekday in the params section implies 0 is allowed, > but the first check in the function disallows it. This is the problem the misspelled comment is about, MSDN also suggest 0 for weekday is OK, however, disassemble showed it isn't on Win XP. > Attaching a test with the patch would certainly help. I'm sending it to wine-patches. Regards Jiri Palecek

Aug. 2, 2007

Re: wine3d3: Check the destination recangle when for FastBlt().

by Peter Dons Tychsen

On Mon, 2007-07-30 at 22:18 +0200, Peter Dons Tychsen wrote: > On Sun, 2007-07-29 at 23:08 +0200, Stefan Dösinger wrote: > > Am Sonntag, 29. Juli 2007 21:28 schrieb Peter Dons Tychsen: > > > Hello Wine! > > > > > > I have fixed a small bug in FastBlt(), which caused Wine to crash if the > > > application tried to do FastBlt() to a surface using a bad destination > > > setup. The problem is already fixed in normal Blt(), but was broken for > > > FastBlt(). > > > > > > This fixes a crash for the little game "Treasure Mole Winter Vacations": > > This check should be in ddraw, not wined3d. ddraw, d3d8 and d3d9 have > > different ideas about such checks, that's why the check was moved from > > WineD3D to the client libs. Originally it was in LockRect, and when it was > > moved the check was lost in Blt and BltFast. There should be a check in ddraw > > already, maybe it is not sufficient. > > > > Also please write a test case to verify that the fix is correct(look at > > dlls/ddraw/tests/dsurface.c). This will also prevent the problem from coming > > back. Treasure Mole broke after the ddraw rewrite because exactly this check > > was missing. I implemented it in LockRect, like in old ddraw, but I forgot to > > write a test for BltFast. Then a d3d9 game was broken because d3d9 LockRect > > behaved differently, a test was written for Lock(Rect) in ddraw, d3d8 and > > d3d9, and the range check moved to ddraw, d3d8 and d3d9 to be able to > > implement different checking in each version. Since I forgot to write a test > > when I originally fixed Treasure Mole the bug could creep back in again :-( > > > > Thanks for your comments Stefan, I have taken another look at it. I still think this specific problem should be fixed in wined3d. The problem occurs because BltFast (ddraw, d3d and d3d-gdi) all take two DWORDs as the offset write position, and then later casts them into a RECT structure which has signed values. This must for any caller be considered an error. No caller could get anything useful out of this. Code: IWineGDISurfaceImpl_BltFast(IWineD3DSurface *iface, DWORD dstx, DWORD dsty, IWineD3DSurface *Source, RECT *rsrc, DWORD trans) ... lock_dst.left = dstx; <--- bad cast! lock_dst.top = dsty; <--- bad cast! lock_dst.right = dstx + w; <--- bad cast! lock_dst.bottom = dsty + h; <--- bad cast! Why does this not trigger a warning? Not sure. I can still move the fix to ddraw, but as said, i am not sure it's the right move. Please comment, /pedro Please note: this is a re-send. I think my mail-server trashed the first one. Not sure what happened. Sorry if you get this more than once.

Aug. 2, 2007

Re: dinput - Allow the use of the standard Joystick GUID when calling CreateDevice

by Peter Dons Tychsen

On Fri, 2007-08-03 at 01:02 +0200, Peter Dons Tychsen wrote: > On Thu, 2007-08-02 at 06:54 -0600, Vitaliy Margolen wrote: > > Peter Dons Tychsen wrote: > > > On Tue, 2007-07-31 at 20:05 -0600, Vitaliy Margolen wrote: > > >> Peter Dons Tychsen wrote: > > >>> On Tue, 2007-07-31 at 06:34 -0600, Vitaliy Margolen wrote: > > >>>> Peter Dons Tychsen wrote: > > >>>>> On Mon, 2007-07-30 at 21:20 +0200, Peter Dons Tychsen wrote: > > >>>>>> On Sun, 2007-07-29 at 17:43 -0600, Vitaliy Margolen wrote: > > >>>>>>> Peter Dons Tychsen wrote: > > >>>>>>>> OK. Please review this diff: > > >>>>>>>> > > >>>>>>>> I will re-submit it if you like it. > > >>>>>>>> > > >>>>>>>> /Pedro > > >>>>>>> Alright looks good now (you might want to remove the extra white space your > > >>>>>>> patch adds - git complains about those). Just left to make the same change > > >>>>>>> to the other joystick interface :) > > >>>>>>> > > >>>>>>> BTW don't forget to cc: the wine-devel list when you replying. > > >>>>>>> > > >>>>>>> Vitaliy. > > >>>>>> OK. I still feel that trying to sync joystick_linuxinput.c and > > >>>>>> joystick_linux.c is out of scope for this patch. But as my steadiness > > >>>>>> has limits i have done some of it anyway... :). I have kept the changes > > >>>>>> modest. > > >>>>>> > > >>>>>> Please take a look at the new patch. > > >>>>>> > > >>>>>> On another note i will agree with you that it would be practical if the > > >>>>>> two files had similar (or maybe even shared) code. Redundant code is > > >>>>>> never very practical as it always ends up as this very example. The two > > >>>>>> implementations which were supposed to identical always drift apart. If > > >>>>>> things are supposed to do the same, keep them together to begin with. > > >>>>>> > > >>>>>> Thanks, > > >>>>>> > > >>>>> Maybe i would be more successful if i actually attached the > > >>>>> patch.... :-) > > >>>>> > > >>>>> Here it is.... > > >>>> Just few small things left: > > >>>> > > >>>>> -#undef MAX_JOYSTICKS > > >>>> As I mentioned earlier you should move this instead of removing it completely. > > >>>> > > >>>> > > >>>>> static HRESULT joydev_create_deviceW(IDirectInputImpl *dinput, REFGUID rguid, REFIID riid, LPDIRECTINPUTDEVICEW* pdev) > > >>>>> { > > >>>>> - int i; > > >>>>> - > > >>>>> - find_joydevs(); > > >>>> You removed a really important function call here. > > >>>> > > >>>> > > >>>>> +static HRESULT joydev_create_deviceA(IDirectInputImpl *dinput, REFGUID rguid, REFIID riid, LPDIRECTINPUTDEVICEA* pdev) > > >>>>> +{ > > >>>>> + unsigned short index; > > >>>>> + > > >>>>> + if ((index = get_joystick_index(rguid)) < MAX_JOYDEV) { > > >>>>> + if ((riid == NULL) || > > >>>> If you reformat the whole function, please make it 4-space indented. > > >>>> > > >>> OK. Here is the new patch: > > >>> > > >> Looks good to me. The problem is, it doesn't work for me (USB joystick). I > > >> think you forgot to look at how joystick_linuxinput.c handles that data3 > > >> field. It's not just 0 based index... > > >> > > > I told you i should bot have merged the changes over to > > > joystick_linuxinput as they were too different... :-) > > > > > > OK, so how do i force my system to use the other implementation (so i > > > can test it) ? > > > > > > I have a Logitech WingMan Extreme (old version). > > > > > Just (re)move /dev/input/js*. Or in dinput change device name to point to > > non-existent directory/device (in joystick_linux.c). > > > > Vitaliy. > > OK, fair enough. Even though i did'nt like messing around with > joystick_linuxinput.c i should have been smarter than trying to submit > something i could not test... :-( > > But... tada!... now i have re-fixed it and re-tested both > implementations. Its seems stable now. :-) > > Please give it another go on your machine Vitaliy. > > Cheers, > > /Pedro As usual, i forgot the patch.... getting late inside my head :-) Here it is... /p

Aug. 2, 2007

Re: dinput - Allow the use of the standard Joystick GUID when calling CreateDevice

by Peter Dons Tychsen

On Thu, 2007-08-02 at 06:54 -0600, Vitaliy Margolen wrote: > Peter Dons Tychsen wrote: > > On Tue, 2007-07-31 at 20:05 -0600, Vitaliy Margolen wrote: > >> Peter Dons Tychsen wrote: > >>> On Tue, 2007-07-31 at 06:34 -0600, Vitaliy Margolen wrote: > >>>> Peter Dons Tychsen wrote: > >>>>> On Mon, 2007-07-30 at 21:20 +0200, Peter Dons Tychsen wrote: > >>>>>> On Sun, 2007-07-29 at 17:43 -0600, Vitaliy Margolen wrote: > >>>>>>> Peter Dons Tychsen wrote: > >>>>>>>> OK. Please review this diff: > >>>>>>>> > >>>>>>>> I will re-submit it if you like it. > >>>>>>>> > >>>>>>>> /Pedro > >>>>>>> Alright looks good now (you might want to remove the extra white space your > >>>>>>> patch adds - git complains about those). Just left to make the same change > >>>>>>> to the other joystick interface :) > >>>>>>> > >>>>>>> BTW don't forget to cc: the wine-devel list when you replying. > >>>>>>> > >>>>>>> Vitaliy. > >>>>>> OK. I still feel that trying to sync joystick_linuxinput.c and > >>>>>> joystick_linux.c is out of scope for this patch. But as my steadiness > >>>>>> has limits i have done some of it anyway... :). I have kept the changes > >>>>>> modest. > >>>>>> > >>>>>> Please take a look at the new patch. > >>>>>> > >>>>>> On another note i will agree with you that it would be practical if the > >>>>>> two files had similar (or maybe even shared) code. Redundant code is > >>>>>> never very practical as it always ends up as this very example. The two > >>>>>> implementations which were supposed to identical always drift apart. If > >>>>>> things are supposed to do the same, keep them together to begin with. > >>>>>> > >>>>>> Thanks, > >>>>>> > >>>>> Maybe i would be more successful if i actually attached the > >>>>> patch.... :-) > >>>>> > >>>>> Here it is.... > >>>> Just few small things left: > >>>> > >>>>> -#undef MAX_JOYSTICKS > >>>> As I mentioned earlier you should move this instead of removing it completely. > >>>> > >>>> > >>>>> static HRESULT joydev_create_deviceW(IDirectInputImpl *dinput, REFGUID rguid, REFIID riid, LPDIRECTINPUTDEVICEW* pdev) > >>>>> { > >>>>> - int i; > >>>>> - > >>>>> - find_joydevs(); > >>>> You removed a really important function call here. > >>>> > >>>> > >>>>> +static HRESULT joydev_create_deviceA(IDirectInputImpl *dinput, REFGUID rguid, REFIID riid, LPDIRECTINPUTDEVICEA* pdev) > >>>>> +{ > >>>>> + unsigned short index; > >>>>> + > >>>>> + if ((index = get_joystick_index(rguid)) < MAX_JOYDEV) { > >>>>> + if ((riid == NULL) || > >>>> If you reformat the whole function, please make it 4-space indented. > >>>> > >>> OK. Here is the new patch: > >>> > >> Looks good to me. The problem is, it doesn't work for me (USB joystick). I > >> think you forgot to look at how joystick_linuxinput.c handles that data3 > >> field. It's not just 0 based index... > >> > > I told you i should bot have merged the changes over to > > joystick_linuxinput as they were too different... :-) > > > > OK, so how do i force my system to use the other implementation (so i > > can test it) ? > > > > I have a Logitech WingMan Extreme (old version). > > > Just (re)move /dev/input/js*. Or in dinput change device name to point to > non-existent directory/device (in joystick_linux.c). > > Vitaliy. OK, fair enough. Even though i did'nt like messing around with joystick_linuxinput.c i should have been smarter than trying to submit something i could not test... :-( But... tada!... now i have re-fixed it and re-tested both implementations. Its seems stable now. :-) Please give it another go on your machine Vitaliy. Cheers, /Pedro

Aug. 2, 2007

Bug introduced in 0.9.42 in printer dialog

by Christian Pradelli

A bug was introduced in version 0.9.42, that makes printer dialogs to not work. When you press Accept button in the printer dialog the I receive the followin error: fixme:commdlg:PRINTDLG_UpdatePrintDlgA No lpdm ptr? fixme:commdlg:PRINTDLG_WMCommandA Update printdlg was not successful! Should I post to bugzilla? Regards Christian c_pradelli(a)yahoo.com

Aug. 2, 2007