March 2017 - Wine-Devel - list.winehq.org

Re: [PATCH 3/3] wininet/test: Improve tests for InternetGetConnectedStateEx
by Bruno Jesus March 6, 2017

March 6, 2017

On Mon, Mar 6, 2017 at 11:24 AM, Jacek Caban <jacek(a)codeweavers.com> wrote: > Hi Bruno, Hi, thanks for the review ;-) > On 06.03.2017 05:24, Bruno Jesus wrote: >> - ok(sz - 1 == lstrlenW(buffer), "Expected %u bytes, got %u\n", sz - 1, lstrlenW(buffer)); >> + if (flags & INTERNET_CONNECTION_MODEM) >> + ok(lstrlenW(buffer) == 0, "Expected 0 bytes, got %u\n", lstrlenW(buffer)); >> + else >> + ok(sz - 1 == lstrlenW(buffer), "Expected %u bytes, got %u\n", sz - 1, lstrlenW(buffer)); > > > You're dropping sz value tests in case is set. What is it set to in this > case? 0? > > As a side note, you don't need strlen() to check if buffer is empty, > something like !*buffer will do the trick. You are right, I will make the change to test both !buffer[0] and sz == 0 in the three cases.

1 0

Re: [PATCH 3/3] winmm: Dispatch MCI_BREAK to the driver (try 2, resend)
by Andrew Eikum March 6, 2017

