February 2015 - Wine-Devel - list.winehq.org

Re: [1/3] dpnet: Add check for mismatched string lengths

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, I'm sorry, I missed one problem in my earlier reviews of this change: Am 2015-02-26 um 04:31 schrieb Alistair Leslie-Hughes: > case DPNA_DATATYPE_STRING: > + if (((strlenW((WCHAR*)lpvData)+1)*sizeof(WCHAR)) != dwDataSize) > + return DPNERR_INVALIDPARAM; > heap_free(entry->data.string); At the time when you do the check and the error return you have already allocated a new entry and added it to the list. Thus if the check fails you'll leave a zombie component attached to the address object. This also applies to the already existing DWORD size check. You can move the check in front of the loop that searches if such a component name already exists. I also recommend to to write extra tests for these cases: What happens if a new component with an invalid data size is added? Does GetComponentByName return something? What happens if a component already exists and you try to re-add it with an invalid data? Does the old data stay intact? The other two patches look good to me and seem to apply without this one. I'll inform Alexandre on IRC. The subject of patch 3 is broken though, maybe Alexandre can correct it. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJU7tO3AAoJEN0/YqbEcdMw6z8P/AoWZwr4ysFdMvMBdqlZTVTH bUFYX62n8gj/IADpL1TIq8NN87Q2H2gv+V9p+6IOcvymMkDOVa/jfK+tBtZVLjmy MoLDx/+hJ2Ef8LteV2qiFO4y50btO32ibPrxY6C/jdJ5gj8dHrsjOQQSUiiAHAfQ N4Hl1j9BVblH7elBEa/Anuk+1aArnXncYU3q2wI50HRfgVShDo+3LO8jA4XzQyQM U3Gk0n5YeRapnPH0WAq9FH8o45dUFg3vtkx8M0wpzIHNGsZn91/RlIHOh7m5O9Xx S6mGU9xkhIDoWsRP+I0dIROa/QDVM7aTw8z8jpPVVwc7gS/z7k95BT/skSXw/ryH s4zOZvoS+P4vtJGgQrwnZDpEywJu7p6hdvWcUBEM85ZWvqh2Ui4em9WrW0nsmFeL VC9WcEFG4N8vzAGRK6yF2luyKrygj8KZGukZ2HCJGrjK0pAnln+BSs2V2M4cZMd6 uJus8bKJMcM2oGoB5UbpHMKRlIgSXrO0nwibUYJ02Kp3dpTUG0oHSXer+HFzImvA 2VLinduPKpA0g9XSXtCdUaXZghpQf6K4e7lwDyoikgFZs5x5G6qNYQIx2MtrJwJ2 oQ0YILD8thFi1QQrH7DLqQAGA0kAarDs0wClct5kQ3uiiT3t9a3Rklql/EYPdz3m VvgO1NqMg7yqPq0KMGKl =5ibs -----END PGP SIGNATURE-----

Feb. 26, 2015

Re: [PATCH 1/3] services: Monitor process handles for process quit

by Alexandre Julliard

Andrew Eikum <aeikum(a)codeweavers.com> writes: > @@ -1636,13 +1638,30 @@ DWORD events_loop(void) > > do > { > - err = WaitForMultipleObjects(2, wait_handles, FALSE, timeout); > + num_handles = 2; > + > + /* monitor tracked process handles for process end */ > + EnterCriticalSection(&timeout_queue_cs); > + LIST_FOR_EACH_ENTRY(iter, &timeout_queue, struct timeout_queue_elem, entry) > + { > + if(num_handles == max_handles) > + { > + max_handles++; > + wait_handles = HeapReAlloc(GetProcessHeap(), 0, wait_handles, sizeof(HANDLE) * max_handles); > + } You don't need heap allocation, you can't wait for more than MAXIMUM_WAIT_OBJECTS anyway. -- Alexandre Julliard julliard(a)winehq.org

Feb. 26, 2015

Re: comctl32: SysDateTimePick32 control add Alt+down hot key likewindows(try 2)

by Changhui Liu

Hi Nikolay, I update a new patch, do you think is it OK now ? Thank you. ------------------ Regards, Changhui. ------------------ Original ------------------ From: "Nikolay Sivov"<bunglehead(a)gmail.com>; Date: Wed, Feb 25, 2015 04:12 PM To: "Changhui Liu"<liuchanghui(a)linuxdeepin.com>; Cc: "wine-devel"<wine-devel(a)winehq.org>; Subject: Re: comctl32: SysDateTimePick32 control add Alt+down hot key likewindows(try 2) On 25.02.2015 10:56, Changhui Liu wrote: > Superseded patch 109085 > http://source.winehq.org/patches/data/109085 > > Change log: > 1, Pass test on all Windows. > > 2, Add DTN_DROPDOWN and DTN_CLOSEUPnotification test. > > 3, > >>>/ + case WM_SYSKEYDOWN: > />>/ + if (wParam == VK_DOWN) > />>/ + { > />>/ + POINT pt; > />>/ + RECT rect; > />>/ + GetClientRect(hwnd, &rect); > />>/ + pt.x = rect.right - GetSystemMetrics(SM_CXVSCROLL)/2; > />>/ + pt.y = rect.top + GetSystemMetrics(SM_CYHSCROLL)/2; > />>/ + > />>/ + return DATETIME_LButtonDown (infoPtr, (SHORT)pt.x, (SHORT)pt.y); > />>/ + } > />>/ + > />> >>>If it really works this way you should just make it drop directly, >>without faking click point coordinates. > > Because most of code of the directly implement are same as DATETIME_LButtonDown, > and too many repeat code looks bad. > So faking click point coordinates can reuse code. > I'm not saying you should duplicate it, but reuse it in a clean way without hit testing - move relevant code somewhere and use it. > + if (WM_NOTIFY == message) > + { > + NMHDR* hdr = (NMHDR*)lParam; > + > + if (hdr->code == DTN_DROPDOWN) > + { > + g_hWndMonthCal = (HWND)SendMessageA(hdr->hwndFrom, DTM_GETMONTHCAL, 0, 0); > + trace("open SysMonthCal32=%p\n", g_hWndMonthCal ); > + } > + else if (hdr->code == DTN_CLOSEUP) > + { > + trace("close SysMonthCal32=%p\n", g_hWndMonthCal); > + } > + } That's not a notification test as you don't check anything. Please add a proper message sequence test, we have tons of examples for those.

Feb. 26, 2015

Re: [PATCH 1/1] handling sysex midi messages in winecoreaudio

by tinez

Ok, i hope i got it right this time. Regards, tinez --- dlls/winecoreaudio.drv/midi.c | 68 ++++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 20 deletions(-) W dniu 25.02.2015 o 22:16, Andrew Eikum pisze: > On Wed, Feb 25, 2015 at 09:58:41PM +0100, tinez wrote: >> Hi, >> >> this patch fixes handling sysex midi messages in winecoreaudio >> driver. The code was based on winealsa driver implementation. >> I've tested it using external midi hardware + midi monitoring tools >> (MidiOX). My motivation was to fix Nord Modular Editor software >> which is needed to control Nord Modular G1 hardware synthetizer. >> After applying the patch it works without problems. >> I've also sent out patch to electro-music.com for other people to >> test and it was received well (you can see the thread here >> electro-music.com/forum/viewtopic.php?t=62683) >> > Thanks, looks mostly good. A few Wine style notes in-line below. > >> regards, tinez >> >> --- >> dlls/winecoreaudio.drv/midi.c | 66 >> ++++++++++++++++++++++++++++++------------- >> 1 file changed, 47 insertions(+), 19 deletions(-) >> From e18b1c9876730d9557b97f3e16249b20eae4e875 Mon Sep 17 00:00:00 2001 >> From: tinez <tinez(a)tlen.pl> > Wine requires full, real names for patches. > >> Date: Thu, 21 Aug 2014 23:13:31 +0200 >> Subject: [PATCH 1/1] handling sysex midi messages in winecoreaudio >> >> --- >> dlls/winecoreaudio.drv/midi.c | 66 ++++++++++++++++++++++++++++++------------- >> 1 file changed, 47 insertions(+), 19 deletions(-) >> >> diff --git a/dlls/winecoreaudio.drv/midi.c b/dlls/winecoreaudio.drv/midi.c >> index 8593e9c..c7a1ded 100644 >> --- a/dlls/winecoreaudio.drv/midi.c >> +++ b/dlls/winecoreaudio.drv/midi.c >> @@ -437,9 +437,9 @@ static DWORD MIDIOut_LongData(WORD wDevID, LPMIDIHDR lpMidiHdr, DWORD dwSize) >> return MMSYSERR_ERROR; >> } >> } >> - else >> + else if (destinations[wDevID].caps.wTechnology == MOD_MIDIPORT) >> { >> - FIXME("MOD_MIDIPORT\n"); >> + MIDIOut_Send(MIDIOutPort, destinations[wDevID].dest, lpData, lpMidiHdr->dwBufferLength); >> } >> >> lpMidiHdr->dwFlags &= ~MHDR_INQUEUE; >> @@ -836,26 +836,55 @@ static CFDataRef MIDIIn_MessageHandler(CFMessagePortRef local, SInt32 msgid, CFD >> if (src->state < 1) >> { >> TRACE("input not started, thrown away\n"); >> - goto done; >> - } >> - /* FIXME skipping SysEx */ >> - if (msg->data[0] == 0xF0) >> - { >> - FIXME("Starting System Exclusive\n"); >> - src->state |= 2; >> + return NULL; >> } >> - if (src->state & 2) >> - { >> - for (i = 0; i < msg->length; ++i) >> - { >> - if (msg->data[i] == 0xF7) >> - { >> - FIXME("Ending System Exclusive\n"); >> - src->state &= ~2; >> + >> + bool sysexStart = (msg->data[0] == 0xF0); >> + > Wine uses C89, so we don't have 'bool'. You could change this to a > BOOL instead. > >> + if (sysexStart || src->state & 2) { >> + if (sysexStart) { >> + TRACE("Receiving sysex message\n"); >> + src->state |= 2; >> + } >> + >> + int pos = 0; >> + int len = msg->length; >> + > C89 also doesn't let you declare new variables in the middle of a > block. They must be declared at the start of a block. > >> + EnterCriticalSection(&midiInLock); >> + currentTime = GetTickCount() - src->startTime; >> + >> + while (len) { >> + LPMIDIHDR lpMidiHdr = src->lpQueueHdr; >> + >> + if (lpMidiHdr != NULL) { >> + int copylen = min(len, lpMidiHdr->dwBufferLength - lpMidiHdr->dwBytesRecorded); >> + memcpy(lpMidiHdr->lpData + lpMidiHdr->dwBytesRecorded, msg->data + pos, copylen); >> + lpMidiHdr->dwBytesRecorded += copylen; >> + len -= copylen; >> + pos += copylen; >> + >> + TRACE("Copied %d bytes of sysex message\n", copylen); >> + >> + if ((lpMidiHdr->dwBytesRecorded == lpMidiHdr->dwBufferLength) || >> + (*(BYTE*)(lpMidiHdr->lpData + lpMidiHdr->dwBytesRecorded - 1) == 0xF7)) { >> + TRACE("Sysex message complete (or buffer limit reached), dispatching %d bytes\n", lpMidiHdr->dwBytesRecorded); > Tabs should be spaces, for consistency with the rest of your code. > >> + src->lpQueueHdr = lpMidiHdr->lpNext; >> + lpMidiHdr->dwFlags &= ~MHDR_INQUEUE; >> + lpMidiHdr->dwFlags |= MHDR_DONE; >> + MIDI_NotifyClient(msg->devID, MIM_LONGDATA, (DWORD_PTR)lpMidiHdr, currentTime); >> + src->state &= ~2; >> + } >> + } >> + else { > Here, too. > >> + FIXME("Sysex data received but no buffer to store it!\n"); >> + break; >> } >> } >> - goto done; >> + >> + LeaveCriticalSection(&midiInLock); >> + return NULL; >> } >> + >> EnterCriticalSection(&midiInLock); >> currentTime = GetTickCount() - src->startTime; >> >> @@ -889,7 +918,6 @@ static CFDataRef MIDIIn_MessageHandler(CFMessagePortRef local, SInt32 msgid, CFD >> CFRunLoopStop(CFRunLoopGetCurrent()); >> break; >> } >> -done: >> return NULL; >> } >> >> -- >> 1.9.3 (Apple Git-50)

Feb. 25, 2015

Re: [PATCH 1/1] handling sysex midi messages in winecoreaudio

by Andrew Eikum

On Wed, Feb 25, 2015 at 09:58:41PM +0100, tinez wrote: > Hi, > > this patch fixes handling sysex midi messages in winecoreaudio > driver. The code was based on winealsa driver implementation. > I've tested it using external midi hardware + midi monitoring tools > (MidiOX). My motivation was to fix Nord Modular Editor software > which is needed to control Nord Modular G1 hardware synthetizer. > After applying the patch it works without problems. > I've also sent out patch to electro-music.com for other people to > test and it was received well (you can see the thread here > electro-music.com/forum/viewtopic.php?t=62683) > Thanks, looks mostly good. A few Wine style notes in-line below. > regards, tinez > > --- > dlls/winecoreaudio.drv/midi.c | 66 > ++++++++++++++++++++++++++++++------------- > 1 file changed, 47 insertions(+), 19 deletions(-) > From e18b1c9876730d9557b97f3e16249b20eae4e875 Mon Sep 17 00:00:00 2001 > From: tinez <tinez(a)tlen.pl> Wine requires full, real names for patches. > Date: Thu, 21 Aug 2014 23:13:31 +0200 > Subject: [PATCH 1/1] handling sysex midi messages in winecoreaudio > > --- > dlls/winecoreaudio.drv/midi.c | 66 ++++++++++++++++++++++++++++++------------- > 1 file changed, 47 insertions(+), 19 deletions(-) > > diff --git a/dlls/winecoreaudio.drv/midi.c b/dlls/winecoreaudio.drv/midi.c > index 8593e9c..c7a1ded 100644 > --- a/dlls/winecoreaudio.drv/midi.c > +++ b/dlls/winecoreaudio.drv/midi.c > @@ -437,9 +437,9 @@ static DWORD MIDIOut_LongData(WORD wDevID, LPMIDIHDR lpMidiHdr, DWORD dwSize) > return MMSYSERR_ERROR; > } > } > - else > + else if (destinations[wDevID].caps.wTechnology == MOD_MIDIPORT) > { > - FIXME("MOD_MIDIPORT\n"); > + MIDIOut_Send(MIDIOutPort, destinations[wDevID].dest, lpData, lpMidiHdr->dwBufferLength); > } > > lpMidiHdr->dwFlags &= ~MHDR_INQUEUE; > @@ -836,26 +836,55 @@ static CFDataRef MIDIIn_MessageHandler(CFMessagePortRef local, SInt32 msgid, CFD > if (src->state < 1) > { > TRACE("input not started, thrown away\n"); > - goto done; > - } > - /* FIXME skipping SysEx */ > - if (msg->data[0] == 0xF0) > - { > - FIXME("Starting System Exclusive\n"); > - src->state |= 2; > + return NULL; > } > - if (src->state & 2) > - { > - for (i = 0; i < msg->length; ++i) > - { > - if (msg->data[i] == 0xF7) > - { > - FIXME("Ending System Exclusive\n"); > - src->state &= ~2; > + > + bool sysexStart = (msg->data[0] == 0xF0); > + Wine uses C89, so we don't have 'bool'. You could change this to a BOOL instead. > + if (sysexStart || src->state & 2) { > + if (sysexStart) { > + TRACE("Receiving sysex message\n"); > + src->state |= 2; > + } > + > + int pos = 0; > + int len = msg->length; > + C89 also doesn't let you declare new variables in the middle of a block. They must be declared at the start of a block. > + EnterCriticalSection(&midiInLock); > + currentTime = GetTickCount() - src->startTime; > + > + while (len) { > + LPMIDIHDR lpMidiHdr = src->lpQueueHdr; > + > + if (lpMidiHdr != NULL) { > + int copylen = min(len, lpMidiHdr->dwBufferLength - lpMidiHdr->dwBytesRecorded); > + memcpy(lpMidiHdr->lpData + lpMidiHdr->dwBytesRecorded, msg->data + pos, copylen); > + lpMidiHdr->dwBytesRecorded += copylen; > + len -= copylen; > + pos += copylen; > + > + TRACE("Copied %d bytes of sysex message\n", copylen); > + > + if ((lpMidiHdr->dwBytesRecorded == lpMidiHdr->dwBufferLength) || > + (*(BYTE*)(lpMidiHdr->lpData + lpMidiHdr->dwBytesRecorded - 1) == 0xF7)) { > + TRACE("Sysex message complete (or buffer limit reached), dispatching %d bytes\n", lpMidiHdr->dwBytesRecorded); Tabs should be spaces, for consistency with the rest of your code. > + src->lpQueueHdr = lpMidiHdr->lpNext; > + lpMidiHdr->dwFlags &= ~MHDR_INQUEUE; > + lpMidiHdr->dwFlags |= MHDR_DONE; > + MIDI_NotifyClient(msg->devID, MIM_LONGDATA, (DWORD_PTR)lpMidiHdr, currentTime); > + src->state &= ~2; > + } > + } > + else { Here, too. > + FIXME("Sysex data received but no buffer to store it!\n"); > + break; > } > } > - goto done; > + > + LeaveCriticalSection(&midiInLock); > + return NULL; > } > + > EnterCriticalSection(&midiInLock); > currentTime = GetTickCount() - src->startTime; > > @@ -889,7 +918,6 @@ static CFDataRef MIDIIn_MessageHandler(CFMessagePortRef local, SInt32 msgid, CFD > CFRunLoopStop(CFRunLoopGetCurrent()); > break; > } > -done: > return NULL; > } > > -- > 1.9.3 (Apple Git-50)

Feb. 25, 2015

Re: urlmon: Added IInternetBindInfo implementation to BindStatusCallback object.

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 https://testbot.winehq.org/JobDetails.pl?Key=11740 Your paranoid android. === w2000pro (32 bit url) === url.c:2683: Test failed: Could not get IInternetBindInfo interface: 80004002 url: unhandled exception c0000005 at 0044A249

Feb. 25, 2015

Re: [PATCH 3/5] wined3d: Remove ARB_/EXT_TEXTURE_ENV_ADD.

by Matteo Bruni

2015-02-25 9:42 GMT+01:00 Henri Verbeet <hverbeet(a)gmail.com>: > On 24 February 2015 at 13:38, Matteo Bruni <mbruni(a)codeweavers.com> wrote: >> Unused. >> --- > But they should be used by ffp_fragment_get_caps(), since set_tex_op() > uses that functionality. It's not all that clear and nice. set_tex_op() really depends on GL_(ARB|EXT)_texture_env_combine, since it's always setting GL_TEXTURE_ENV_MODE to GL_COMBINE (or GL_COMBINE4_NV) and using its knobs to set the texture color or alpha operation state (and those include the ADD setting for the COMBINE pname). There is actually no instance of GL_TEXTURE_ENV_MODE being set to GL_ADD in wined3d, which is the only addition in the GL_ARB_texture_env_add spec. So TEXTURE_ENV_ADD is really unused AFAICS. We do always depend on GL_(ARB|EXT)_texture_env_combine and we're not exposing that dependency correctly. I'm not sure I want to spend much time on fixing that or introducing a fallback for the case where those extensions are missing. Maybe I should just add an early-exit branch in set_tex_op for the no *_texture_env_combine case, handling only the few settings supported in unextended OpenGL (which are MODULATE, SELECTARG1 and SELECTARG2 I think) and update ffp_fragment_get_caps() accordingly.

Feb. 25, 2015

Re: (try 6)[1/5] imm32: Move thread data from TLSEntry to an internal list

by Aric Stewart

On 2/25/15 1:57 AM, Alexandre Julliard wrote: > Aric Stewart <aric(a)codeweavers.com> writes: > > > You can't create a window while holding the critical section, this will > cause deadlocks. > Thanks, fixed and resent -aric

Feb. 25, 2015

Re: mshtml: Correctly handle S_OK with NULL URL return from TranslateUrl.

by Jacek Caban

On 02/25/15 13:10, Marvin wrote: > 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 > https://testbot.winehq.org/JobDetails.pl?Key=11724 > > Your paranoid android. > > > === wvistau64 (32 bit htmldoc) === > The test timed out This seems random and unrelated: https://testbot.winehq.org/JobDetails.pl?Key=11725 Jacek

Feb. 25, 2015

Re: mshtml: Correctly handle S_OK with NULL URL return from TranslateUrl.

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 https://testbot.winehq.org/JobDetails.pl?Key=11724 Your paranoid android. === wvistau64 (32 bit htmldoc) === The test timed out

Feb. 25, 2015