March 6, 2017

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> On Mon, Mar 06, 2017 at 01:24:28AM -0300, Bruno Jesus wrote: > From: Bruno Jesus <bjesus(a)codeweavers.com> > > try 2: > Fix trace (spotted by Sebastian Lackner) > > Worms 2: > trace:mci:MCI_Break (00000001, 00000100, vkey 001B, hwnd 0x2003c) > > Half-Life: > trace:mci:MCI_Break (00000002, 00000102, vkey 001B, hwnd (nil)) > > Signed-off-by: Bruno Jesus <bjesus(a)codeweavers.com> > --- > dlls/winmm/mci.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/dlls/winmm/mci.c b/dlls/winmm/mci.c > index f772388..56a537d 100644 > --- a/dlls/winmm/mci.c > +++ b/dlls/winmm/mci.c > @@ -2063,13 +2063,18 @@ static DWORD MCI_SysInfo(UINT uDevID, DWORD dwFlags, LPMCI_SYSINFO_PARMSW lpParm > */ > static DWORD MCI_Break(UINT wDevID, DWORD dwFlags, LPMCI_BREAK_PARMS lpParms) > { > + DWORD dwRet; > + > if (lpParms == NULL) > return MCIERR_NULL_PARAMETER_BLOCK; > - FIXME("(%04x) vkey %04X stub\n", dwFlags, lpParms->nVirtKey); > > - if (dwFlags & MCI_NOTIFY) > + TRACE("(%08x, %08X, vkey %04X, hwnd %p)\n", wDevID, dwFlags, > + lpParms->nVirtKey, lpParms->hwndBreak); > + > + dwRet = MCI_SendCommandFrom32(wDevID, MCI_BREAK, dwFlags, (DWORD_PTR)lpParms); > + if (!dwRet && (dwFlags & MCI_NOTIFY)) > mciDriverNotify((HWND)lpParms->dwCallback, wDevID, MCI_NOTIFY_SUCCESSFUL); > - return MMSYSERR_NOERROR; > + return dwRet; > } > > /************************************************************************** > -- > 2.9.3 > > >

1 0

Re: [PATCH 1/4] user32: Don't forward EnableWindow to the window's thread.
by Sebastian Lackner March 5, 2017

March 5, 2017

On 05.03.2017 20:21, Zebediah Figura wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=42568 > > The only apparent use of this is to kill the focus and capture of > the window if the thread doesn't own it. However, testing reveals > that on Windows the focus isn't killed either, and the capture is > handled by DefWindowProc, as already implemented in Wine. > > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/user32/message.c | 3 --- > dlls/user32/spy.c | 1 - > dlls/user32/tests/msg.c | 8 +++----- > dlls/user32/user_private.h | 1 - > dlls/user32/win.c | 12 ------------ > 5 files changed, 3 insertions(+), 22 deletions(-) > You will at least have to forward for other processes (see for example the WIN_SetStyle implementation). Best regards, Sebastian

2 1

Re: gdiplus: Check for null in GdipGetFontHeight
by Vincent Povirk March 5, 2017

March 5, 2017

This fails the gdiplus:font tests. font.c:550: Test failed: Expected 0, got 2 font.c:627: Test failed: font_height 63875314620738696996458594304.000000 != 15.691406 font.c:640: Test failed: font_height 63875314620738696996458594304.000000 != 15.691406 font.c:550: Test failed: Expected 0, got 2 font.c:668: Test failed: font_height 63875314620738696996458594304.000000 != 13.277344 font.c:681: Test failed: font_height 63875314620738696996458594304.000000 != 13.277344 font.c:550: Test failed: Expected 0, got 2 font.c:710: Test failed: font_height 63875314620738696996458594304.000000 != 15.691406

1 0

Re: [PATCH 4/4] user32: Only process sent messages from driver in PeekMessage().
by Marvin March 5, 2017

March 5, 2017

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=29130 Your paranoid android. === w7u (32 bit win) === win.c:1003: Test failed: wrong dwWindowStatus: 0000 != 0001 active 00130156 fg 000301B4 in hook HCBT_MINMAX

1 0

Re: [PATCH 2/2] user32: scrollbars are vertical by default
by Nikolay Sivov March 5, 2017

March 5, 2017

On 04.03.2017 23:29, Kees Beets wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=38029 > > Changed calculation of boolean vertical so that default is true. > > Tested on Ubuntu 16.04. > > Signed-off-by: kbeets68(a)gmail.com > --- > dlls/user32/scroll.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/user32/scroll.c b/dlls/user32/scroll.c > index 4b572e5..202fac8 100644 > --- a/dlls/user32/scroll.c > +++ b/dlls/user32/scroll.c > @@ -252,7 +252,7 @@ static BOOL SCROLL_GetScrollBarRect( HWND hwnd, INT nBar, RECT *lprect, > > case SB_CTL: > GetClientRect( hwnd, lprect ); > - vertical = ((wndPtr->dwStyle & SBS_VERT) != 0); > + vertical = ((wndPtr->dwStyle & SBS_HORZ) == 0); > break; > > default: > Hi, this doesn't look right, SBS_HORZ is 0, which makes 'vertical' always 1.

2 1

Re: GSOC 2017 - Resource/Dialog Editor
by Austin English March 5, 2017

March 5, 2017

On Mar 3, 2017 3:16 AM, "Joseph-Eugene Winzer" <ml999(a)openmailbox.org> wrote: On 03/02/2017 10:45 PM, Austin English wrote: > On Thu, Mar 2, 2017 at 1:58 PM, Joe <ml999(a)openmailbox.org> wrote: > >> Hi, >> >> First of all, congrats for getting picked for another GSoC! >> Wine made my transition to Linux way easier and with more and more >> applications being supported the year of the desktop can come :) >> >> My name is Joseph Winzer and I am a bachelor math/CS student looking >> forward >> to contribute to open source in this year's GSoC. >> The 'Resource/Dialog Editor' project caught my eye as I remember a few >> years >> back digging through executables out of curiousity. Last time I looked >> for a >> resource editor it was either bundled with other software (Visual Studio, >> PellesC) or as mentioned in the project description bundled up with adware >> like ResEdit. >> I want to change that by contributing to Wine with an open source >> standalone >> editor anyone can use free of charge. >> Equipped with my Petzold, MSDN and the help of the community I am sure I >> will be able to realize this project. >> >> >> Looking forward working with you guys :) >> > > Hi Joseph, > > Thanks for your interest. Could you describe your experience with > C/Wine a bit? Have you thought about how you would implement/design > the resource editor at all? Could you share your plans? > > Thanks for your reply. So far, I experienced Wine exclusively from a user perspective, using it mostly for running games and apps I didn't find a substitute on Linux. I compiled Wine a few times when certain milestones were reached like improved D3D9 or bugfixes for certain apps. I did grep for stubs once in a while, browsed through the GDI implementation but wasn't confident enough to contribute. OK. Note that contributing a patch is a requirement of applicants. Patches related to the project are highly encouraged, but not required. I read more code than I wrote myself and most of it has been C or C-style C++. WinAPI-wise, I remember writing small games like connect four or apps where I tried to use what I had recently learned. I also did some reverse engineering for writing trainers or just for the fun of it. That has been awhile, though. What did you reverse engineer? If it's Microsoft code, or if you've seen Microsoft code, you're (in general) ineligible to contribute to Wine. More recently I looked at quake, tuxpaint, util-linux, .. whatever caught my interest. Concerning the technical aspects of implementing the editor, I haven't given it much thought yet. MSDN et al provide enough documentation to tackle the task and problems can be dealt with once they come up (I assume in my naivete). Design-wise it depends on what the Wine community expects it to be but personally I imagined ResEdit trimmed to reading out and altering resources with dialog creation on top. From there it can be extended in whatever direction the community sees fit. The rough sketch would be as follows: * Establishing bare bones UI (sidebar for items, main window as viewer) * Read PE compatible file resources * Export resources * Modify resources (substitute images, edit items) * Refining UI for editing/viewing This covers the basic functionality of the resource viewer. The Dialog Editor provides rendering of dialog elements what would complement part of the resource viewer and exporting as .rc. My estimate is that the dialog editor will be the biggest part of the project with parsing resources, dialog rendering, layout, .. Overall the planning and focus of the project depends on what the community expects it to be and if there is an immediate use case it can cover in Wine or thought of as a general tool. OK. Sorry for the wall of text but I hope I could describe to you my idea I have for it. Of course, in the official application the gaps for the implementation will be filled but getting feedback from the community first makes planning also easier for me. IMO walls of text are good. I'd rather see more text than brief descriptions that show a lack of thought/research ;) -- -Austin GPG: 14FB D7EA A041 937B

1 0

some source files are not listed at dl.winehq.org/wine/source/2.x/
by version2013＠openmailbox.org March 4, 2017

March 4, 2017

https://dl.winehq.org/wine/source/2.x/ wine-2.2 and wine-2.3 are not listed. If I manually type the expected filename in the url, then I can download them.

4 4

Re: regedit/tests: Add some line concatenation tests
by Alexandre Julliard March 3, 2017

March 3, 2017

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > programs/regedit/tests/regedit.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) This fails here: ../../../tools/runtest -q -P wine -T ../../.. -M regedit.exe -p regedit.exe_test.exe.so regedit && touch regedit.ok regedit.c:484: Test succeeded inside todo block: Reg value shouldn't exist: Multi-Line2 regedit.c:496: Test succeeded inside todo block: Reg value shouldn't exist: Multi-Line4 Makefile:174: recipe for target 'regedit.ok' failed make[1]: *** [regedit.ok] Error 2 -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: ntdll: Don't modify LockCount in RtlLeaveCriticalSection if section is not acquired.
by Sebastian Lackner March 3, 2017

March 3, 2017

On 03.03.2017 18:48, Jacek Caban wrote: > Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> > --- > dlls/ntdll/critsection.c | 6 ++++- > dlls/ntdll/tests/rtl.c | 59 > ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 64 insertions(+), 1 deletion(-) > > > > 0001-ntdll-Don-t-modify-LockCount-in-RtlLeaveCriticalSecti.diff > > > diff --git a/dlls/ntdll/critsection.c b/dlls/ntdll/critsection.c > index e2eb364..e405b08 100644 > --- a/dlls/ntdll/critsection.c > +++ b/dlls/ntdll/critsection.c > @@ -665,7 +665,11 @@ BOOL WINAPI RtlIsCriticalSectionLockedByThread( RTL_CRITICAL_SECTION *crit ) > */ > NTSTATUS WINAPI RtlLeaveCriticalSection( RTL_CRITICAL_SECTION *crit ) > { > - if (--crit->RecursionCount) interlocked_dec( &crit->LockCount ); > + if (--crit->RecursionCount) > + { > + if (crit->RecursionCount > 0) interlocked_dec( &crit->LockCount ); > + else ERR( "section %p is not acquired\n", crit ); > + } > else > { > crit->OwningThread = 0; > diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c > index 43c26bd..4b6ee35 100644 > --- a/dlls/ntdll/tests/rtl.c > +++ b/dlls/ntdll/tests/rtl.c > @@ -98,6 +98,9 @@ static NTSTATUS (WINAPI *pRtlCompressBuffer)(USHORT, const UCHAR*, ULONG, PUCHA > static BOOL (WINAPI *pRtlIsCriticalSectionLocked)(CRITICAL_SECTION *); > static BOOL (WINAPI *pRtlIsCriticalSectionLockedByThread)(CRITICAL_SECTION *); > static NTSTATUS (WINAPI *pRtlInitializeCriticalSectionEx)(CRITICAL_SECTION *, ULONG, ULONG); > +static NTSTATUS (WINAPI *pRtlInitializeCriticalSection)(CRITICAL_SECTION *); > +static NTSTATUS (WINAPI *pRtlEnterCriticalSection)(RTL_CRITICAL_SECTION *); > +static NTSTATUS (WINAPI *pRtlLeaveCriticalSection)(RTL_CRITICAL_SECTION *); > > static HMODULE hkernel32 = 0; > static BOOL (WINAPI *pIsWow64Process)(HANDLE, PBOOL); > @@ -151,6 +154,9 @@ static void InitFunctionPtrs(void) > pRtlIsCriticalSectionLocked = (void *)GetProcAddress(hntdll, "RtlIsCriticalSectionLocked"); > pRtlIsCriticalSectionLockedByThread = (void *)GetProcAddress(hntdll, "RtlIsCriticalSectionLockedByThread"); > pRtlInitializeCriticalSectionEx = (void *)GetProcAddress(hntdll, "RtlInitializeCriticalSectionEx"); > + pRtlInitializeCriticalSection = (void *)GetProcAddress(hntdll, "RtlInitializeCriticalSection"); > + pRtlEnterCriticalSection = (void *)GetProcAddress(hntdll, "RtlEnterCriticalSection"); > + pRtlLeaveCriticalSection = (void *)GetProcAddress(hntdll, "RtlLeaveCriticalSection"); Those functions should not require dynamic loading. The *Ex function is only loaded dynamically because it was introduced in later Windows versions. > } > hkernel32 = LoadLibraryA("kernel32.dll"); > ok(hkernel32 != 0, "LoadLibrary failed\n"); > @@ -2094,6 +2100,58 @@ static void test_RtlInitializeCriticalSectionEx(void) > RtlDeleteCriticalSection(&cs); > } > > +static void test_RtlLeaveCriticalSection(void) > +{ > + RTL_CRITICAL_SECTION cs; > + NTSTATUS status; > + > + pRtlInitializeCriticalSection(&cs); > + > + status = pRtlEnterCriticalSection(&cs); > + ok(!status, "RtlEnterCriticalSection failed: %x\n", status); > + todo_wine > + ok(cs.LockCount == -2, "expected LockCount == -2, got %d\n", cs.LockCount); Some of these tests seem to fail on XP (see testbot). > + ok(cs.RecursionCount == 1, "expected RecursionCount == 1, got %d\n", cs.RecursionCount); > + ok(cs.OwningThread == ULongToHandle(GetCurrentThreadId()), "unexpected OwningThread\n"); > + > + status = pRtlLeaveCriticalSection(&cs); > + ok(!status, "RtlLeaveCriticalSection failed: %x\n", status); > + ok(cs.LockCount == -1, "expected LockCount == -1, got %d\n", cs.LockCount); > + ok(cs.RecursionCount == 0, "expected RecursionCount == 0, got %d\n", cs.RecursionCount); > + ok(!cs.OwningThread, "unexpected OwningThread %p\n", cs.OwningThread); > + > + /* > + * Trying to leave a section that wasn't acquired modifies RecusionCount to an invalid values, Should this be "invalid value" ? > + * but doesn't modify LockCount so that an attempt to enter the section later will work. > + */ > + status = pRtlLeaveCriticalSection(&cs); > + ok(!status, "RtlLeaveCriticalSection failed: %x\n", status); > + ok(cs.LockCount == -1, "expected LockCount == -1, got %d\n", cs.LockCount); > + ok(cs.RecursionCount == -1, "expected RecursionCount == -1, got %d\n", cs.RecursionCount); > + ok(!cs.OwningThread, "unexpected OwningThread %p\n", cs.OwningThread); > + > + /* and again */ > + status = pRtlLeaveCriticalSection(&cs); > + ok(!status, "RtlLeaveCriticalSection failed: %x\n", status); > + ok(cs.LockCount == -1, "expected LockCount == -1, got %d\n", cs.LockCount); > + ok(cs.RecursionCount == -2, "expected RecursionCount == -1, got %d\n", cs.RecursionCount); Copy and paste error in the ok message. > + ok(!cs.OwningThread, "unexpected OwningThread %p\n", cs.OwningThread); > + > + /* entering section fixes RecursionCount */ > + status = pRtlEnterCriticalSection(&cs); > + ok(!status, "RtlEnterCriticalSection failed: %x\n", status); > + todo_wine > + ok(cs.LockCount == -2, "expected LockCount == -2, got %d\n", cs.LockCount); > + ok(cs.RecursionCount == 1, "expected RecursionCount == 1, got %d\n", cs.RecursionCount); > + ok(cs.OwningThread == ULongToHandle(GetCurrentThreadId()), "unexpected OwningThread\n"); > + > + status = pRtlLeaveCriticalSection(&cs); > + ok(!status, "RtlLeaveCriticalSection failed: %x\n", status); > + ok(cs.LockCount == -1, "expected LockCount == -1, got %d\n", cs.LockCount); > + ok(cs.RecursionCount == 0, "expected RecursionCount == 0, got %d\n", cs.RecursionCount); > + ok(!cs.OwningThread, "unexpected OwningThread %p\n", cs.OwningThread); Please delete the critical section after the test to avoid leaks. > +} > + > START_TEST(rtl) > { > InitFunctionPtrs(); > @@ -2125,4 +2183,5 @@ START_TEST(rtl) > test_RtlDecompressBuffer(); > test_RtlIsCriticalSectionLocked(); > test_RtlInitializeCriticalSectionEx(); > + test_RtlLeaveCriticalSection(); > } > > >

3 3

Re: ntdll: Don't modify LockCount in RtlLeaveCriticalSection if section is not acquired.
by Marvin March 3, 2017

March 3, 2017

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=29096 Your paranoid android. === wxppro (32 bit rtl) === rtl.c:2113: Test failed: expected LockCount == -2, got 0 rtl.c:2129: Test failed: expected LockCount == -1, got -2 rtl.c:2136: Test failed: expected LockCount == -1, got -3 ntdll:rtl has unreported failures

1 0

Re: [PATCH 2/2] ntdll: Fix NtFlushBuffersFile() for files with FILE_APPEND_DATA access.
by Paul Gofman March 3, 2017

March 3, 2017

Sorry for introducing that in f7d40d45da7a1bc0accd58bca6d6da4e34861937. I just verified that server_get_unix_fd() returns success if to request FILE_APPEND_DATA for handles created with (read/)write access. I also checked that requesting FILE_APPEND_DATA gets no error if using it with a usb serial port. Is it anything bad to just use FILE_APPEND_DATA instead of FILE_WRITE_DATA in server_get_unix_fd()? If its not that I am missing something why it should not be done like that, I can add test for serial port. On 03/03/2017 06:21 PM, Józef Kucia wrote: > This fixes a regression introduced by commit > f7d40d45da7a1bc0accd58bca6d6da4e34861937. > > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > dlls/ntdll/file.c | 2 ++ > dlls/ntdll/tests/file.c | 1 - > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c > index 3f38c6f..e2cfe7f 100644 > --- a/dlls/ntdll/file.c > +++ b/dlls/ntdll/file.c > @@ -3418,6 +3418,8 @@ NTSTATUS WINAPI NtFlushBuffersFile( HANDLE hFile, IO_STATUS_BLOCK* IoStatusBlock > int fd, needs_close; > > ret = server_get_unix_fd( hFile, FILE_WRITE_DATA, &fd, &needs_close, &type, NULL ); > + if (ret == STATUS_ACCESS_DENIED) > + ret = server_get_unix_fd( hFile, FILE_APPEND_DATA, &fd, &needs_close, &type, NULL ); > > if (!ret && type == FD_TYPE_SERIAL) > { > diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c > index 5e7e1ee..09bf688 100644 > --- a/dlls/ntdll/tests/file.c > +++ b/dlls/ntdll/tests/file.c > @@ -4441,7 +4441,6 @@ static void test_flush_buffers_file(void) > ok(hfile != INVALID_HANDLE_VALUE, "could not open temp file, error %d.\n", GetLastError()); > > status = pNtFlushBuffersFile(hfile, &io_status_block); > - todo_wine > ok(status == STATUS_SUCCESS, "expected STATUS_SUCCESS, got %#x.\n", status); > > CloseHandle(hfile);

2 1

GSOC 2017 - Resource/Dialog Editor
by Joe March 3, 2017

March 3, 2017

Hi, First of all, congrats for getting picked for another GSoC! Wine made my transition to Linux way easier and with more and more applications being supported the year of the desktop can come :) My name is Joseph Winzer and I am a bachelor math/CS student looking forward to contribute to open source in this year's GSoC. The 'Resource/Dialog Editor' project caught my eye as I remember a few years back digging through executables out of curiousity. Last time I looked for a resource editor it was either bundled with other software (Visual Studio, PellesC) or as mentioned in the project description bundled up with adware like ResEdit. I want to change that by contributing to Wine with an open source standalone editor anyone can use free of charge. Equipped with my Petzold, MSDN and the help of the community I am sure I will be able to realize this project. Looking forward working with you guys :)

3 2

Re: [PATCH 1/2] ws2_32/tests: Rework WSAStringToAddress tests
by Marvin March 2, 2017

March 2, 2017

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=29078 Your paranoid android. === w2008s64 (64 bit sock) === ws2_32:sock returned an error (3221225477) despite having no failures ws2_32:sock has no test summary line (early exit of the main process?)

2 1

Re: [PATCH] ws2_32: Turn SIO_GET_EXTENSION_FUNCTION_POINTER into a table
by Sebastian Lackner March 2, 2017

March 2, 2017

On 02.03.2017 06:38, Bruno Jesus wrote: > From: Bruno Jesus <bjesus(a)codeweavers.com> > > I could not find any better way to avoid compilation warning than using a typedef for the function pointer. If there is a better way I'm all ears. > > Signed-off-by: Bruno Jesus <bjesus(a)codeweavers.com> > --- > dlls/ws2_32/socket.c | 94 +++++++++++++++++++++++----------------------------- > 1 file changed, 41 insertions(+), 53 deletions(-) It works fine here without any typedefs: --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -4945,10 +4945,8 @@ INT WINAPI WSAIoctl(SOCKET s, DWORD code, LPVOID in_buff, DWORD in_size, LPVOID if (found) { - typedef void (*func_ptr)(void); - TRACE("-> got %s\n", guid_funcs[i].name); - *(func_ptr *)out_buff = guid_funcs[i].func_ptr; + *(void **)out_buff = guid_funcs[i].func_ptr; break; } What kind of compiler warning do you get?

2 1

Re: [PATCH 2/2] comctl32/propsheet: Implement PSPCB_ADDREF/PSPCB_RELEASE notifications
by Dmitry Timoshkov March 2, 2017

March 2, 2017

Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/comctl32/propsheet.c | 41 +++++++++++++++++++++++++++++++++++------ > dlls/comctl32/tests/propsheet.c | 4 ---- > 2 files changed, 35 insertions(+), 10 deletions(-) And of course it's a pure coincidence that you immediately started to work on PSPCB_ADDREF/PSPCB_RELEASE once a patch for it has been added to wine-staging. I'd suggest to stop such ill-behaved practices, or at least give a credit to the source if you decided to "borrow" an idea. Or is this an indication that you just ran out of your own ideas to work on? :) -- Dmitry.

3 4

Re: [PATCH 1/2] mciavi32: Implement MCI_BREAK
by Andrew Eikum March 2, 2017

March 2, 2017

On Wed, Mar 01, 2017 at 04:10:43AM -0300, Bruno Jesus wrote: > + wma->mci_break.flags = dwFlags; > + wma->mci_break.parms = *lpParms; The rest of this looks OK to me, but please guard access to wma's members with wma->cs. Andrew

1 0

Re: [v2 2/4] dinput: Handle username in EnumDevicesBySemantics.
by Andrew Eikum March 2, 2017

March 2, 2017

Thanks for fixing these up, they look good to me. However this patch introduces a test failure: dinput.c:573: Test succeeded inside todo block: No device should be available after action mapping available=0 dinput.c:596: Test succeeded inside todo block: This user should own no devices owned=0 This is obviously fixed by your 4/4 patch, but we like to keep the tests clean in every commit. It's fine to squash the test changes into the code changes to create a 2-patch sequence with no test failures. Then I think it should be good to go in. Thanks, Andrew On Thu, Mar 02, 2017 at 12:11:24AM +0200, Jetro Jormalainen wrote: > EnumDevicesBySemantics enums only devices with given username when > DIEDBSFL_THISUSER is set and only unowned devices when > DIEDBSFL_AVAILABLEDEVICES is set. > > v2: Fixed problems noted by Andrew Eikum. > > Tested on Arch Linux and Rally Trophy Demo. > > Signed-off-by: Jetro Jormalainen <jje-wine(a)jv.jetro.fi> > --- > dlls/dinput/dinput_main.c | 111 +++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 96 insertions(+), 15 deletions(-) > > diff --git a/dlls/dinput/dinput_main.c b/dlls/dinput/dinput_main.c > index 2bf9f38ba4..a59217472c 100644 > --- a/dlls/dinput/dinput_main.c > +++ b/dlls/dinput/dinput_main.c > @@ -923,6 +923,47 @@ static HRESULT WINAPI IDirectInput8WImpl_FindDevice(LPDIRECTINPUT8W iface, REFGU > return IDirectInput2WImpl_FindDevice( &This->IDirectInput7W_iface, rguid, pszName, pguidInstance ); > } > > +static BOOL should_enumerate_device(const WCHAR *username, DWORD dwFlags, > + struct list *device_players, REFGUID guid) > +{ > + BOOL should_enumerate = TRUE; > + struct DevicePlayer *device_player; > + > + /* Check if user owns this device */ > + if (dwFlags & DIEDBSFL_THISUSER && username && *username) > + { > + should_enumerate = FALSE; > + LIST_FOR_EACH_ENTRY(device_player, device_players, struct DevicePlayer, entry) > + { > + if (IsEqualGUID(&device_player->instance_guid, guid)) > + { > + if (*device_player->username && !lstrcmpW(username, device_player->username)) > + return TRUE; /* Device username matches */ > + break; > + } > + } > + } > + > + /* Check if this device is not owned by anyone */ > + if (dwFlags & DIEDBSFL_AVAILABLEDEVICES) { > + BOOL found = FALSE; > + should_enumerate = FALSE; > + LIST_FOR_EACH_ENTRY(device_player, device_players, struct DevicePlayer, entry) > + { > + if (IsEqualGUID(&device_player->instance_guid, guid)) > + { > + if (*device_player->username) > + found = TRUE; > + break; > + } > + } > + if (!found) > + return TRUE; /* Device does not have a username */ > + } > + > + return should_enumerate; > +} > + > static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics( > LPDIRECTINPUT8A iface, LPCSTR ptszUserName, LPDIACTIONFORMATA lpdiActionFormat, > LPDIENUMDEVICESBYSEMANTICSCBA lpCallback, > @@ -939,6 +980,7 @@ static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics( > int device_count = 0; > int remain; > DIDEVICEINSTANCEA *didevis = 0; > + WCHAR *username_w = 0; > > FIXME("(this=%p,%s,%p,%p,%p,%04x): semi-stub\n", This, debugstr_a(ptszUserName), lpdiActionFormat, > lpCallback, pvRef, dwFlags); > @@ -955,6 +997,14 @@ static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics( > > didevi.dwSize = sizeof(didevi); > > + if (ptszUserName) > + { > + int len = MultiByteToWideChar(CP_ACP, 0, ptszUserName, -1, 0, 0); > + > + username_w = HeapAlloc(GetProcessHeap(), 0, sizeof(WCHAR)*len); > + MultiByteToWideChar(CP_ACP, 0, ptszUserName, -1, username_w, len); > + } > + > /* Enumerate all the joysticks */ > for (i = 0; i < NB_DINPUT_DEVICES; i++) > { > @@ -968,7 +1018,8 @@ static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics( > > /* Default behavior is to enumerate attached game controllers */ > enumSuccess = dinput_devices[i]->enum_deviceA(DI8DEVCLASS_GAMECTRL, DIEDFL_ATTACHEDONLY | dwFlags, &didevi, This->dwVersion, j); > - if (enumSuccess == S_OK) > + if (enumSuccess == S_OK && > + should_enumerate_device(username_w, dwFlags, &This->device_players, &didevi.guidInstance)) > { > if (device_count++) > didevis = HeapReAlloc(GetProcessHeap(), 0, didevis, sizeof(DIDEVICEINSTANCEA)*device_count); > @@ -980,8 +1031,15 @@ static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics( > } > > remain = device_count; > + /* Add keyboard and mouse to remaining device count */ > if (!(dwFlags & DIEDBSFL_FORCEFEEDBACK)) > - remain += sizeof(guids)/sizeof(guids[0]); > + { > + for (i = 0; i < sizeof(guids) / sizeof(guids[0]); i++) > + { > + if (should_enumerate_device(username_w, dwFlags, &This->device_players, guids[i])) > + remain++; > + } > + } > > for (i = 0; i < device_count; i++) > { > @@ -991,26 +1049,38 @@ static HRESULT WINAPI IDirectInput8AImpl_EnumDevicesBySemantics( > if (lpCallback(&didevis[i], lpdid, callbackFlags, --remain, pvRef) == DIENUM_STOP) > { > HeapFree(GetProcessHeap(), 0, didevis); > + HeapFree(GetProcessHeap(), 0, username_w); > return DI_OK; > } > } > > HeapFree(GetProcessHeap(), 0, didevis); > > - if (dwFlags & DIEDBSFL_FORCEFEEDBACK) return DI_OK; > + if (dwFlags & DIEDBSFL_FORCEFEEDBACK) > + { > + HeapFree(GetProcessHeap(), 0, username_w); > + return DI_OK; > + } > > /* Enumerate keyboard and mouse */ > for(i=0; i < sizeof(guids)/sizeof(guids[0]); i++) > { > - callbackFlags = diactionformat_priorityA(lpdiActionFormat, actionMasks[i]); > + if (should_enumerate_device(username_w, dwFlags, &This->device_players, guids[i])) > + { > + callbackFlags = diactionformat_priorityA(lpdiActionFormat, actionMasks[i]); > > - IDirectInput_CreateDevice(iface, guids[i], &lpdid, NULL); > - IDirectInputDevice_GetDeviceInfo(lpdid, &didevi); > + IDirectInput_CreateDevice(iface, guids[i], &lpdid, NULL); > + IDirectInputDevice_GetDeviceInfo(lpdid, &didevi); > > - if (lpCallback(&didevi, lpdid, callbackFlags, sizeof(guids)/sizeof(guids[0]) - (i+1), pvRef) == DIENUM_STOP) > - return DI_OK; > + if (lpCallback(&didevi, lpdid, callbackFlags, --remain, pvRef) == DIENUM_STOP) > + { > + HeapFree(GetProcessHeap(), 0, username_w); > + return DI_OK; > + } > + } > } > > + HeapFree(GetProcessHeap(), 0, username_w); > return DI_OK; > } > > @@ -1049,7 +1119,8 @@ static HRESULT WINAPI IDirectInput8WImpl_EnumDevicesBySemantics( > > /* Default behavior is to enumerate attached game controllers */ > enumSuccess = dinput_devices[i]->enum_deviceW(DI8DEVCLASS_GAMECTRL, DIEDFL_ATTACHEDONLY | dwFlags, &didevi, This->dwVersion, j); > - if (enumSuccess == S_OK) > + if (enumSuccess == S_OK && > + should_enumerate_device(ptszUserName, dwFlags, &This->device_players, &didevi.guidInstance)) > { > if (device_count++) > didevis = HeapReAlloc(GetProcessHeap(), 0, didevis, sizeof(DIDEVICEINSTANCEW)*device_count); > @@ -1061,8 +1132,15 @@ static HRESULT WINAPI IDirectInput8WImpl_EnumDevicesBySemantics( > } > > remain = device_count; > + /* Add keyboard and mouse to remaining device count */ > if (!(dwFlags & DIEDBSFL_FORCEFEEDBACK)) > - remain += sizeof(guids)/sizeof(guids[0]); > + { > + for (i = 0; i < sizeof(guids) / sizeof(guids[0]); i++) > + { > + if (should_enumerate_device(ptszUserName, dwFlags, &This->device_players, guids[i])) > + remain++; > + } > + } > > for (i = 0; i < device_count; i++) > { > @@ -1083,13 +1161,16 @@ static HRESULT WINAPI IDirectInput8WImpl_EnumDevicesBySemantics( > /* Enumerate keyboard and mouse */ > for(i=0; i < sizeof(guids)/sizeof(guids[0]); i++) > { > - callbackFlags = diactionformat_priorityW(lpdiActionFormat, actionMasks[i]); > + if (should_enumerate_device(ptszUserName, dwFlags, &This->device_players, guids[i])) > + { > + callbackFlags = diactionformat_priorityW(lpdiActionFormat, actionMasks[i]); > > - IDirectInput_CreateDevice(iface, guids[i], &lpdid, NULL); > - IDirectInputDevice_GetDeviceInfo(lpdid, &didevi); > + IDirectInput_CreateDevice(iface, guids[i], &lpdid, NULL); > + IDirectInputDevice_GetDeviceInfo(lpdid, &didevi); > > - if (lpCallback(&didevi, lpdid, callbackFlags, sizeof(guids)/sizeof(guids[0]) - (i+1), pvRef) == DIENUM_STOP) > - return DI_OK; > + if (lpCallback(&didevi, lpdid, callbackFlags, --remain, pvRef) == DIENUM_STOP) > + return DI_OK; > + } > } > > return DI_OK; > -- > 2.11.0 > > >

1 0

Re: [PATCH v3 1/5] winebus.sys: Handle linux input event device via udev
by Sebastian Lackner March 1, 2017

March 1, 2017

I haven't done a full review yet, but here are some first comments. On 01.03.2017 17:16, Aric Stewart wrote: > + subsystem = udev_device_get_subsystem(dev); > usbdev = udev_device_get_parent_with_subsystem_devtype(dev, "usb", "usb_device"); > if (usbdev) > { > + #ifdef HAS_PROPER_INPUT_HEADER > + const platform_vtbl *other_vtbl = NULL; > + DEVICE_OBJECT *dup = NULL; > + if (strcmp(subsystem, "hidraw") == 0) > + other_vtbl = &lnxev_vtbl; > + else if (strcmp(subsystem, "input") == 0) > + other_vtbl = &hidraw_vtbl; > + > + if (other_vtbl) > + dup = bus_enumerate_hid_devices(other_vtbl, check_same_device, dev); Shouldn't we have some preference of either hidraw or linux event devices? Otherwise this could lead to different results in each run. > } > > +DEVICE_OBJECT* bus_enumerate_hid_devices(const platform_vtbl *vtbl, enum_func function, void* context) > +{ > + struct pnp_device *dev; > + DEVICE_OBJECT *ret = NULL; > + > + TRACE("(%p)\n", vtbl); > + > + EnterCriticalSection(&device_list_cs); > + LIST_FOR_EACH_ENTRY(dev, &pnp_devset, struct pnp_device, entry) > + { > + struct device_extension *ext = (struct device_extension *)dev->device->DeviceExtension; > + if (ext->vtbl != vtbl) continue; > + if (function(dev->device, context) == 0) > + ret = dev->device; > + break; This is wrong and will always abort after the first function call. > + } > + LeaveCriticalSection(&device_list_cs); > + return ret; > +} > +

2 1

Re: [PATCH 3/6] wined3d: Handle raw thread group shared memory declarations.
by Sebastian Lackner March 1, 2017

March 1, 2017

On 28.02.2017 14:42, Józef Kucia wrote: > + reg_maps->tgsm_count = reg_idx + 1; Wouldn't it be better to set it to max(reg_maps->tgsm_count, ...) to allow decreasing reg indices (even if not seen in practice yet)? Alternatively you could also get rid of this variable and update patch 6 to check the index against the capacity. Best regards, Sebastian

2 1

Re: [PATCH 2/2] winmm: Dispatch MCI_BREAK to the driver
by Sebastian Lackner March 1, 2017

March 1, 2017

On 01.03.2017 08:10, Bruno Jesus wrote: > + TRACE("(%08x, %08X, vkey %04X\n", wDevID, dwFlags, lpParms->nVirtKey); Did you forget to add a ")" ?

2 1

Re: [PATCH 1/2] server: Added server side named pipe read and write implementation and use it for message mode pipes.
by Sebastian Lackner March 1, 2017

March 1, 2017

On 28.02.2017 14:14, Jacek Caban wrote: > + LIST_FOR_EACH_ENTRY( message, &pipe_end->message_queue, struct pipe_message, entry ) > + avail += message->iosb->in_size - message->read_pos; > + reply_size = min( reply_size, avail ); > + > + if (avail) > + { > + message = LIST_ENTRY( list_head(&pipe_end->message_queue), struct pipe_message, entry ); > + reply_size = min( reply_size, message->iosb->in_size ); > + } Isn't there a problem with this size check? Lets assume we have two messages with 10 byte, for the first one the read_pos = 5. Then avail == 15, and reply_size is clamped to 10 - but it will attempt to copy 10 bytes starting position 5? > + > + if (!(buffer = set_reply_data_size(FIELD_OFFSET( FILE_PIPE_PEEK_BUFFER, Data[reply_size] )))) return; > + buffer->NamedPipeState = 0; /* FIXME */ > + buffer->ReadDataAvailable = avail; > + buffer->NumberOfMessages = 0; /* FIXME */ > + buffer->MessageLength = 0; /* FIXME */ > + if (avail) memcpy( buffer->Data, (const char *)message->iosb->in_data + message->read_pos, reply_size ); > +} > +

2 1

Re: [PATCH v3] oledb32: Correct length calculation in DataConvert for DBTYPE_STR
by Huw Davies March 1, 2017

March 1, 2017

On Tue, Feb 28, 2017 at 11:17:29PM +0000, Alistair Leslie-Hughes wrote: > --- a/dlls/oledb32/convert.c > +++ b/dlls/oledb32/convert.c > @@ -816,23 +816,23 @@ static HRESULT WINAPI convert_DataConvert(IDataConvert* iface, > case DBTYPE_STR: > { > BSTR b; > - DBLENGTH bstr_len; > + DBLENGTH length; > INT bytes_to_copy; > - hr = IDataConvert_DataConvert(iface, src_type, DBTYPE_BSTR, src_len, &bstr_len, > + hr = IDataConvert_DataConvert(iface, src_type, DBTYPE_BSTR, src_len, &length, > src, &b, sizeof(BSTR), src_status, dst_status, > precision, scale, flags); > if(hr != S_OK) return hr; > - bstr_len = SysStringLen(b); > - *dst_len = bstr_len * sizeof(char); /* Doesn't include size for '\0' */ > + *dst_len = SysStringLen(b); /* Doesn't include size for '\0' */ You've gone back to confusing the length of a BSTR with the length of the char * str. We seem to be going round in circles here, so I've just sent in the correct fix. Please take a look at it to figure out where you were going wrong. Huw.

1 0

Re: [PATCH] d3dx9_36/tests: Fix a memory leak in test_createtext.
by Matteo Bruni Feb. 28, 2017

Feb. 28, 2017

2017-02-27 7:52 GMT+01:00 Alex Henrie <alexhenrie24(a)gmail.com>: > diff --git a/dlls/d3dx9_36/tests/mesh.c b/dlls/d3dx9_36/tests/mesh.c > index 95dd06e466..64b7b8823c 100644 > --- a/dlls/d3dx9_36/tests/mesh.c > +++ b/dlls/d3dx9_36/tests/mesh.c > @@ -4340,8 +4340,7 @@ static void test_createtext(IDirect3DDevice9 *device, HDC hdc, const char *text, > if (!compute_text_mesh(&mesh, text, deviation, extrusion, otm.otmEMSquare, glyphs)) > { > skip("Couldn't create mesh\n"); > - d3dxmesh->lpVtbl->Release(d3dxmesh); > - return; > + goto error; > } > mesh.fvf = D3DFVF_XYZ | D3DFVF_NORMAL; I would go in the other direction and strip all the error checking branches and gotos from those tests. compute_text_mesh() returns FALSE only on allocation failure and that's not supposed to happen in the controlled conditions of the test. It's probably better to just fail and crash in those cases.

2 1