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

Outreachy applicant
by Prerna Garg March 14, 2017

March 14, 2017

Hi, I would like to participate in the current round of Outreachy. I am new to the world of open source. I am interested to contribute in the project Winetest Graphical User Interface. Can somebody please guide me as to how I should go about it. Regards, Prerna Garg B. Tech. Computer Science NIT Hamirpur India

2 1

Re: appwiz.cpl: Use sha256 checksums to validate Mono/Gecko downloads.
by Alexandre Julliard March 14, 2017

March 14, 2017

Sebastian Lackner <sebastian(a)fds-team.de> writes: > This patch also replaces the memory mapping logic with ReadFile. That's noticeably slower, any particular reason for doing this? -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [2/2] include: Introduce a with_context macro and use it to simplify vcomp tests. [RFC]
by Bruno Jesus March 14, 2017

March 14, 2017

On Tue, Mar 7, 2017 at 2:29 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > Based on ideas by Mark Jansen. > ... > > The with_context macro allows to specify a format string which is printed in > case of test failures and for trace messages. Currently, to track down the > origin of a test failure, it is necessary to write down all relevant variables > in each ok message. With the new macro it is much easier: > ... Hi, Sebastian. I like this idea, it would be specially useful for table struct tests (eg ws2_32) and functions that are called more than once for different tests (eg rsaenh). Each maintainer/helper could slowly update the tests as they think it fits and new tests could start benefiting from it. I hope more people join this conversation as it clearly took a long time to produce the patch and letting it drop from the list without talk would not be nice ;-)

3 3

Re: [PATCH 8/8] d3d9/tests: Add test for vertex textures.
by Marvin March 14, 2017

March 14, 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=29507 Your paranoid android. === w8 (32 bit visual) === visual.c:8099: Test failed: Got unexpected color 0x00ff00ff for no color attribute test. === w864 (64 bit visual) === visual.c:8099: Test failed: Got unexpected color 0x00ff00ff for no color attribute test.

1 0

Re: [PATCH 3/3] msvcrt100/tests: Add tests for concurrency::event
by Piotr Caban March 14, 2017

March 14, 2017

Hi, On 03/13/17 21:53, Daniel Lehman wrote: > +static void test_event(void) > +{ > + int i; > + int ret; > + int left; > + int wait; > + event evt; > + event *evts[70]; > + HANDLE thread; > + HANDLE threads[NUMELMS(evts)]; > + > + call_func1(p_event_ctor, &evt); > + > + ret = call_func2(p_event_wait, &evt, 100); > + ok(ret == -1, "expected -1, got %d\n", ret); > + > + call_func1(p_event_set, &evt); > + ret = call_func2(p_event_wait, &evt, 100); > + ok(!ret, "expected 0, got %d\n", ret); Could you please add following test here (to show that event is not reset after wait): ret = call_func2(p_event_wait, &evt, 100); ok(!ret, "expected 0, got %d\n", ret); > + i = 0; > + left = NUMELMS(evts); > + while (left) { > + wait = min(left, MAXIMUM_WAIT_OBJECTS); > + WaitForMultipleObjects(wait, &threads[i], TRUE, 5000); > + left -= wait; > + i += wait; > + } > + > + for (i = 0; i < NUMELMS(evts); i++) > + CloseHandle(threads[i]); I think it's cleaner if you change the while and for loop to something like: for (i = 0; i < NUMELMS(threads); i++) { WaitForSingleObject(threads[i], INFINITE); CloseHandle(threads[i]); } > + /* reset and test wait for multiple with any */ > + for (i = 0; i < NUMELMS(evts); i++) > + call_func1(p_event_reset, evts[i]); > + > + for (i = 0; i < NUMELMS(evts); i++) > + threads[i] = CreateThread(NULL, 0, test_event_thread, (void*)evts[i], 0, NULL); > + > + ret = p_event_wait_for_multiple(evts, NUMELMS(evts), FALSE, 5000); > + ok(ret != -1, "didn't expect -1\n"); Could you please start only one thread in this case and check exact return value from p_event_wait_for_multiple. Could you please also add following test: static DWORD WINAPI multiple_events_thread(void *arg) { event **events = arg; Sleep(50); call_func1(p_event_set, events[0]); call_func1(p_event_reset, events[0]); call_func1(p_event_set, events[1]); call_func1(p_event_reset, events[1]); return 0; } call_func1(p_event_reset, evts[0]); call_func1(p_event_reset, evts[1]); thread = CreateThread(NULL, 0, multiple_events_thread, (void*)evts, 0, NULL); ret = p_event_wait_for_multiple(evts, 2, TRUE, 500); ok(ret == -1, "expect -1, got %d\n", ret); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); Thanks, Piotr

1 0

1.8.7 maintenance release?
by version2013＠openmailbox.org March 14, 2017

March 14, 2017

I still have much to learn. Why have a 1.8.7 maintenance release after a 2.0 stable release? In advance, thank you for sharing your knowledge as to why.

4 4

Re: [PATCH 1/3] msvcrt: Add Concurrency::event stubs
by Piotr Caban March 14, 2017

March 14, 2017

Hi Daniel, On 03/13/17 21:53, Daniel Lehman wrote: > include/concrt.h | 25 ++++++++++++++++ The c-runtimes related headers are defined in include/msvcrt/*. In this case I think that it will be better to define needed constants in dlls/msvcrt/msvcrt.h because the concrt.h header is not useful. Thanks, Piotr

1 0

Re: [PATCH] user32: Removed 16-bit windows position and size constraints
by Alexandre Julliard March 14, 2017

March 14, 2017

Kees Beets <kbeets68(a)gmail.com> writes: > Fixes https://bugs.winehq.org/show_bug.cgi?id=38231 > > Removed 16-bit windows position and size constraints > introduced in 2006. Only kept truncation to 16-bit > in windows messaging to keep same behaviour as windows. I'd find it surprising that the messages wouldn't match the actual window sizes. This would need more test cases. In fact we already had some window size tests, but they were commented out. I've re-enabled them now, so if you try your patch against latest git you'll see new failures, indicating that fixing up the messages is not correct. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 2/3] msvcrt: Implement Concurrency::event
by Sebastian Lackner March 14, 2017

March 14, 2017

On 13.03.2017 21:53, Daniel Lehman wrote: > > From 44d603b75fe10d51377c20e4c3c2a1bf031c4fcf Mon Sep 17 00:00:00 2001 > From: Daniel Lehman <dlehman(a)esri.com> > Date: Mon, 12 Dec 2016 16:44:44 -0800 > Subject: [PATCH 2/3] msvcrt: Implement Concurrency::event > > wait_for_multiple can handle more than WaitForMultipleObjects' limit of 64 > a separate thread is used to handle threads that wait on multiple handles > and is loosely modeled after wineserver's select request handler > a very simple command structure is used to communicate with it over a single shared pipe > > Signed-off-by: Daniel Lehman <dlehman(a)esri.com> > --- > dlls/msvcrt/lock.c | 387 +++++++++++++++++++++++++++++++++++++++++++++++++-- > dlls/msvcrt/main.c | 3 + > dlls/msvcrt/msvcrt.h | 18 +++ > 3 files changed, 397 insertions(+), 11 deletions(-) Not sure if its suitable here, but please note that Wines threadpool implementation also supports waiting on more than 64 objects (using similar tricks). If possible, it would be better to avoid the second implementation.

2 3

Re: [PATCH 3/3] d3dx9: Apply changed states only in CommitChanges.
by Matteo Bruni March 14, 2017

March 14, 2017

2017-03-09 23:15 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/d3dx9_private.h | 8 ++- > dlls/d3dx9_36/effect.c | 146 ++++++++++++++++++++++++++++-------------- > dlls/d3dx9_36/preshader.c | 40 ++++++++++-- > dlls/d3dx9_36/tests/effect.c | 13 ---- > 4 files changed, 137 insertions(+), 70 deletions(-) > > diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h > index 27b91a7..53cf99f 100644 > --- a/dlls/d3dx9_36/d3dx9_private.h > +++ b/dlls/d3dx9_36/d3dx9_private.h > @@ -222,6 +222,11 @@ struct d3dx_parameter > DWORD *dirty_flag_ptr; > }; > > +static inline BOOL is_param_dirty(struct d3dx_parameter *param) > +{ > + return (*param->dirty_flag_ptr & PARAMETER_FLAG_DIRTY); > +} > + > struct d3dx9_base_effect; > > struct d3dx_parameter *get_parameter_by_name(struct d3dx9_base_effect *base, > @@ -233,6 +238,7 @@ void d3dx_free_param_eval(struct d3dx_param_eval *peval) DECLSPEC_HIDDEN; > HRESULT d3dx_evaluate_parameter(struct d3dx_param_eval *peval, > const struct d3dx_parameter *param, void *param_value) DECLSPEC_HIDDEN; > HRESULT d3dx_param_eval_set_shader_constants(struct IDirect3DDevice9 *device, > - struct d3dx_param_eval *peval) DECLSPEC_HIDDEN; > + struct d3dx_param_eval *peval, BOOL update_all) DECLSPEC_HIDDEN; > +BOOL is_param_eval_input_dirty(struct d3dx_param_eval *peval) DECLSPEC_HIDDEN; > > #endif /* __WINE_D3DX9_PRIVATE_H */ > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c > index ada78f9..b18465c 100644 > --- a/dlls/d3dx9_36/effect.c > +++ b/dlls/d3dx9_36/effect.c > @@ -123,6 +123,9 @@ struct d3dx_pass > > struct d3dx_state *states; > struct d3dx_parameter *annotations; > + > + D3DLIGHT9 current_light[8]; > + D3DMATERIAL9 current_material; > }; I think these might be stored in the effect struct instead. > +static void clear_dirty_all(struct d3dx9_base_effect *base) clear_dirty_params() or something like that would be a clearer name. > @@ -1570,8 +1587,12 @@ static HRESULT d3dx9_base_effect_set_int(struct d3dx9_base_effect *base, D3DXHAN > { > if (param->rows == 1 && param->columns == 1) > { > - set_number(param->data, param->type, &n, D3DXPT_INT); > - set_dirty(param); > + DWORD value; > + > + set_number(&value, param->type, &n, D3DXPT_INT); > + if (value != *(DWORD *)param->data) > + set_dirty(param); > + *(DWORD *)param->data = value; Shouldn't this go in the previous patch?

2 1

Re: [PATCH 2/3] d3dx9: Set dirty flag on parameter change.
by Matteo Bruni March 14, 2017

March 14, 2017

2017-03-09 23:15 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > @@ -2021,9 +2055,11 @@ static HRESULT d3dx9_base_effect_set_matrix_array(struct d3dx9_base_effect *base > switch (param->class) > { > case D3DXPC_MATRIX_ROWS: > + set_dirty(param); > for (i = 0; i < count; ++i) > { > set_matrix(&param->members[i], &matrix[i]); > + set_dirty(&param->members[i]); > } > return D3D_OK; > Here and below it should be possible to avoid marking dirty each array element individually. IIRC I had some test showing that arrays are always considered completely dirty or clean on native i.e. CommitChanges updates the whole array even if you only change a subset of the elements.

2 1

Re: [PATCH 1/3] d3dx9/tests: Add test for CommitChanges in effect.
by Matteo Bruni March 14, 2017

March 14, 2017

2017-03-09 23:15 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/tests/effect.c | 1267 +++++++++++++++++++++++++++++++----------- > 1 file changed, 930 insertions(+), 337 deletions(-) First of all, please split up this patch, this is way too large and does multiple things at once (I see that it extends the test effect and checks the new states, factors out two helper functions from test_effect_preshader() and it also adds the CommitChanges test in the subject). Those are all good changes but when they are squeezed in one patch it becomes hard to review. I haven't looked in too much detail but anyway there are a few additional comments below. > +#define TEST_EFFECT_PRES_NOPTESTS ARRAY_SIZE(test_effect_preshader_op_results) I know this isn't new but I don't think this define is very useful. If you really want to keep it, please name it xxx_TEST_COUNT. > + else if (!state_updated[i]) > + { > + todo_wine_if(memcmp(v, empty_v, sizeof(float) * 4)) > + ok_(__FILE__, line)(!memcmp(v, empty_v, sizeof(float) * 4), > + "Parameter %s, test %d, operation %s, state updated unexpectedly.\n", > + updated_param, i, test_effect_preshader_op_results[i].comment); Eh, this isn't really all that nice. I don't see an obvious way to make this any better though so it probably makes sense to move this part (or even all the tests) to the end of the patch series and avoid the problem altogether. BTW, if you define empty_v as an array you can use sizeof(empty_v) here. > + } > + } > +} > + > +static const D3DXVECTOR4 fvect_empty = {-9999.0f, -9999.0f, -9999.0f, -9999.0f}; > + > +static void test_effect_preshader_clear_vconsts(IDirect3DDevice9 *device) > +{ > + int i; unsigned int i; > + static const struct > + { > + const char *param_name; > + BOOL const_updated[TEST_EFFECT_PRES_NFLOATV]; > + } > + check_vconsts_parameters[] = > + { > + {"g_Selector", > + {FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, > + FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, > + FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, > + FALSE, FALSE, FALSE, TRUE}}, It would be nicer to represent these as 32-bit hexadecimal bitmasks. > + for (i = 0; i < sizeof(check_op_parameters) / sizeof(*check_op_parameters); ++i) > + { > + int j; unsigned int j; Also please leave a blank line between declarations and code. > + for (j = 0; j < 8; j++) Consistency, please stick to the '++j' form.

2 1

Re: [PATCH 3/7] d3d9/tests: Fix test_updatetexture() on GPUs not supporting WINED3DFMT_R32_FLOAT.
by Marvin March 13, 2017

March 13, 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=29498 Your paranoid android. === w1064 (32 bit visual) === visual.c:7995: Test failed: Got unexpected color 0x00008080 for quad 2 (different colors). visual.c:7995: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors).

1 0

Re: [PATCH 1/7] d3d9/tests: Ignore test_vshader_input() "no color attribute" test result on WARP.
by Marvin March 13, 2017

March 13, 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=29497 Your paranoid android. === w1064 (32 bit visual) === visual.c:7995: Test failed: Got unexpected color 0x00008080 for quad 2 (different colors). === w1064 (64 bit visual) === visual.c:7995: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors).

1 0

Re: [PATCH] ddraw/tests: Rewrite SrcColorKey32BlitTest (v2).
by Henri Verbeet March 13, 2017

March 13, 2017

On 8 March 2017 at 20:29, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > *) Accept the Nvidia XRGB color key results with X != 0. I am open to > removing this test entirely as we're accepting a lot of different > outcomes now. Yeah, probably. It does seem consistent with the idea that the "X" channel is undefined, and I don't think that particular test was motivated by application behaviour.

2 1

[PATCH] user32: Fix handling of internal messages.
by Zebediah Figura March 13, 2017

March 13, 2017

I'm submitting this to wine-devel because the code is probably messy and I'm not sure if this is the right approach to solving the problem. This patch adds a new flag and a server reply, allowing GetQueueStatus to ignore internal messages, and MsgWaitForMultipleObjects to process internal messages until a real sent message is found. Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/user32/message.c | 129 +++++++++++++++++++++++++++++++++++++++-- dlls/user32/tests/msg.c | 8 +-- include/wine/server_protocol.h | 4 +- include/winuser.h | 3 + server/protocol.def | 1 + server/queue.c | 14 ++++- server/request.h | 3 +- server/trace.c | 1 + 8 files changed, 150 insertions(+), 13 deletions(-) diff --git a/dlls/user32/message.c b/dlls/user32/message.c index 9bfb453..92caf64 100644 --- a/dlls/user32/message.c +++ b/dlls/user32/message.c @@ -2964,6 +2964,98 @@ static BOOL peek_message( MSG *msg, HWND hwnd, UINT first, UINT last, UINT flags /*********************************************************************** + * receive_internal_message + * + * Retrieves and processes a single sent internal message. + */ +static DWORD receive_internal_message(size_t buffer_size) +{ + LRESULT result; + struct user_thread_info *thread_info = get_user_thread_info(); + struct received_message_info info, *old_info; + void *buffer; + size_t size = 0; + + buffer = HeapAlloc(GetProcessHeap(), 0, buffer_size); + + SERVER_START_REQ( get_message ) + { + NTSTATUS res; + + req->flags = PM_REMOVE | PM_QS_SENDMESSAGE; + req->get_win = 0; + req->get_first = 0; + req->get_last = 0; + req->hw_id = 0; + req->wake_mask = 0; + req->changed_mask = 0; + wine_server_set_reply( req, buffer, buffer_size ); + + if ((res = wine_server_call( req )) != STATUS_SUCCESS) + { + HeapFree(GetProcessHeap(), 0, buffer); + if (res == STATUS_BUFFER_OVERFLOW) + return receive_internal_message(reply->total); + + return RtlNtStatusToDosError(res); + } + + size = wine_server_reply_size( reply ); + info.type = reply->type; + info.msg.hwnd = wine_server_ptr_handle( reply->win ); + info.msg.message = reply->msg; + info.msg.wParam = reply->wparam; + info.msg.lParam = reply->lparam; + info.msg.time = reply->time; + info.msg.pt.x = reply->x; + info.msg.pt.y = reply->y; + thread_info->active_hooks = reply->active_hooks; + } + SERVER_END_REQ; + + switch(info.type) + { + case MSG_ASCII: + case MSG_UNICODE: + info.flags = ISMEX_SEND; + break; + case MSG_NOTIFY: + info.flags = ISMEX_NOTIFY; + break; + case MSG_CALLBACK: + info.flags = ISMEX_CALLBACK; + break; + case MSG_HOOK_LL: + info.flags = ISMEX_SEND; + reply_message(&info, 0, TRUE); + return STATUS_SUCCESS; + case MSG_OTHER_PROCESS: + info.flags = ISMEX_SEND; + if (!unpack_message(info.msg.hwnd, info.msg.message, &info.msg.wParam, + &info.msg.lParam, &buffer, size)) + { + /* ignore it */ + reply_message(&info, 0, TRUE); + return ERROR_SUCCESS; + } + break; + default: + ERR("unexpected type %d for internal message %08x\n", info.type, info.msg.message); + } + + old_info = thread_info->receive_info; + thread_info->receive_info = &info; + result = handle_internal_message(info.msg.hwnd, info.msg.message, + info.msg.wParam, info.msg.lParam); + reply_message( &info, result, TRUE ); + thread_info->receive_info = old_info; + + HeapFree(GetProcessHeap(), 0, buffer); + return ERROR_SUCCESS; +} + + +/*********************************************************************** * process_sent_messages * * Process all pending sent messages. @@ -3811,7 +3903,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH PeekMessageA( MSG *msg, HWND hwnd, UINT first, UIN BOOL WINAPI DECLSPEC_HOTPATCH GetMessageW( MSG *msg, HWND hwnd, UINT first, UINT last ) { HANDLE server_queue = get_server_queue_handle(); - unsigned int mask = QS_POSTMESSAGE | QS_SENDMESSAGE; /* Always selected */ + unsigned int mask = QS_POSTMESSAGE | QS_SENDMESSAGE | QS_WINE_SENDMESSAGE; /* Always selected */ USER_CheckNotLock(); check_for_driver_events( 0 ); @@ -3825,7 +3917,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetMessageW( MSG *msg, HWND hwnd, UINT first, UINT if ((first <= WM_SYSTIMER) && (last >= WM_SYSTIMER)) mask |= QS_TIMER; if ((first <= WM_PAINT) && (last >= WM_PAINT)) mask |= QS_PAINT; } - else mask = QS_ALLINPUT; + else mask |= QS_ALLINPUT; while (!peek_message( msg, hwnd, first, last, PM_REMOVE | (mask << 16), mask )) { @@ -4142,7 +4234,7 @@ DWORD WINAPI MsgWaitForMultipleObjectsEx( DWORD count, const HANDLE *pHandles, DWORD timeout, DWORD mask, DWORD flags ) { HANDLE handles[MAXIMUM_WAIT_OBJECTS]; - DWORD i; + DWORD i, res; if (count > MAXIMUM_WAIT_OBJECTS-1) { @@ -4154,8 +4246,37 @@ DWORD WINAPI MsgWaitForMultipleObjectsEx( DWORD count, const HANDLE *pHandles, for (i = 0; i < count; i++) handles[i] = pHandles[i]; handles[count] = get_server_queue_handle(); - return wait_objects( count+1, handles, timeout, + mask |= QS_WINE_SENDMESSAGE; + + for (;;) + { + res = wait_objects( count+1, handles, timeout, (flags & MWMO_INPUTAVAILABLE) ? mask : 0, mask, flags ); + if (res == WAIT_OBJECT_0+count) + { + NTSTATUS server_res; + UINT msg; + SERVER_START_REQ( get_queue_status ) + { + req->clear_bits = 0; + if ((server_res = wine_server_call( req )) != STATUS_SUCCESS) + { + SetLastError(RtlNtStatusToDosError(server_res)); + return WAIT_FAILED; + } + msg = reply->first_sent_msg; + } + SERVER_END_REQ; + + if (msg & 0x80000000) + { + /* If it's a WINE internal message, process it and keep waiting */ + receive_internal_message(256); + continue; + } + } + return res; + } } diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 19a1f31..a682b70 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -16332,7 +16332,7 @@ static void test_SendMessage_other_thread(int thread_n) GetMessageA(&msg, 0, 0, 0); ok(msg.message == WM_USER, "expected WM_USER, got %04x\n", msg.message); DispatchMessageA(&msg); - ok_sequence(send_message_1, "SendMessage from other thread 1", thread_n == 2); + ok_sequence(send_message_1, "SendMessage from other thread 1", FALSE); /* intentionally yield */ MsgWaitForMultipleObjects(0, NULL, FALSE, 100, qs_all_input); @@ -16347,7 +16347,7 @@ static void test_SendMessage_other_thread(int thread_n) trace("main: call PeekMessage\n"); ok(PeekMessageA(&msg, 0, 0, 0, PM_NOREMOVE), "PeekMessage should not fail\n"); ok(msg.message == WM_USER+1, "expected WM_USER+1, got %04x\n", msg.message); - ok_sequence(send_message_3, "SendMessage from other thread 3", thread_n == 2); + ok_sequence(send_message_3, "SendMessage from other thread 3", FALSE); trace("main: call PeekMessage\n"); ok(PeekMessageA(&msg, 0, 0, 0, PM_REMOVE), "PeekMessage should not fail\n"); @@ -16359,13 +16359,11 @@ static void test_SendMessage_other_thread(int thread_n) MsgWaitForMultipleObjects(0, NULL, FALSE, 100, qs_all_input); ret = GetQueueStatus(QS_SENDMESSAGE|QS_POSTMESSAGE); - /* FIXME: remove once Wine is fixed */ -todo_wine_if (thread_n == 2) ok(ret == 0, "wrong status %08x\n", ret); trace("main: call PeekMessage\n"); ok(!PeekMessageA(&msg, 0, 0, 0, PM_REMOVE), "PeekMessage should fail\n"); - ok_sequence(WmEmptySeq, "SendMessage from other thread 5", thread_n == 2); + ok_sequence(WmEmptySeq, "SendMessage from other thread 5", FALSE); ret = GetQueueStatus(QS_SENDMESSAGE|QS_POSTMESSAGE); ok(ret == 0, "wrong status %08x\n", ret); diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index ee9fdd1..4417cb6 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -2941,6 +2941,8 @@ struct get_queue_status_reply struct reply_header __header; unsigned int wake_bits; unsigned int changed_bits; + unsigned int first_sent_msg; + char __pad_20[4]; }; @@ -6412,6 +6414,6 @@ union generic_reply struct terminate_job_reply terminate_job_reply; }; -#define SERVER_PROTOCOL_VERSION 524 +#define SERVER_PROTOCOL_VERSION 525 #endif /* __WINE_WINE_SERVER_PROTOCOL_H */ diff --git a/include/winuser.h b/include/winuser.h index 2b8331c..d10caba 100644 --- a/include/winuser.h +++ b/include/winuser.h @@ -2898,6 +2898,9 @@ typedef struct tagTRACKMOUSEEVENT { /* Extra (undocumented) queue wake bits - see "Undoc. Windows" */ #define QS_SMRESULT 0x8000 +/* Internal WINE message */ +#define QS_WINE_SENDMESSAGE 0x80000000 + /* InSendMessageEx flags */ #define ISMEX_NOSEND 0x00000000 #define ISMEX_SEND 0x00000001 diff --git a/server/protocol.def b/server/protocol.def index 60865a6..7b797af 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2162,6 +2162,7 @@ enum char_info_mode @REPLY unsigned int wake_bits; /* wake bits */ unsigned int changed_bits; /* changed bits since last time */ + unsigned int first_sent_msg; /* first message in the sent message queue */ @END diff --git a/server/queue.c b/server/queue.c index c479b38..ea6d5e6 100644 --- a/server/queue.c +++ b/server/queue.c @@ -744,7 +744,8 @@ static void receive_message( struct msg_queue *queue, struct message *msg, queue->recv_result = result; } free( msg ); - if (list_empty( &queue->msg_list[SEND_MESSAGE] )) clear_queue_bits( queue, QS_SENDMESSAGE ); + if (list_empty( &queue->msg_list[SEND_MESSAGE] )) + clear_queue_bits( queue, QS_SENDMESSAGE | QS_WINE_SENDMESSAGE ); } /* set the result of the current received message */ @@ -2272,11 +2273,17 @@ DECL_HANDLER(set_queue_mask) DECL_HANDLER(get_queue_status) { struct msg_queue *queue = current->queue; + struct list *ptr; if (queue) { reply->wake_bits = queue->wake_bits; reply->changed_bits = queue->changed_bits; queue->changed_bits &= ~req->clear_bits; + + if ((ptr = list_head( &queue->msg_list[SEND_MESSAGE] ))) + reply->first_sent_msg = LIST_ENTRY( ptr, struct message, entry )->msg; + else + reply->first_sent_msg = 0; } else reply->wake_bits = reply->changed_bits = 0; } @@ -2339,7 +2346,10 @@ DECL_HANDLER(send_message) /* fall through */ case MSG_NOTIFY: list_add_tail( &recv_queue->msg_list[SEND_MESSAGE], &msg->entry ); - set_queue_bits( recv_queue, QS_SENDMESSAGE ); + if (msg->msg & 0x80000000) /* Wine internal message */ + set_queue_bits( recv_queue, QS_WINE_SENDMESSAGE ); + else + set_queue_bits( recv_queue, QS_SENDMESSAGE ); break; case MSG_POSTED: list_add_tail( &recv_queue->msg_list[POST_MESSAGE], &msg->entry ); diff --git a/server/request.h b/server/request.h index cd396ee..374150d 100644 --- a/server/request.h +++ b/server/request.h @@ -1480,7 +1480,8 @@ C_ASSERT( FIELD_OFFSET(struct get_queue_status_request, clear_bits) == 12 ); C_ASSERT( sizeof(struct get_queue_status_request) == 16 ); C_ASSERT( FIELD_OFFSET(struct get_queue_status_reply, wake_bits) == 8 ); C_ASSERT( FIELD_OFFSET(struct get_queue_status_reply, changed_bits) == 12 ); -C_ASSERT( sizeof(struct get_queue_status_reply) == 16 ); +C_ASSERT( FIELD_OFFSET(struct get_queue_status_reply, first_sent_msg) == 16 ); +C_ASSERT( sizeof(struct get_queue_status_reply) == 24 ); C_ASSERT( FIELD_OFFSET(struct get_process_idle_event_request, handle) == 12 ); C_ASSERT( sizeof(struct get_process_idle_event_request) == 16 ); C_ASSERT( FIELD_OFFSET(struct get_process_idle_event_reply, event) == 8 ); diff --git a/server/trace.c b/server/trace.c index e0ce45a..5ba65da 100644 --- a/server/trace.c +++ b/server/trace.c @@ -2698,6 +2698,7 @@ static void dump_get_queue_status_reply( const struct get_queue_status_reply *re { fprintf( stderr, " wake_bits=%08x", req->wake_bits ); fprintf( stderr, ", changed_bits=%08x", req->changed_bits ); + fprintf( stderr, ", first_sent_msg=%08x", req->first_sent_msg ); } static void dump_get_process_idle_event_request( const struct get_process_idle_event_request *req ) -- 2.7.4

1 0

Re: [PATCH 1/5] wined3d: Get rid of "texture_stages" variable in draw_primitive_immediate_mode().
by Henri Verbeet March 13, 2017

March 13, 2017

On 13 March 2017 at 11:57, Józef Kucia <jkucia(a)codeweavers.com> wrote: > @@ -281,8 +281,7 @@ static void draw_primitive_immediate_mode(struct wined3d_context *context, const > GL_EXTCALL(glSecondaryColor3fEXT)(0.0f, 0.0f, 0.0f); > } > > - texture_stages = d3d_info->limits.ffp_blend_stages; > - for (texture_idx = 0; texture_idx < texture_stages; ++texture_idx) > + for (texture_idx = 0; texture_idx < d3d_info->limits.ffp_blend_stages; ++texture_idx) It doesn't matter that much, but I'm not sure that's an improvement.

1 0

Re: [PATCH] ole32/tests: Stop crash on XP
by Jacek Caban March 13, 2017

March 13, 2017

Hi Alistair, On 12.03.2017 22:11, Alistair Leslie-Hughes wrote: > Fixes crash > http://test.winehq.org/data/4b1445940d47870895b7fa87b7d69d12abe701ef/xp_fg-… > > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/ole32/tests/compobj.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c > index 8f6ff7107c..db68fd05c6 100644 > --- a/dlls/ole32/tests/compobj.c > +++ b/dlls/ole32/tests/compobj.c > @@ -3569,7 +3569,12 @@ static void test_GlobalOptions(void) > > hres = CoCreateInstance(&CLSID_GlobalOptions, NULL, CLSCTX_INPROC_SERVER, > &IID_IGlobalOptions, (void**)&global_options); > - ok(hres == S_OK, "CoCreateInstance(CLSID_GlobalOptions) failed: %08x\n", hres); Instead of removing ok(), please change it with '|| broken(hres == E_NOINTERFACE)'. > + if(FAILED(hres)) > + { > + win_skip("CLSID_GlobalOptions not available\n"); > + CoUninitialize(); > + return; > + } > > IGlobalOptions_Release(global_options); Jacek

1 0

Re: [v3 04/12] comctl32: Extend TaskDialog, add simple callback and tests
by Nikolay Sivov March 13, 2017

March 13, 2017

On 10.03.2017 21:21, Fabian Maurer wrote: > v3: Rewrite to implement Nikolay Sivov's suggestions > > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > dlls/comctl32/taskdialog.c | 29 +++++++++++++++++++++++-- > dlls/comctl32/tests/taskdialog.c | 46 +++++++++++++++++++++++++++++++++------- > 2 files changed, 65 insertions(+), 10 deletions(-) > > diff --git a/dlls/comctl32/taskdialog.c b/dlls/comctl32/taskdialog.c > index 2166ce1898..c5f410716e 100644 > --- a/dlls/comctl32/taskdialog.c > +++ b/dlls/comctl32/taskdialog.c > @@ -147,17 +147,41 @@ static void controls_add(struct list *controls, WORD id, const WCHAR *class, con > list_add_tail(controls, &data->entry); > } > > +/* FIXME: Make thread safe */ > +static const TASKDIALOGCONFIG *task_config = 0; > +static HRESULT callback(HWND hwnd, UINT uNotification, WPARAM wParam, LPARAM lParam) > +{ > + if(task_config->pfCallback) > + return task_config->pfCallback(hwnd, uNotification, wParam, lParam, task_config->lpCallbackData); > + return S_OK; > +} > + There's no reason to introduce global variable for that, just to remove it two patches after. > static INT_PTR CALLBACK DialogProc(HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lParam) > { > + HRESULT ret_callback; > + > switch (uMsg) > { > + case WM_INITDIALOG: > + callback(hwndDlg, TDN_DIALOG_CONSTRUCTED, 0, 0); > + callback(hwndDlg, TDN_CREATED, 0, 0); > + return TRUE; > case WM_COMMAND: > - if(HIWORD(wParam) == BN_CLICKED && LOWORD(wParam) == IDOK) > + if(HIWORD(wParam) == BN_CLICKED) > { > - EndDialog(hwndDlg, 0); > + WORD command_id = LOWORD(wParam); > + > + ret_callback = callback(hwndDlg, TDN_BUTTON_CLICKED, command_id, 0); > + if(ret_callback == S_OK) /* FIXME */ > + { > + EndDialog(hwndDlg, command_id); > + } > return TRUE; > } > break; > + case WM_DESTROY: > + callback(hwndDlg, TDN_DESTROYED, 0, 0); > + break; > } > return FALSE; For minimal working replacement of MessageBox callback support is not needed, right?

2 4

Re: [v2] user32: Removed 16-bit windows position and size constraints
by Alex Henrie March 12, 2017

March 12, 2017

2017-03-12 4:23 GMT-06:00 Kees Beets <kbeets68(a)gmail.com>: > Thanks. Changed and submitted again. > > > I'm new to wine. From the instructions here > https://wiki.winehq.org/Submitting_Patches it sounds like there is a > possibility to submit corrections to patches without creating new entries in > the patch list. Somehow I've not been able to get that working. Hope I don't > get on people's nerves :). Alexandre manually removes old patches from the list after a couple of days. If you suspect that there is something wrong with a patch, don't send it to wine-patches, but don't feel bad about sending a new patch to fix an honest mistake. -Alex

1 0

Re: wine/test.h: Identify the process responsible for test summary lines.
by Sebastian Lackner March 12, 2017

March 12, 2017

On 22.02.2017 15:33, Francois Gouget wrote: > Currently the child process summary lines are indistinguishable from the > main process one. This makes it impossible to automatically detect > when a process exits abruptly, i.e. without printing a summary line, > if it has run a subprocess. > This patch will let analysis tools identify which summary lines > correspond to processes they started and thus when some are missing. > Note also that in case of a crash the 'unhandled exception' play the > same role as the test summary lines. Thus it is important to also tag > them with the pid of the source process. > > Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> > --- > include/wine/test.h | 8 +++++--- > programs/winetest/main.c | 14 +++++++++----- > 2 files changed, 14 insertions(+), 8 deletions(-) Ugh... actually it looks like we have a summary line hardcoded in schedsvc tests: --- snip --- if (winetest_debug) { fprintf(stdout, "rpcapi: 0 tests executed (0 marked as todo, 0 failures), 1 skipped.\n"); fflush(stdout); } ExitProcess(0); --- snip --- Is it still recognized by the testbot?

2 1

Re: [v3 03/12] comctl32: Added basic implementation for task dialogs and add tests
by Nikolay Sivov March 12, 2017

March 12, 2017

On 10.03.2017 21:21, Fabian Maurer wrote: > + UINT text_size; /* Length in bytes including null-terminator */ > + UINT class_size; /* Length in bytes including null-terminator */ > + struct list entry; > +}control_info; > + > +typedef struct > +{ > + DLGTEMPLATE template; > + WORD menu; > + WORD class; > + const WCHAR *title; > + > + /* Not part of actual template */ > + UINT titleSize; /* Length in bytes including null-terminator */ > +}dialog_header; About formatting issues, for consistency please add space after bracket, same goes for 'if' and other places. "titleSize" -> "title_size". > +#define MEMCPY_MOVEPTR(target, source, size) memcpy(target, source, size); target += size; I think this one will be better as a function, also this macro assumes that target point is char*, which is impossible to guess. > +static void* align_pointer(void *ptr, unsigned int boundary) > +{ > + boundary--; > + return (void *)(((UINT_PTR)ptr + boundary) & ~boundary); > +} This is usually done with macros in Wine I think, like ((x) + size - 1) & ~(size - 1). > +LPDLGTEMPLATEW dialog_template_create(dialog_header header, struct list *controls) Why is 'header' passed by value? > + void *template_data_start; I think it's better to declare this with a type you're going to return. > + data_size += sizeof(WORD)*2 + header.titleSize; Same formatting issue - please add spaces around operator. > + /* Align on WORD boundary for the strings */ > + template_data = align_pointer(template_data, 2); I don't see how it could be not aligned to WORD, could you explain?

2 3

Interested to contribute to WINE via Outreachy
by Himadri Pandya March 12, 2017

March 12, 2017

Hello Everyone, I'm Himadri, an undergraduate student from India pursuing my bachelors in computer engineering. I'm a newbie to the FOSS community and keen to be a part by contributing to some interesting project. And I guess starting the journey with Outreachy would be the best ! Gsoc project : "Tools - Winetest Graphical User Interface" seems quite interesting to me. I would love to be a part of it. It would be really appreciable if I can get some replies about how can I contribute to it. Regards Himadri Pandya Linked In : https://www.linkedin.com/in/himadri2117

3 3

Re: [PATCH] oledb32: Ignore IRunnableObject in IDataSourceLocator_QueryInterface.
by Sebastian Lackner March 12, 2017

March 12, 2017

On 12.03.2017 05:34, Alistair Leslie-Hughes wrote: > + else if (IsEqualIID(riid, &IID_IDataInitialize)) > + { > + TRACE("IID_IRunnableObject returning NULL\n"); > + } You are checking for the wrong IID. Also, the TRACE does not really prevent that the FIXME message shows up.

1 0

Re: [v2] user32: Removed 16-bit windows position and size constraints
by Alex Henrie March 11, 2017

March 11, 2017

+ // truncate x,y to 16-bit in WM_WINDOWPOSCHANGING message (only) + // truncate x,y to 16-bit in WM_WINDOWPOSCHANGED message (only) C++-style comments are not permitted in Wine patches. See https://wiki.winehq.org/Developer_Hints#Using_only_C89-compliant_code -Alex

1 0

GSoC 2017 : CMD - implement more robust parser
by Kevin Jojo March 11, 2017

March 11, 2017

I am Prateek Bilung , a first year undergraduate student of Mathematics & Computing at BIT Mesra ,Ranchi , India. I am interested in contributing for project "CMD - implement more robust parser" . Please guide me . Thank you !

1 0

Re: [PATCH] user32: Removed 16-bit windows position and size constraints
by Alexandre Julliard March 11, 2017

March 11, 2017

Kees Beets <kbeets68(a)gmail.com> writes: > Fixes https://bugs.winehq.org/show_bug.cgi?id=38231 > > Removed code introduced in 2006 to correctly reflect Win9x 16-bit winpos > behaviour. Newer versions of Windows do not have the 16-bit constraint > and this code is now actually causing a bug in Adobe Lightroom on Wine. > I've not used iswin9x to ensure compatibility with Win9x as: > - Applications that can run on Win9x should take care of this > - Adding such code will over time make wine more complex > > Tested on Ubuntu 16.04, HP Elitebook with AMD A10. > 13 failures before change, 10 failures after change. Test code uses > iswin9x, so to have 0 failures means changes to the test suite or > further introduction of 'if isinw9x' logic. > > Signed-off-by: Kees Beets <kbeets68(a)gmail.com> > --- > dlls/user32/winpos.c | 16 ---------------- > 1 file changed, 16 deletions(-) It breaks the tests: ../../../tools/runtest -q -P wine -T ../../.. -M user32.dll -p user32_test.exe.so win && touch win.ok win.c:776: Test failed: bad winpos->x -32769 win.c:777: Test failed: bad winpos->y -40000 win.c:795: Test failed: bad winpos->x -32769 win.c:796: Test failed: bad winpos->y -40000 win.c:776: Test failed: bad winpos->x 32768 win.c:777: Test failed: bad winpos->y 40000 win.c:795: Test failed: bad winpos->x 32768 win.c:796: Test failed: bad winpos->y 40000 Makefile:649: recipe for target 'win.ok' failed -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [v3 01/12] comctl32: Move TaskDialogIndirect and tests into own source files
by Nikolay Sivov March 11, 2017

March 11, 2017

On 10.03.2017 21:21, Fabian Maurer wrote: > diff --git a/dlls/comctl32/taskdialog.c b/dlls/comctl32/taskdialog.c > new file mode 100644 > index 0000000000..c26d53bac9 > --- /dev/null > +++ b/dlls/comctl32/taskdialog.c > @@ -0,0 +1,72 @@ > +/* > + * Task dialog control > + * > + * Copyright 2017 Fabian Maurer for the Wine project > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA > + * > + */ > + > +#include <stdarg.h> > +#include <string.h> > + > +#include "windef.h" > +#include "winbase.h" > +#include "winuser.h" > +#include "commctrl.h" > +#include "winerror.h" > +#include "comctl32.h" > +#include "wine/debug.h" > + > +WINE_DEFAULT_DEBUG_CHANNEL(commctrl); > + Since it's move to its own file, let's call new debug channel "taskdlg" for instance.

2 2

Re: [v3 02/12] comctl32: Add notification codes and messages for TaskDialog to header
by Nikolay Sivov March 11, 2017

March 11, 2017

On 10.03.2017 21:21, Fabian Maurer wrote: > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > include/commctrl.h | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/include/commctrl.h b/include/commctrl.h > index d176b4fd0e..1bf9c279c5 100644 > --- a/include/commctrl.h > +++ b/include/commctrl.h > @@ -5228,6 +5228,38 @@ typedef struct _TASKDIALOG_BUTTON > PCWSTR pszButtonText; > } TASKDIALOG_BUTTON; > > +enum _TASKDIALOG_NOTIFICATIONS > +{ > + TDN_CREATED = 0, > + TDN_NAVIGATED = 1, > + TDN_BUTTON_CLICKED = 2, > + TDN_HYPERLINK_CLICKED = 3, > + TDN_TIMER = 4, > + TDN_DESTROYED = 5, > + TDN_RADIO_BUTTON_CLICKED = 6, > + TDN_DIALOG_CONSTRUCTED = 7, > + TDN_VERIFICATION_CLICKED = 8, > + TDN_HELP = 9, > + TDN_EXPANDO_BUTTON_CLICKED = 10 > +}; This is not how it's defined in SDK (v10). > + > +/* Taskdialog messages */ > +#define TDM_NAVIGATE_PAGE (WM_USER + 101) > +#define TDM_CLICK_BUTTON (WM_USER + 102) > +#define TDM_SET_MARQUEE_PROGRESS_BAR (WM_USER + 103) > +#define TDM_SET_PROGRESS_BAR_STATE (WM_USER + 104) > +#define TDM_SET_PROGRESS_BAR_RANGE (WM_USER + 105) > +#define TDM_SET_PROGRESS_BAR_POS (WM_USER + 106) > +#define TDM_SET_PROGRESS_BAR_MARQUEE (WM_USER + 107) > +#define TDM_SET_ELEMENT_TEXT (WM_USER + 108) > +#define TDM_CLICK_RADIO_BUTTON (WM_USER + 110) > +#define TDM_ENABLE_BUTTON (WM_USER + 111) > +#define TDM_ENABLE_RADIO_BUTTON (WM_USER + 112) > +#define TDM_CLICK_VERIFICATION (WM_USER + 113) > +#define TDM_UPDATE_ELEMENT_TEXT (WM_USER + 114) > +#define TDM_SET_BUTTON_ELEVATION_REQUIRED_STATE (WM_USER + 115) > +#define TDM_UPDATE_ICON (WM_USER + 116) > + Same here, message numbers are defined as enum members. > typedef HRESULT (CALLBACK *PFTASKDIALOGCALLBACK)(HWND, UINT, WPARAM, LPARAM, LONG_PTR); > > typedef struct _TASKDIALOGCONFIG >

2 2

Re: [PATCH 3/5] usp10: Use heap_alloc_zero() instead of HeapAlloc() with HEAP_ZERO_MEMORY.
by Sebastian Lackner March 11, 2017

March 11, 2017

On 09.03.2017 10:37, Henri Verbeet wrote: > -static inline BOOL heap_free(void *mem) > -{ > - return HeapFree(GetProcessHeap(), 0, mem); > -} [...] > +static inline void heap_free(void *mem) > +{ > + HeapFree(GetProcessHeap(), 0, mem); > +} I'm not sure if Francois is happy with this change. In other dlls, he has made the exact opposite change in his effort to standardize those functions.

4 3

Re: [v3 03/12] comctl32: Added basic implementation for task dialogs and add tests
by Sebastian Lackner March 10, 2017

March 10, 2017

On 10.03.2017 19:21, Fabian Maurer wrote: > + /* Skip this test on wine, because it doesn't really fail, > + * it would displays a dialog that doesn't automatically close */ > + if (strcmp(winetest_platform, "wine")) > + { > + ret = pTaskDialogIndirect(&info, NULL, NULL, NULL); > + ok(ret == S_OK, "Expected S_OK, got %x\n", ret); > + } We usually use winetest_interactive for that. There might be more comments, so please wait a bit before resending. ;) Also, please try to send your changes in reasonable sized chunks (max 5-7) in the future. Sending more patches at once only makes sense if they do not depend on each other. Best regards, Sebastian

3 6

Re: [v3 06/12] comctl32: TaskDialog - make thread-safe
by Nikolay Sivov March 10, 2017

March 10, 2017

On 10.03.2017 21:21, Fabian Maurer wrote: > + > +typedef struct > +{ > + const TASKDIALOGCONFIG *task_config; > + HWND hwnd; > +}taskdialog_info; It doesn't look very safe to keep original pointer here. If you only need callback pointer and user callback data, store them directly.

2 1

Re: [PATCH] dbghelp: Make SymGetLineFromAddr64 call SymGetLineFromAddrW64
by Sebastian Lackner March 10, 2017

March 10, 2017

On 10.03.2017 04:24, Alistair Leslie-Hughes wrote: > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/dbghelp/dbghelp_private.h | 3 ++ > dlls/dbghelp/symbol.c | 96 ++++++++++++++++++++++++++++++------------ > 2 files changed, 73 insertions(+), 26 deletions(-) > Could you give some explanation for the purpose of this patch? It looks a bit odd that we need two almost identical symt_fill_func_line_info implementations. > + if (found) > + { > + char *filename = (char*)source_get(module, dli->u.source_file); > + int len = lstrlenA(filename)+1; > + > + line->FileName = fetch_buffer(module->process, len * sizeof(WCHAR)); > + > + MultiByteToWideChar(CP_ACP, 0, filename, -1, line->FileName, len); [...] > - len = MultiByteToWideChar(CP_ACP, 0, l64->FileName, -1, NULL, 0); > - if ((l64w->FileName = fetch_buffer(pcs, len * sizeof(WCHAR)))) > - MultiByteToWideChar(CP_ACP, 0, l64->FileName, -1, l64w->FileName, len); > - l64w->Address = l64->Address; Actually, the old way of converting the strings looked more reasonable. Computing the correct length and checking for errors is preferred in such cases. Best regards, Sebastian

2 1

Re: [v3 09/12] comctl32: TaskDialog - Changed tests to allow sending messages
by Marvin March 10, 2017

March 10, 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=29422 Your paranoid android. === wvistau64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w2008s64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w7pro64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w864 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w1064 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?)

2 1

Re: [v3 12/12] comctl32: TaskDialog - Add ability to cancel dialog
by Marvin March 10, 2017

March 10, 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=29425 Your paranoid android. === wvistau64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w2008s64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w7pro64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w864 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w1064 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?)

1 0

Re: [v3 11/12] comctl32: TaskDialog - Set default button and add tests
by Marvin March 10, 2017

March 10, 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=29424 Your paranoid android. === wvistau64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w2008s64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w7pro64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w864 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w1064 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?)

1 0

Re: [PATCH 2/2] odbccp32: Implement SQLRemoveDriver/SQLRemoveDriverW
by Vincent Povirk March 10, 2017

March 10, 2017

> + if (usage_count) > + *usage_count = 0; If we're not going to do refcounting properly, it'd make more sense to hard-code this to 1 and never actually remove the drivers. Better to leave things around than delete them while they're still needed.

1 0

Re: [v3 10/12] comctl32: TaskDialog - Add button return value and tests
by Marvin March 10, 2017

March 10, 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=29423 Your paranoid android. === wvistau64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w2008s64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w7pro64 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w864 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?) === w1064 (64 bit taskdialog) === comctl32:taskdialog returned an error (3221226356) despite having no failures comctl32:taskdialog has no test summary line (early exit of the main process?)

1 0

Re: [PATCH 1/2] odbccp32: Implement SQLInstallDriverEx correctly
by Vincent Povirk March 10, 2017

March 10, 2017

> + if(RegSetValueExW(hkeydriver, driver, 0, REG_SZ, (BYTE*)installed, > + (sizeof(installed)+1)*sizeof(WCHAR)) != ERROR_SUCCESS) sizeof(installed) is already in bytes and includes the null terminator. > + /* Driver and Setup entries use the system path unless a path is specified. */ > + if(lstrcmpiW(driverW, entry) == 0 || lstrcmpiW(setupW, entry) == 0) > + { > + lstrcpyW(value, path); > + lstrcatW(value, slash); > + lstrcatW(value, divider); > + } > + else > + lstrcpyW(value, divider); Some sort of overflow check might be good here. > + ret = SQLInstallDriverEx("WINE ODBC Driver\0Driver=sample.dll\0Setup=sample.dll\0\0", NULL, path, MAX_PATH, &size, ODBC_INSTALL_COMPLETE, NULL); > + sql_ret = SQLInstallerErrorW(1, &error_code, NULL, 0, NULL); > + if (sql_ret && error_code == ODBC_ERROR_WRITING_SYSINFO_FAILED) > + { > + win_skip("not enough privileges\n"); > + return; > + } > + ok(sql_ret && error_code == SQL_SUCCESS, "SQLInstallDriverEx failed %d, %u\n", sql_ret, error_code); > + ok(!strcmp(path, syspath), "invalid path %s\n", path); This doesn't use the value of ret.

1 0

Re: [PATCH 4/5] usp10: Use heap_alloc() instead of HeapAlloc().
by Henri Verbeet March 10, 2017

March 10, 2017

On 10 March 2017 at 16:23, Aric Stewart <aric(a)codeweavers.com> wrote: > Feels a little like there are 2 unrelated changes here the heap_alloc work and then using the loaded_feature temporary variable to clean up the code. I am fine with both changes so I will sign off but i wonder if it makes the commit message a bit less clear. > Yeah, somewhat. It came up during the heap_alloc() work, but ended up touching a fair number of lines. In that regard it could have been split out, although I figured it wasn't too bad.

1 0

Re: [PATCH 2/2 resend] devenum: Return S_OK in IMoniker::IsEqual() stub.
by Andrew Eikum March 10, 2017

March 10, 2017

I'm not sure how application use these monikers, but lying about moniker equality makes me a little nervous. This doesn't look that hard to implement correctly. Andrew On Fri, Mar 10, 2017 at 06:26:55PM +0800, Jactry Zeng wrote: > Superseded patch 131295. > > Signed-off-by: Jactry Zeng <jzeng(a)codeweavers.com> > --- > dlls/devenum/mediacatenum.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > > diff --git a/dlls/devenum/mediacatenum.c b/dlls/devenum/mediacatenum.c > index 66343342c9..3543182cf4 100644 > --- a/dlls/devenum/mediacatenum.c > +++ b/dlls/devenum/mediacatenum.c > @@ -530,7 +530,10 @@ static HRESULT WINAPI DEVENUM_IMediaCatMoniker_IsEqual(IMoniker *iface, IMoniker > { > FIXME("(%p)->(%p): stub\n", iface, pmkOtherMoniker); > > - return E_NOTIMPL; > + if (!pmkOtherMoniker) > + return E_INVALIDARG; > + > + return S_OK; > } > > static HRESULT WINAPI DEVENUM_IMediaCatMoniker_Hash(IMoniker *iface, DWORD *pdwHash) > >

1 0

Re: [1/2] services/tests: Add test for database locking.
by Marvin March 10, 2017

March 10, 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=29411 Your paranoid android. === w7u (32 bit service) === service.c:407: Test failed: LockServiceDatabase failed: 1055 service.c:413: Test failed: UnlockServiceDatabase failed: 1071

2 1

Re: [PATCH 1/2] devenum: Implement IMoniker::GetClassID().
by Nikolay Sivov March 10, 2017

March 10, 2017

On 10.03.2017 12:32, Jactry Zeng wrote: > Signed-off-by: Jactry Zeng <jzeng(a)codeweavers.com> > --- > dlls/devenum/mediacatenum.c | 8 +++++--- > dlls/devenum/tests/devenum.c | 9 +++++++++ > 2 files changed, 14 insertions(+), 3 deletions(-) > > - FIXME("(%p)->(%p): stub\n", This, pClassID); > + TRACE("(%p)->(%p): stub\n", This, pClassID); You can remove 'stub' now.

2 1

Re: [PATCH] oledb32: Downgrade IDataSourceLocator_QueryInterface FIXME to WARN
by Huw Davies March 10, 2017

March 10, 2017

On Fri, Mar 10, 2017 at 04:31:02AM +0000, Alistair Leslie-Hughes wrote: > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/oledb32/dslocator.c | 2 +- > dlls/oledb32/tests/database.c | 4 ++++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/dlls/oledb32/dslocator.c b/dlls/oledb32/dslocator.c > index 11ed3e9..055357e 100644 > --- a/dlls/oledb32/dslocator.c > +++ b/dlls/oledb32/dslocator.c > @@ -80,7 +80,7 @@ static HRESULT WINAPI dslocator_QueryInterface(IDataSourceLocator *iface, REFIID > return S_OK; > } > > - FIXME("interface %s not implemented\n", debugstr_guid(riid)); > + WARN("interface %s not implemented\n", debugstr_guid(riid)); > return E_NOINTERFACE; > } I think leaving the FIXME, but skipping it if the iid is IID_IRunnableObject would be better. Huw.

1 0

Re: [PATCH 1/2] comctl32/propsheet: Diverse UI fixes
by Nikolay Sivov March 10, 2017

March 10, 2017

On 09.03.2017 23:19, Hermès BÉLUSCA-MAÏTO wrote: > [PATCH 1/2] comctl32/propsheet: Diverse UI fixes: > > - Draw static text elements with a transparent background, so that they > acquire the correct background color of the wizard window. > > - Modify the check for the header bitmap, as the header's hbmHeader is > initialized also if the flag PSH_HEADER but not PSH_USEHBMHEADER is set > (see function PROPSHEET_LoadWizardBitmaps). > > - Add a check for header's hbmWatermark so that the watermark is drawn > only when a valid bitmap handle is present. - Fix a copy-pasta error > when drawing the header's subtitle. > > > > Hi, Hermès. Thanks for the patches, some comments follow. > case WM_CTLCOLORSTATIC: > - SetBkColor((HDC)wParam, GetSysColor(COLOR_WINDOW)); > - return (INT_PTR)GetSysColorBrush(COLOR_WINDOW); > + SetBkMode((HDC)wParam, TRANSPARENT); > + return (INT_PTR)GetStockObject(HOLLOW_BRUSH); > } Do you have a simple test program to confirm this? This should be a separate patch. > @@ -3373,7 +3373,7 @@ static LRESULT PROPSHEET_Paint(HWND hwnd, HDC hdcParam) > if (ppshpage->dwFlags & PSP_USEHEADERSUBTITLE) { > SelectObject(hdc, psInfo->hFont); > SetRect(&r, 40, 25, rzone.right - 69, rzone.bottom); > - if (!IS_INTRESOURCE(ppshpage->pszHeaderTitle)) > + if (!IS_INTRESOURCE(ppshpage->pszHeaderSubTitle)) > DrawTextW(hdc, ppshpage->pszHeaderSubTitle, -1, &r, DT_LEFT | DT_WORDBREAK); > else > { Looks good, please make it a separate patch too. > @@ -3298,7 +3298,7 @@ static LRESULT PROPSHEET_Paint(HWND hwnd, HDC hdcParam) > > hOldFont = SelectObject(hdc, psInfo->hFontBold); > > - if (psInfo->ppshheader.dwFlags & PSH_USEHBMHEADER) > + if (psInfo->ppshheader.u5.hbmHeader) > { > hbmp = SelectObject(hdcSrc, psInfo->ppshheader.u5.hbmHeader); > I'll need a visual test to confirm this, looks a bit strange that flags are ignored. > @@ -3395,7 +3395,8 @@ static LRESULT PROPSHEET_Paint(HWND hwnd, HDC hdcParam) > > if ( (ppshpage && (ppshpage->dwFlags & PSP_HIDEHEADER)) && > (psInfo->ppshheader.dwFlags & (PSH_WIZARD97_OLD | PSH_WIZARD97_NEW)) && > - (psInfo->ppshheader.dwFlags & PSH_WATERMARK) ) > + (psInfo->ppshheader.dwFlags & PSH_WATERMARK) && > + (psInfo->ppshheader.u4.hbmWatermark) ) > { > HWND hwndLine = GetDlgItem(hwnd, IDC_SUNKEN_LINE); This looks reasonable, also deserved a separate patch.

1 1

Re: [PATCH 5/7] xmllite/reader: Fix position methods return values in closed reader state
by Marvin March 10, 2017

March 10, 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=29381 Your paranoid android. === wxppro (32 bit reader) === reader.c:2536: Test failed: Expected (XmlReadState_Initial), got (XmlReadState_Closed) === w2003std (32 bit reader) === reader.c:2536: Test failed: Expected (XmlReadState_Initial), got (XmlReadState_Closed)

1 0

Wine Mono vagrant build environment
by Vincent Povirk March 10, 2017

March 10, 2017

I'm not sure if anyone will care about this, but I figured I should mention it somewhere. With the release of Wine Mono 4.7.0 today, the source tree now ships with a Vagrantfile, which can be used with Vagrant to create a virtual machine that can build the project. This was the environment used to create the msi package, and I plan to continue using it this way for future releases. Previously, the official build environment was my work PC, which means there was always the possibility that upgrading or changing it would have some effect on the official packages. I don't know if this is useful for anyone else, but I figure that an easy way to create a build environment, along with the complete documentation of that environment which we get as a side-effect, can only be a good thing. This is not a replacement for building on real hardware. I only plan to use it to make official builds, not for development, and I still want to hear about any issues people encounter while trying to do the build on their own systems.

2 1

Re: [PATCH 4/5] d3d9/tests: Skip UpdateTexture tests if texture filtering isn't supported.
by Marvin March 10, 2017

March 10, 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=29375 Your paranoid android. === w1064 (32 bit visual) === visual.c:7947: Test failed: Input test: Quad 3(2crd-wrongidx) returned color 0x00ff00ff, expected 0x00ff0080 visual.c:7995: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors). visual.c:8099: Test failed: Got unexpected color 0x00ff0000 for no color attribute test.

1 0

Re: [PATCH v2 2/5] ddraw/tests: Add a ddraw7 vertex buffer refcount test.
by Marvin March 10, 2017

March 10, 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=29374 Your paranoid android. === w8 (32 bit ddraw7) === ddraw7.c:3763: Test failed: Lit quad with light has color 0x00000000. ddraw7.c:3763: Test failed: Lit quad with singular world matrix has color 0x00000000. ddraw7.c:3763: Test failed: Lit quad with transformation matrix has color 0x00000000.

1 0

RE: ***SPAM*** Re: [PATCH 1/2] comctl32/propsheet: Diverse UI fixes
by Hermès BÉLUSCA-MAÏTO March 9, 2017

March 9, 2017

Hello Nikolay, I'll resend tomorrow the fixes in separate patches, and add a simple test app (with source) for you to have a look. Thank you for your reviewing, Hermès -----Message d'origine----- De : wine-patches [mailto:wine-patches-bounces(a)winehq.org] De la part de Nikolay Sivov Envoyé : jeudi 9 mars 2017 23:17 À : wine-patches(a)winehq.org Objet : ***SPAM*** Re: [PATCH 1/2] comctl32/propsheet: Diverse UI fixes On 09.03.2017 23:19, Hermès BÉLUSCA-MAÏTO wrote: > [PATCH 1/2] comctl32/propsheet: Diverse UI fixes: > > - Draw static text elements with a transparent background, so that > they acquire the correct background color of the wizard window. > > - Modify the check for the header bitmap, as the header's hbmHeader is > initialized also if the flag PSH_HEADER but not PSH_USEHBMHEADER is > set (see function PROPSHEET_LoadWizardBitmaps). > > - Add a check for header's hbmWatermark so that the watermark is drawn > only when a valid bitmap handle is present. - Fix a copy-pasta error > when drawing the header's subtitle. > > > > Hi, Hermès. Thanks for the patches, some comments follow. > case WM_CTLCOLORSTATIC: > - SetBkColor((HDC)wParam, GetSysColor(COLOR_WINDOW)); > - return (INT_PTR)GetSysColorBrush(COLOR_WINDOW); > + SetBkMode((HDC)wParam, TRANSPARENT); > + return (INT_PTR)GetStockObject(HOLLOW_BRUSH); > } Do you have a simple test program to confirm this? This should be a separate patch. > @@ -3373,7 +3373,7 @@ static LRESULT PROPSHEET_Paint(HWND hwnd, HDC hdcParam) > if (ppshpage->dwFlags & PSP_USEHEADERSUBTITLE) { > SelectObject(hdc, psInfo->hFont); > SetRect(&r, 40, 25, rzone.right - 69, rzone.bottom); > - if (!IS_INTRESOURCE(ppshpage->pszHeaderTitle)) > + if (!IS_INTRESOURCE(ppshpage->pszHeaderSubTitle)) > DrawTextW(hdc, ppshpage->pszHeaderSubTitle, -1, &r, DT_LEFT | DT_WORDBREAK); > else > { Looks good, please make it a separate patch too. > @@ -3298,7 +3298,7 @@ static LRESULT PROPSHEET_Paint(HWND hwnd, HDC > hdcParam) > > hOldFont = SelectObject(hdc, psInfo->hFontBold); > > - if (psInfo->ppshheader.dwFlags & PSH_USEHBMHEADER) > + if (psInfo->ppshheader.u5.hbmHeader) > { > hbmp = SelectObject(hdcSrc, > psInfo->ppshheader.u5.hbmHeader); > I'll need a visual test to confirm this, looks a bit strange that flags are ignored. > @@ -3395,7 +3395,8 @@ static LRESULT PROPSHEET_Paint(HWND hwnd, HDC > hdcParam) > > if ( (ppshpage && (ppshpage->dwFlags & PSP_HIDEHEADER)) && > (psInfo->ppshheader.dwFlags & (PSH_WIZARD97_OLD | PSH_WIZARD97_NEW)) && > - (psInfo->ppshheader.dwFlags & PSH_WATERMARK) ) > + (psInfo->ppshheader.dwFlags & PSH_WATERMARK) && > + (psInfo->ppshheader.u4.hbmWatermark) ) > { > HWND hwndLine = GetDlgItem(hwnd, IDC_SUNKEN_LINE); This looks reasonable, also deserved a separate patch.

1 0

Re: [PATCH 2/2] comctl32/propsheet: Make the PROPSHEET_RemovePage function similar to PROPSHEET_CleanUp
by Nikolay Sivov March 9, 2017

March 9, 2017

On 09.03.2017 23:20, Hermès BÉLUSCA-MAÏTO wrote: > [PATCH 2/2] comctl32/propsheet: Make the PROPSHEET_RemovePage function > similar to PROPSHEET_CleanUp: > > - Use an auxiliary psp pointer to hold the psInfo->proppage[index].hpage; > > - Check psInfo->proppage[index].hwndPage before destroying the > associated window. > > /* Destroy page dialog window */ > - DestroyWindow(psInfo->proppage[index].hwndPage); > + if(psInfo->proppage[index].hwndPage) > + DestroyWindow(psInfo->proppage[index].hwndPage); Why is this important?

1 0

Re: [1/4] webservices: Unlock critical section before returning.
by Alexandre Julliard March 9, 2017

March 9, 2017

Sebastian Lackner <sebastian(a)fds-team.de> writes: > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > > Do I have to send feedback to the patches list to make sure it is not missed? ;) Sorry about that. Having replies go to the patches list would make things easier for me, but I expect others prefer to keep wine-patches only for patches. OTOH we already have signoffs in there... -- Alexandre Julliard julliard(a)winehq.org

5 5

Re: [PATCH] user32: Always send WM_NCACTIVATE when changing the foreground window.
by Alexandre Julliard March 9, 2017

March 9, 2017

Alex Henrie <alexhenrie24(a)gmail.com> writes: > + if (send_msg_old) /* old window belongs to other thread */ > + SendNotifyMessageW( previous, WM_WINE_SETACTIVEWINDOW, 0, 0 ); > + else if (send_msg_new) /* old window belongs to us but new one to other thread */ > + ret = set_active_window( 0, NULL, mouse, TRUE ); > + > + if (GetActiveWindow() == hwnd) > { The active window is per-thread but SetForegroundWindow works across threads, so this can't do the right thing. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 10/10] wininet/tests: Added more HTTP read tests.
by Marvin March 9, 2017

March 9, 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=29364 Your paranoid android. === wxppro (32 bit http) === http.c:5050: Test failed: http.c:314: Test failed: InternetReadFileExW failed: 997 http.c:314: Test failed: unexpected status 100 (INTERNET_STATUS_REQUEST_COMPLETE) http.c:4634: Test failed: expected status 100 (INTERNET_STATUS_REQUEST_COMPLETE) 1 times, received 2 times === wvistau64 (32 bit http) === http.c:5050: Test failed: InternetReadFileExW failed: 997 http.c:5056: Test failed: InternetQueryDataAvailable returned: 1(997) http.c:5056: Test failed: size = 10 http.c:314: Test failed: unexpected status 100 (INTERNET_STATUS_REQUEST_COMPLETE) http.c:5060: Test failed: dwBufferLength = 10 http.c:5061: Test failed: expected status 50 (INTERNET_STATUS_CLOSING_CONNECTION) 1 times, received 0 times http.c:5062: Test failed: expected status 51 (INTERNET_STATUS_CONNECTION_CLOSED) 1 times, received 0 times http.c:314: Test failed: unexpected status 50 (INTERNET_STATUS_CLOSING_CONNECTION) http.c:314: Test failed: unexpected status 51 (INTERNET_STATUS_CONNECTION_CLOSED) http.c:5104: Test failed: expected status 50 (INTERNET_STATUS_CLOSING_CONNECTION) 1 times, received 2 times http.c:5105: Test failed: expected status 51 (INTERNET_STATUS_CONNECTION_CLOSED) 1 times, received 2 times === w2008s64 (32 bit http) === http.c:5050: Test failed: InternetReadFileExW failed: 997 http.c:314: Test failed: unexpected status 100 (INTERNET_STATUS_REQUEST_COMPLETE) http.c:5061: Test failed: expected status 50 (INTERNET_STATUS_CLOSING_CONNECTION) 1 times, received 0 times http.c:5062: Test failed: expected status 51 (INTERNET_STATUS_CONNECTION_CLOSED) 1 times, received 0 times http.c:314: Test failed: unexpected status 51 (INTERNET_STATUS_CONNECTION_CLOSED) http.c:314: Test failed: unexpected status 100 (INTERNET_STATUS_REQUEST_COMPLETE) http.c:4634: Test failed: expected status 100 (INTERNET_STATUS_REQUEST_COMPLETE) 1 times, received 2 times http.c:5105: Test failed: expected status 51 (INTERNET_STATUS_CONNECTION_CLOSED) 1 times, received 2 times === wvistau64 (64 bit http) === http.c:5050: Test failed: InternetReadFileExW failed: 997 http.c:314: Test failed: http.c:382: 0364:Callback 0000000000CC0014 0xdeadbeef INTERNET_STATUS_CLOSING_CONNECTION 0000000000000000 0 http.c:4731: Test failed: expected status 100 (INTERNET_STATUS_REQUEST_COMPLETE) 1 times, received 2 times === w2008s64 (64 bit http) === http.c:5050: Test failed: http.c:314: Test failed: unexpected status 100 (INTERNET_STATUS_REQUEST_COMPLETE) http.c:5062: Test failed: expected status 51 (INTERNET_STATUS_CONNECTION_CLOSED) 1 times, received 0 times http.c:314: Test failed: unexpected status 100 (INTERNET_STATUS_REQUEST_COMPLETE) http.c:4634: Test failed: expected status 100 (INTERNET_STATUS_REQUEST_COMPLETE) 1 times, received 2 times

2 1

Re: [PATCH 05/10] wininet: Rewrite InternetReadFileEx for http protocol to make use of non-blocking reads.
by Marvin March 9, 2017

March 9, 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=29362 Your paranoid android. === w2008s64 (64 bit http) === http.c:628: Test failed: req_error = 12029 http.c:652: Test failed: expected status 30 (INTERNET_STATUS_SENDING_REQUEST) 1 times, received 0 times http.c:653: Test failed: expected status 31 (INTERNET_STATUS_REQUEST_SENT) 1 times, received 0 times http.c:654: Test failed: expected status 40 (INTERNET_STATUS_RECEIVING_RESPONSE) 1 times, received 0 times http.c:655: Test failed: expected status 41 (INTERNET_STATUS_RESPONSE_RECEIVED) 1 times, received 0 times http.c:664: Test failed: flags = 8, expected 0 0b30:http: unhandled exception c0000005 at 00000000004162E8

1 0

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

March 9, 2017

On 27.02.2017 16:09, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus.h | 3 + > dlls/winebus.sys/bus_udev.c | 150 ++++++++++++++++++++++++++++++++++++++++++-- > dlls/winebus.sys/main.c | 26 ++++++++ > 3 files changed, 173 insertions(+), 6 deletions(-) > I assume this is meant as a fallback, if we don't have access to hidraw, right? If you plan to use this as a replacement for dinput, it would be better to implement it without fixed dependency on udev. > > > 0001-winebus.sys-Handle-linux-input-event-device-via-udev.txt > > > diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h > index effbd4afd0..fed35e8c1e 100644 > --- a/dlls/winebus.sys/bus.h > +++ b/dlls/winebus.sys/bus.h > @@ -43,3 +43,6 @@ DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev > void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; > void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD length) DECLSPEC_HIDDEN; > + > +/* General Bus Functions */ > +DWORD check_bus_option(UNICODE_STRING *registry_path, const UNICODE_STRING *option) DECLSPEC_HIDDEN; > diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c > index 415bd3db08..2dcc7fb442 100644 > --- a/dlls/winebus.sys/bus_udev.c > +++ b/dlls/winebus.sys/bus_udev.c > @@ -42,6 +42,14 @@ > # include <sys/ioctl.h> > #endif > > +#ifdef HAVE_LINUX_INPUT_H > +# include <linux/input.h> > +# undef SW_MAX > +# if defined(EVIOCGBIT) && defined(EV_ABS) && defined(BTN_PINKIE) > +# define HAS_PROPER_INPUT_HEADER > +# endif > +#endif > + > #define NONAMELESSUNION > > #include "ntstatus.h" > @@ -65,11 +73,15 @@ WINE_DECLARE_DEBUG_CHANNEL(hid_report); > > static struct udev *udev_context = NULL; > static DRIVER_OBJECT *udev_driver_obj = NULL; > +static DWORD disable_hidraw = 0; > +static DWORD disable_input = 0; > > static const WCHAR hidraw_busidW[] = {'H','I','D','R','A','W',0}; > +static const WCHAR lnxev_busidW[] = {'L','N','X','E','V',0}; > > #include "initguid.h" > DEFINE_GUID(GUID_DEVCLASS_HIDRAW, 0x3def44ad,0x242e,0x46e5,0x82,0x6d,0x70,0x72,0x13,0xf3,0xaa,0x81); > +DEFINE_GUID(GUID_DEVCLASS_LINUXEVENT, 0x1b932c0d,0xfea7,0x42cd,0x8e,0xaa,0x0e,0x48,0x79,0xb6,0x9e,0xaa); > > struct platform_private > { > @@ -392,6 +404,52 @@ static const platform_vtbl hidraw_vtbl = > hidraw_set_feature_report, > }; > > +#ifdef HAS_PROPER_INPUT_HEADER > + > +static NTSTATUS lnxev_get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS lnxev_get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS lnxev_begin_report_processing(DEVICE_OBJECT *device) > +{ > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS lnxev_set_output_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > +{ > + *written = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS lnxev_get_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *read) > +{ > + *read = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static NTSTATUS lnxev_set_feature_report(DEVICE_OBJECT *device, UCHAR id, BYTE *report, DWORD length, ULONG_PTR *written) > +{ > + *written = 0; > + return STATUS_NOT_IMPLEMENTED; > +} > + > +static const platform_vtbl lnxev_vtbl = { > + compare_platform_device, > + lnxev_get_reportdescriptor, > + lnxev_get_string, > + lnxev_begin_report_processing, > + lnxev_set_output_report, > + lnxev_get_feature_report, > + lnxev_set_feature_report, > +}; > +#endif > + > static void try_add_device(struct udev_device *dev) > { > DWORD vid = 0, pid = 0, version = 0; > @@ -400,6 +458,7 @@ static void try_add_device(struct udev_device *dev) > const char *subsystem; > const char *devnode; > WCHAR *serial = NULL; > + const char* gamepad = NULL; > int fd; > > if (!(devnode = udev_device_get_devnode(dev))) > @@ -419,6 +478,26 @@ static void try_add_device(struct udev_device *dev) > version = get_sysattr_dword(usbdev, "version", 10); > serial = get_sysattr_string(usbdev, "serial"); > } > +#ifdef HAS_PROPER_INPUT_HEADER > + else > + { > + struct input_id device_id = {0}; > + char device_uid[255] = {0}; > + > + if (ioctl(fd, EVIOCGID, &device_id) == -1) > + WARN("ioctl(EVIOCGID) failed: %d %s\n", errno, strerror(errno)); > + if (ioctl(fd, EVIOCGUNIQ(254), device_uid) && device_uid[0]) > + MultiByteToWideChar(CP_UNIXCP, 0, device_uid, -1, serial, 255); I don't think this will work as expected. serial is a NULL pointer, and the check if ioctl() was successful is also done wrong. On error a value < 0 is returned. > + > + gamepad = udev_device_get_property_value(dev, "ID_INPUT_JOYSTICK"); > + vid = device_id.vendor; > + pid = device_id.product; > + version = device_id.version; > + } > +#else > + else > + WARN("Could not get device to query VID, PID, Version and Serial\n"); > +#endif > > TRACE("Found udev device %s (vid %04x, pid %04x, version %u, serial %s)\n", > debugstr_a(devnode), vid, pid, version, debugstr_w(serial)); > @@ -429,6 +508,12 @@ static void try_add_device(struct udev_device *dev) > device = bus_create_hid_device(udev_driver_obj, hidraw_busidW, vid, pid, version, 0, serial, FALSE, > &GUID_DEVCLASS_HIDRAW, &hidraw_vtbl, sizeof(struct platform_private)); > } > +#ifdef HAS_PROPER_INPUT_HEADER > + else if (strcmp(subsystem, "input") == 0) > + { > + device = bus_create_hid_device(udev_driver_obj, lnxev_busidW, vid, pid, version, 0, serial, (gamepad != NULL), &GUID_DEVCLASS_LINUXEVENT, &lnxev_vtbl, sizeof(struct platform_private)); Some linebreaks wouldn't hurt. > + } > +#endif > > if (device) > { > @@ -448,8 +533,17 @@ static void try_add_device(struct udev_device *dev) > > static void try_remove_device(struct udev_device *dev) > { > - DEVICE_OBJECT *device = bus_find_hid_device(&hidraw_vtbl, dev); > - struct platform_private *private; > + DEVICE_OBJECT *device = NULL; > + const char* subsystem; > + struct platform_private* private; > + > + subsystem = udev_device_get_subsystem(dev); > + if (strcmp(subsystem, "hidraw") == 0) > + device = bus_find_hid_device(&hidraw_vtbl, dev); > +#ifdef HAS_PROPER_INPUT_HEADER > + else if (strcmp(subsystem, "input") == 0) > + device = bus_find_hid_device(&lnxev_vtbl, dev); > +#endif It might be easier to just check both after each other, like: device = bus_find_hid_device(...); if (!device) device = bus_find_hid_device(...); > if (!device) return; > > IoInvalidateDeviceRelations(device, RemovalRelations); > @@ -483,8 +577,18 @@ static void build_initial_deviceset(void) > return; > } > > - if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0) > - WARN("Failed to add subsystem 'hidraw' to enumeration\n"); > + if (!disable_hidraw) > + if (udev_enumerate_add_match_subsystem(enumerate, "hidraw") < 0) > + WARN("Failed to add subsystem 'hidraw' to enumeration\n"); > +#ifdef HAS_PROPER_INPUT_HEADER > + if (!disable_input) > + { > + if (udev_enumerate_add_match_subsystem(enumerate, "event") < 0) > + WARN("Failed to add subsystem 'event' to enumeration\n"); Why do you add this match when results are not processed anywhere? > + if (udev_enumerate_add_match_subsystem(enumerate, "input") < 0) > + WARN("Failed to add subsystem 'input' to enumeration\n"); > + } > +#endif > > if (udev_enumerate_scan_devices(enumerate) < 0) > WARN("Enumeration scan failed\n"); > @@ -509,6 +613,7 @@ static void build_initial_deviceset(void) > static struct udev_monitor *create_monitor(struct pollfd *pfd) > { > struct udev_monitor *monitor; > + int systems = 0; > > monitor = udev_monitor_new_from_netlink(udev_context, "udev"); > if (!monitor) > @@ -517,8 +622,27 @@ static struct udev_monitor *create_monitor(struct pollfd *pfd) > return NULL; > } > > - if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "hidraw", NULL) < 0) > - WARN("Failed to add subsystem 'hidraw' to monitor\n"); > + if (!disable_hidraw) > + { > + if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "hidraw", NULL) < 0) > + WARN("Failed to add 'hidraw' subsystem to monitor\n"); > + else > + systems++; > + } > +#ifdef HAS_PROPER_INPUT_HEADER > + if (!disable_input) > + { > + if (udev_monitor_filter_add_match_subsystem_devtype(monitor, "input", NULL) < 0) > + WARN("Failed to add 'input' subsystem to monitor\n"); > + else > + systems++; > + } > +#endif > + if (systems == 0) > + { > + WARN("No subsystems added to monitor\n"); > + goto error; > + } > > if (udev_monitor_enable_receiving(monitor) < 0) > goto error; > @@ -589,6 +713,10 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry > { > HANDLE events[2]; > DWORD result; > + static const WCHAR hidraw_disabledW[] = {'D','i','s','a','b','l','e',' ','h','i','d','r','a','w',0}; > + static const UNICODE_STRING hidraw_disabled = {sizeof(hidraw_disabledW) - sizeof(WCHAR), sizeof(hidraw_disabledW), (WCHAR*)hidraw_disabledW}; > + static const WCHAR input_disabledW[] = {'D','i','s','a','b','l','e',' ','i','n','p','u','t',0}; > + static const UNICODE_STRING input_disabled = {sizeof(input_disabledW) - sizeof(WCHAR), sizeof(input_disabledW), (WCHAR*)input_disabledW}; > > TRACE("(%p, %s)\n", driver, debugstr_w(registry_path->Buffer)); > > @@ -602,6 +730,16 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry > driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch; > driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch; > > + disable_hidraw = check_bus_option(registry_path, &hidraw_disabled); > + if (disable_hidraw) > + TRACE("UDEV hidraw devices disabled in registry\n"); > + > +#ifdef HAS_PROPER_INPUT_HEADER > + disable_input = check_bus_option(registry_path, &input_disabled); > + if (disable_input) > + TRACE("UDEV input devices disabled in registry\n"); > +#endif Does it hurt when both hidraw and input event devices are found? If yes, it would be better to filter duplicates somewhere else, instead of using registry keys. > + > if (!(events[0] = CreateEventW(NULL, TRUE, FALSE, NULL))) > goto error; > if (!(events[1] = CreateThread(NULL, 0, deviceloop_thread, events[0], 0, NULL))) > diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c > index 978540bb17..0388f0263c 100644 > --- a/dlls/winebus.sys/main.c > +++ b/dlls/winebus.sys/main.c > @@ -625,6 +625,32 @@ void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD length) > LeaveCriticalSection(&ext->report_cs); > } > > +DWORD check_bus_option(UNICODE_STRING *registry_path, const UNICODE_STRING *option) > +{ > + OBJECT_ATTRIBUTES attr; > + HANDLE key; > + DWORD output = 0; > + > + InitializeObjectAttributes(&attr, registry_path, OBJ_CASE_INSENSITIVE | OBJ_KERNEL_HANDLE, NULL, NULL); > + if (NtOpenKey(&key, KEY_ALL_ACCESS, &attr) == STATUS_SUCCESS) > + { > + DWORD size; > + char buffer[FIELD_OFFSET(KEY_VALUE_PARTIAL_INFORMATION, Data[sizeof(DWORD)])]; > + > + KEY_VALUE_PARTIAL_INFORMATION *info = (KEY_VALUE_PARTIAL_INFORMATION*)buffer; > + > + if (NtQueryValueKey(key, option, KeyValuePartialInformation, info, sizeof(buffer), &size) == STATUS_SUCCESS) > + { > + if (info->Type == REG_DWORD) > + output = *(DWORD*)info->Data; > + } > + > + NtClose(key); > + } > + > + return output; > +} > + > NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) > { > static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0}; > > >

3 5

Re: [PATCH 2/8] wined3d: Unbind textures in context_bind_shader_resources().
by Henri Verbeet March 9, 2017

March 9, 2017

On 9 March 2017 at 10:03, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Signed-off-by: Józef Kucia <jkucia(a)codeweavers.com> > --- > > The WINED3D_LEGACY_TEXTURE_BINDING flag should prevent from generating > the "No resource view bound at index" warnings in D3D9-, allows to > safely unbind resources in context_bind_shader_resources(), and make it > easier easier to add support for more samplers in D3D10+. > The issue is texture bindings set through wined3d_device_set_texture(), right? Could we get rid of wined3d_device_set_texture() and create shader resource views for those textures instead, similar to how we create rendertarget views in d3d9, etc.? IIRC one of the issues was interactions with sampler and texture stage states. Tangentially, how does this work with "Aon9" shaders?

2 3

Re: [PATCH] crypt32/tests: Fix a memory leak in testFindCRL.
by Erich E. Hoover March 8, 2017

March 8, 2017

On Sun, Feb 26, 2017 at 11:52 PM, Alex Henrie <alexhenrie24(a)gmail.com> wrote: > ... > + store = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, 0, > + CERT_STORE_CREATE_NEW_FLAG, NULL); > + if (!store) return; > + > ... Would you mind adding an error when this happens? Otherwise we may not know that these tests did not run. Best, Erich

2 2

Re: [PATCH 5/5] d3d9/tests: Fix pixel shader version check in test_pointsize().
by Marvin March 8, 2017

March 8, 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=29312 Your paranoid android. === w8 (32 bit visual) === visual.c:7931: Test failed: Input test: Quad 3(2crd-wrongidx) returned color 0x00ff00ff, expected 0x00ff0080 visual.c:8099: Test failed: Got unexpected color 0x00ff0000 for no color attribute test. === w864 (64 bit visual) === visual.c:7947: Test failed: Input test: Quad 3(2crd-wrongidx) returned color 0x00ff00ff, expected 0x00ff0080 visual.c:7995: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors). visual.c:8099: Test failed: Got unexpected color 0x00ff0000 for no color attribute test.

1 0

Re: [PATCH 4/5] d3d9/tests: Skip test_vshader_input() if PS 3.0 isn't supported.
by Marvin March 8, 2017

March 8, 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=29311 Your paranoid android. === w864 (32 bit visual) === visual.c:8099: Test failed: Got unexpected color 0x00ff00ff for no color attribute test.

1 0

Re: [2/8] webservices: Protect messages with a critical section.
by Sebastian Lackner March 8, 2017

March 8, 2017

On 08.03.2017 11:25, Hans Leidekker wrote: > + } > } > > msg->is_addressed = TRUE; Not sure if it matters, but previously this was skipped in case of a failure. Also please note that depending on how many objects are used by an application, a separate CS for each might significantly increase memory usage. > - return S_OK; > + > + LeaveCriticalSection( &msg->cs ); > + return hr; [...] > - msg->init = init; > - msg->state = WS_MESSAGE_STATE_INITIALIZED; > - return write_envelope( msg ); > + if ((hr = write_envelope( msg )) == S_OK) > + { > + msg->init = init; > + msg->state = WS_MESSAGE_STATE_INITIALIZED; > + } Here, the new code also does something else than the old one. Previously the msg fields were changed unconditionally.

2 1

Re: [5/8] webservices: Protect writers with a critical section.
by Sebastian Lackner March 8, 2017

March 8, 2017

On 08.03.2017 11:25, Hans Leidekker wrote: > + if ((hr = write_set_element_namespace( writer )) != S_OK) goto done; > + if ((hr = write_startelement( writer )) != S_OK) goto done; > + if ((hr = write_endstartelement( writer )) != S_OK) goto done; > writer->state = WRITER_STATE_ENDSTARTELEMENT; > + > +done: > + LeaveCriticalSection( &writer->cs ); > return S_OK; You should return hr here. > for (i = offset; i < count; i++) > { > const char *ptr = (const char *)array + (offset + i) * type_size; > - if ((hr = write_element_node( writer, NULL, localname, ns )) != S_OK) return hr; > + if ((hr = write_element_node( writer, NULL, localname, ns )) != S_OK) goto done; > if ((hr = write_type( writer, WS_ELEMENT_TYPE_MAPPING, type, NULL, WS_WRITE_REQUIRED_POINTER, > - &ptr, sizeof(ptr) )) != S_OK) return hr; > - if ((hr = write_endelement_node( writer )) != S_OK) return hr; > + &ptr, sizeof(ptr) )) != S_OK) goto done; > + if ((hr = write_endelement_node( writer )) != S_OK) goto done; > } > > - return S_OK; > +done: > + LeaveCriticalSection( &writer->cs ); > + return hr; It looks like hr will be undefined here if count == 0 > } > > /************************************************************************** > @@ -2392,9 +2662,20 @@ HRESULT WINAPI WsWriteXmlBuffer( WS_XML_WRITER *handle, WS_XML_BUFFER *buffer, W > > if (!writer || !xmlbuf) return E_INVALIDARG; > > - if ((hr = write_flush( writer )) != S_OK) return hr; > - if ((hr = write_grow_buffer( writer, xmlbuf->size )) != S_OK) return hr; > + EnterCriticalSection( &writer->cs ); > + > + if (writer->magic != WRITER_MAGIC) > + { > + LeaveCriticalSection( &writer->cs ); > + return E_INVALIDARG; > + } > + > + if ((hr = write_flush( writer )) != S_OK) goto done; > + if ((hr = write_grow_buffer( writer, xmlbuf->size )) != S_OK) goto done; > write_bytes( writer, xmlbuf->ptr, xmlbuf->size ); > + > +done: > + LeaveCriticalSection( &writer->cs ); > return S_OK; Here you should also use hr.

1 0

Re: [PATCH] kernel32/tests: Add 64bit number tests for FormatMessage[A|W] (try 2)
by Sebastian Lackner March 8, 2017

March 8, 2017

On 08.03.2017 15:42, Bruno Jesus wrote: > + { -1, "-1", 2}, > + { ULL(0xFFFFFFFF,0xFFFFFFFF), "-1", 2}, You can remove the second test. 0xffffffffffffffff and -1 are just different representations for the same number. Best regards, Sebastian

2 1

Re: bcrypt: Add a builtin sha384/sha512 implementation. (v2)
by Sebastian Lackner March 8, 2017

March 8, 2017

On 08.03.2017 12:17, Jacek Caban wrote: > On 08.03.2017 12:03, Sebastian Lackner wrote: >> +void sha512_init(SHA512_CTX *ctx) >> +{ >> + ctx->len = 0; >> + ctx->h[0] = 0x6a09e667f3bcc908; >> + ctx->h[1] = 0xbb67ae8584caa73b; >> + ctx->h[2] = 0x3c6ef372fe94f82b; >> + ctx->h[3] = 0xa54ff53a5f1d36f1; >> + ctx->h[4] = 0x510e527fade682d1; >> + ctx->h[5] = 0x9b05688c2b3e6c1f; >> + ctx->h[6] = 0x1f83d9abfb41bd6b; >> + ctx->h[7] = 0x5be0cd19137e2179; > > > I think those are still not portable, because those constants are too > large. You may use something like ((ULONG64)0x6a09e667<<32)|0xf3bcc908. > > > Jacek I was wondering about that too, but it looks like we have already various places where 64-bit consts are used without any ugly tricks. Examples are in tests, include files, and also implementation code. Is it really necessary? Or is it only the suffix which causes trouble? Best regards, Sebastian

2 1

Re: [4/5] bcrypt: Add a builtin sha384/sha512 implementation.
by Hans Leidekker March 8, 2017

March 8, 2017

On Tue, 2017-03-07 at 23:16 +0100, Sebastian Lackner wrote: > +void sha384_init(SHA512_CTX *ctx) > +{ > + ctx->len = 0; > + ctx->h[0] = 0xcbbb9d5dc1059ed8ULL; > + ctx->h[1] = 0x629a292a367cd507ULL; > + ctx->h[2] = 0x9159015a3070dd17ULL; > + ctx->h[3] = 0x152fecd8f70e5939ULL; > + ctx->h[4] = 0x67332667ffc00b31ULL; > + ctx->h[5] = 0x8eb44a8768581511ULL; > + ctx->h[6] = 0xdb0c2e0d64f98fa7ULL; > + ctx->h[7] = 0x47b5481dbefa4fa4ULL; We try to avoid the (U)LL suffix. The patch set looks good otherwise.

2 1

Re: [PATCH] d3dx9/tests: Add test for out of bound array selector in effect.
by Matteo Bruni March 7, 2017

March 7, 2017

2017-03-07 18:47 GMT+01:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> Hi Paul! Patch looks mostly good to me BUT I have a couple comments anyway... > --- > dlls/d3dx9_36/tests/effect.c | 159 ++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 141 insertions(+), 18 deletions(-) > > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index b3845c5..fe84173 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -3813,6 +3813,33 @@ static const DWORD test_effect_preshader_effect_blob[] = > #define TEST_EFFECT_PRESHADER_VSHADER_POS 2458 > #define TEST_EFFECT_PRESHADER_VSHADER_LEN 13 > > +static BOOL test_effect_preshader_compare_shader(IDirect3DDevice9 *device, int expected_shader_index) > +{ > + IDirect3DVertexShader9 *vshader; > + void *byte_code; > + unsigned int byte_code_size; > + HRESULT hr; > + BOOL ret; > + > + if (FAILED(hr = IDirect3DDevice9_GetVertexShader(device, &vshader)) || !vshader) > + return FALSE; It seems preferable to keep the detailed ok() calls from the old test instead of a generic ok() message on the return value of test_effect_preshader_compare_shader(). I guess you want to avoid to have ambiguous ok() calls in this helper. Until we get something like the with_context() thing from https://source.winehq.org/patches/data/131137 just passing a string with the test name to the helper function and using that in the ok() messages should do the trick. > + hr = effect->lpVtbl->BeginPass(effect, 1); > + todo_wine ok(hr == E_FAIL, "Got result %#x.\n", hr); > + if (SUCCEEDED(hr)) > + effect->lpVtbl->EndPass(effect); > + > + hr = effect->lpVtbl->BeginPass(effect, 1); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); That reminds me, do we have tests for two BeginPass() calls without an EndPass() in between? Might be interesting to see what happens both with the same or different pass index. Other somewhat interesting cases might be BeginPass() outside of Begin() / End() and multiple Begin() / End() / EndPass() calls in a row. It should probably be in a separate patch though. BTW, I think it's somewhat idiomatic to call it "out of bounds" even when technically there is a single bound involved.

2 3

Interested in Wine Outreachy Application
by Sreya Mittal March 7, 2017

March 7, 2017

Hi, I am Sreya Mittal pursuing my Btech in Computer Science at International Institute of Information Technology, Hyderabad. I am interested in getting started with open-source contributions and looking forward to becoming a part of Wine and Outreachy community. I am proficient in C. This would help in the work. I would be grateful if I could get a few pointers regarding how to get started. Thank you, Sreya

4 5

Re: kernel32: add stub for GetCurrentPackageFullName
by Sebastian Lackner March 7, 2017

March 7, 2017

On 04.03.2017 11:05, Louis Lenders wrote: > --- a/dlls/kernel32/kernel32.spec > +++ b/dlls/kernel32/kernel32.spec > @@ -629,6 +629,7 @@ > @ stdcall GetCurrentDirectoryA(long ptr) > @ stdcall GetCurrentDirectoryW(long ptr) > @ stdcall GetCurrentPackageId(ptr ptr) > +@ stdcall GetCurrentPackageFullName(ptr ptr) > @ stdcall -norelay GetCurrentProcess() > @ stdcall -norelay GetCurrentProcessId() > @ stdcall GetCurrentProcessorNumber() ntdll.NtGetCurrentProcessorNumber In this case its too late, but please make sure to keep the specfile ordered alphabetically in the future.

2 1

Re: [PATCH v2 2/5] winebus.sys: Build device reports for linux event devices
by Roderick Colenbrander March 7, 2017

March 7, 2017

Hi Aric, I have had a quick look over this patch and have some initial feedback, maybe more later if time permits. For the absolute and relative axes don't hard code the minimum and maximum values for the report descriptors. These are not correct for many devices e.g. Xbox controllers use 16-bit absolute axes, while DS3/DS4 for example use 8-bit. In addition xbox controllers use signed data, while DS3/DS4 use unsigned (0-255)Also many devices use signed data. You should rely on the absinfo data returned by EVIOCGABS and base your input ranges on that. Thanks, Roderick On Mon, Feb 27, 2017 at 9:32 AM, Aric Stewart <aric(a)codeweavers.com> wrote: > > v2: Updates from Sebastian Lackner > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_udev.c | 438 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 436 insertions(+), 2 deletions(-) > > > > >

2 2

Re: qcap: Fix compile error when O_CLOEXEC is not defined.
by Alexandre Julliard March 7, 2017

March 7, 2017

Sebastian Lackner <sebastian(a)fds-team.de> writes: > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > > Defining to 0 ensures that it doesn't do any harm. If we don't > care about ancient systems, we can probably also remove the fcntl call. > On modern systems, setting CLOEXEC twice (via open and fcntl) shouldn't > hurt. I'm too lazy to check, but I'm wondering if passing O_CLOEXEC would cause the open to fail on an ancient system, and whether we should be checking for EINVAL and retrying... -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH v2 3/5] winebus.sys: Process device reports for linux event devices
by Roderick Colenbrander March 7, 2017

March 7, 2017

Hi Aric, Some quick feedback on this patch as well. High-level looks okay (need to have a deeper dive), one thing really implement proper support for EV_SYN and particular you MUST handle SYN_DROPPEPD, it has bitten me many times. Basically if you don't process input fast enough (can happen for various reason either you are too slow or device is too fast), evdev drops input and you can't trust the state. When input is dropped SYN_DROPPED is sent and you must follow the required procedures to recover (ignore any input data until next SYN_REPORT). I'm not sure how you are marking the end of input reports, but you may want to mark a report completed based on the proper SYN flag as well else you may build up unneeded input latency. Didn't follow too closely what you are doing yet. Thanks, Roderick On Mon, Feb 27, 2017 at 9:33 AM, Aric Stewart <aric(a)codeweavers.com> wrote: > > v2: Updates from Sebastian Lackner > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_udev.c | 203 ++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 197 insertions(+), 6 deletions(-) > > > > >

1 1

Re: [PATCH 5/5] usp10/tests: Add shaping tests for fLogicalOrder and fRTL.
by Sebastian Lackner March 7, 2017

March 7, 2017

On 02.03.2017 09:38, Henri Verbeet wrote: > Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com> > --- > dlls/usp10/tests/usp10.c | 153 +++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 148 insertions(+), 5 deletions(-) > I'm getting a lot of test failures after this series (starting with patch 2): $ (cd dlls/usp10/tests/; make test) ../../../tools/runtest -q -P wine -T ../../.. -M usp10.dll -p usp10_test.exe.so usp10 && touch usp10.ok fixme:uniscribe:GPOS_convert_design_units_to_device Font with lfWidth set not handled properly fixme:uniscribe:GPOS_convert_design_units_to_device Font with lfWidth set not handled properly usp10.c:1662: Test failed: 0: invalid LogClust(8) usp10.c:1662: Test failed: 1: invalid LogClust(7) usp10.c:1662: Test failed: 2: invalid LogClust(6) usp10.c:1662: Test failed: 3: invalid LogClust(6) usp10.c:1662: Test failed: 4: invalid LogClust(4) usp10.c:1662: Test failed: 5: invalid LogClust(3) usp10.c:1662: Test failed: 6: invalid LogClust(1) usp10.c:1662: Test failed: got incorrect number of glyphs (9) usp10.c:1662: Test failed: 2: uJustification incorrect (2) usp10.c:1662: Test failed: 2: fClusterStart incorrect (0) usp10.c:1662: Test failed: 3: fClusterStart incorrect (1) usp10.c:1662: Test failed: 3: fDiacritic incorrect (0) usp10.c:1662: Test failed: 3: fZeroWidth incorrect (0) usp10.c:1662: Test failed: 5: fClusterStart incorrect (0) usp10.c:1662: Test failed: 5: fDiacritic incorrect (1) usp10.c:1662: Test failed: 5: fZeroWidth incorrect (1) usp10.c:1662: Test failed: 7: uJustification incorrect (0) usp10.c:1662: Test failed: 8: uJustification incorrect (0) usp10.c:1662: Test failed: 8: fClusterStart incorrect (1) usp10.c:1662: Test failed: 8: fDiacritic incorrect (0) usp10.c:1662: Test failed: Got unexpected glyphProp2[6].sva.uJustification 0, expected 0x2. usp10.c:1662: Test failed: Got unexpected glyphProp2[6].sva.fClusterStart 0x1, expected 0. usp10.c:1662: Test failed: Got unexpected glyphProp2[7].sva.uJustification 0x2, expected 0. usp10.c:1662: Test failed: Got unexpected glyphProp2[7].sva.fClusterStart 0, expected 0x1. fixme:uniscribe:GPOS_convert_design_units_to_device Font with lfWidth set not handled properly fixme:uniscribe:get_cluster_advance multi-glyph reversed clusters found fixme:uniscribe:get_cluster_advance multi-glyph reversed clusters found fixme:uniscribe:get_cluster_advance multi-glyph reversed clusters found fixme:uniscribe:get_cluster_advance multi-glyph reversed clusters found fixme:uniscribe:get_cluster_advance multi-glyph reversed clusters found fixme:uniscribe:get_cluster_advance multi-glyph reversed clusters found fixme:uniscribe:get_cluster_advance multi-glyph reversed clusters found fixme:uniscribe:get_cluster_advance multi-glyph reversed clusters found fixme:uniscribe:get_cluster_advance multi-glyph reversed clusters found fixme:uniscribe:GPOS_convert_design_units_to_device Font with lfWidth set not handled properly fixme:uniscribe:GPOS_convert_design_units_to_device Font with lfWidth set not handled properly fixme:uniscribe:SHAPE_GetFontFeatureTags Filtering not implemented Is this a problem with the fonts shipped on Archlinux, or is something going wrong on the Wide side? The font "Estrangelo Edessa" is provided by /usr/share/fonts/OTF/SyrCOMEdessa.otf which can be downloaded at https://www.archlinux.org/packages/extra/any/xorg-fonts-misc/download/. If you need any logs or want me to open a bug report, please let me know. Best regards, Sebastian

2 4

[v5 00/20] x86: Enable User-Mode Instruction Prevention
by Ricardo Neri March 7, 2017

March 7, 2017

This is v5 of this series. The four previous submissions can be found here [1], here [2], here[3], and here [4]. This version addresses the comments received in v4 plus improvements of the handling of emulation in 64-bit builds. Please see details in the change log. === What is UMIP? User-Mode Instruction Prevention (UMIP) is a security feature present in new Intel Processors. If enabled, it prevents the execution of certain instructions if the Current Privilege Level (CPL) is greater than 0. If these instructions were executed while in CPL > 0, user space applications could have access to system-wide settings such as the global and local descriptor tables, the segment selectors to the current task state and the local descriptor table. These are the instructions covered by UMIP: * SGDT - Store Global Descriptor Table * SIDT - Store Interrupt Descriptor Table * SLDT - Store Local Descriptor Table * SMSW - Store Machine Status Word * STR - Store Task Register If any of these instructions is executed with CPL > 0, a general protection exception is issued when UMIP is enabled. === How does it impact applications? There is a caveat, however. Certain applications rely on some of these instructions to function. An example of this are applications that use WineHQ[6]. For instance, these applications rely on sidt returning a non- accessible memory location[6]. During the discussions, it was proposed that the fault could be relied to the user-space and perform the emulation in user-mode. However, this would break existing applications until, for instance, they update to a new WineHQ version. However, this approach would require UMIP to be disabled by default. The consensus in this forum is to always enable it. This patchset initially treated tasks running in virtual-8086 mode as a special case. However, I received clarification that DOSEMU[7] does not support applications that use these instructions. It relies on WineHQ for this [8]. Furthermore, the applications for which the concern was raised run in protected mode [6]. Please note that UMIP is always enabled for both 64-bit and 32-bit Linux builds. However, emulation of the UMIP-protected instructions is not done for 64-bit processes. 64-bit user space applications will receive the SIGSEGV signal when UMIP instructions causes a general protection fault. === How are UMIP-protected instructions emulated? This version keeps UMIP enabled at all times and by default. If a general protection fault caused by the instructions protected by UMIP is detected, such fault will be fixed-up by returning dummy values as follows: * SGDT and SIDT return hard-coded dummy values as the base of the global descriptor and interrupt descriptor tables. These hard-coded values correspond to memory addresses that are near the end of the kernel memory map. This is also the case for virtual-8086 mode tasks. In all my experiments in x86_32, the base of GDT and IDT was always a 4-byte address, even for 16-bit operands. Thus, my emulation code does the same. In all cases, the limit of the table is set to 0. * STR and SLDT return 0 as the segment selector. This looks appropriate since we are providing a dummy value as the base address of the global descriptor table. * SMSW returns the value with which the CR0 register is programmed in head_32/64.S at boot time. This is, the following bits are enabled: CR0.0 for Protection Enable, CR.1 for Monitor Coprocessor, CR.4 for Extension Type, which will always be 1 in recent processors with UMIP; CR.5 for Numeric Error, CR0.16 for Write Protect, CR0.18 for Alignment Mask. As per the Intel 64 and IA-32 Architectures Software Developer's Manual, SMSW returns a 16-bit results for memory operands. However, when the operand is a register, the results can be up to CR0[63:0]. Since the emulation code only kicks-in in x86_32, we return up to CR[31:0]. * The proposed emulation code is handles faults that happens in both protected and virtual-8086 mode. === How is this series laid out? ++ Fix bugs in MPX address evaluator I found very useful the code for Intel MPX (Memory Protection Extensions) used to parse opcodes and the memory locations contained in the general purpose registers when used as operands. I put some of this code in a separate library file that both MPX and UMIP can access and avoid code duplication. Before creating the new library, I fixed a couple of bugs that I found in how MPX determines the address contained in the instruction and operands. ++ Provide a new x86 instruction evaluating library With bugs fixed, the MPX evaluating code is relocated in a new insn-eval.c library. The basic functionality of this library is extended to obtain the segment descriptor selected by either segment override prefixes or the default segment by the involved registers in the calculation of the effective address. It was also extended to obtain the default address and operand sizes as well as the segment base address. Also, support to process 16-bit address encodings. Armed with this arsenal, it is now possible to determine the linear address onto which the emulated results shall be copied. This code supports Normal 32-bit and 64-bit (i.e., __USER32_CS and/or __USER_CS) protected mode, virtual-8086 mode, 16-bit protected mode with 32-bit base address. ++ Emulate UMIP instructions A new fixup_umip_exception functions inspect the instruction at the instruction pointer. If it is an UMIP-protected instruction, it executes the emulation code. This uses all the address-computing code of the previous section. ++ Add self-tests Lastly, self-tests are added to entry_from_v86.c to exercise the most typical use cases of UMIP-protected instructions in a virtual-8086 mode. ++ Extensive tests Extensive tests were performed to test all the combinations of ModRM, SiB and displacements for 16-bit and 32-bit encodings for the ss, ds, es and fs segments. Tests also include a 64-bit program that uses segmentation via FS and GS. For this purpose, I temporarily, and not as part of this patchset, enabled UMIP support for 64-bit process with the intention to test the computations of linear addresses in 64-bit mode, including the extra R8-R15 registers. Extensive test is also implemented for virtual-8086 tasks. Code of these tests can be found here [9] and here [10]. [1]. https://lwn.net/Articles/705877/ [2]. https://lkml.org/lkml/2016/12/23/265 [3]. https://lkml.org/lkml/2017/1/25/622 [4]. https://lkml.org/lkml/2017/2/23/40 [5]. https://www.winehq.org/ [6]. https://www.winehq.org/pipermail/wine-devel/2016-November/115320.html [7]. http://www.dosemu.org/ [8]. http://marc.info/?l=linux-kernel&m=147876798717927&w=2 [9]. https://github.com/01org/luv-yocto/tree/rneri/umip/meta-luv/recipes-core/um… [10]. https://github.com/01org/luv-yocto/commit/a72a7fe7d68693c0f4100ad86de6ecabd… Thanks and BR, Ricardo Changes since V4: * Audited patches to use braces in all the branches of conditional. statements, except those in which the conditional action only takes one line. * Implemented support in 64-builds for both 32-bit and 64-bit tasks in the instruction evaluating library. * Split segment selector function in the instruction evaluating library into two functions to resolve the segment type by instruction override or default and a separate function to actually read the segment selector. * Fixed a bug when evaluating 32-bit effective addresses with 64-bit kernels. * Split patches further for for easier review. * Use signed variables for computation of effective address. * Fixed issue with a spurious static modifier in function insn_get_addr_ref found by kbuild test bot. * Removed comparison between true and fixup_umip_exception. * Reworked check logic when identifying erroneous vs invalid values of the SiB base and index. Changes since V3: * Limited emulation to 32-bit and 16-bit modes. For 64-bit mode, a general protection fault is still issued when UMIP-protected instructions are executed with CPL > 0. * Expanded instruction-evaluating code to obtain segment descriptor along with their attributes such as base address and default address and operand sizes. Also, support for 16-bit encodings in protected mode was implemented. * When getting a segment descriptor, this include support to obtain those of a local descriptor table. * Now the instruction-evaluating code returns -EDOM when the value of registers should not be used in calculating the effective address. The value -EINVAL is left for errors. * Incorporate the value of the segment base address in the computation of linear addresses. * Renamed new instruction evaluation library from insn-kernel.c to insn-eval.c * Exported functions insn_get_reg_offset_* to obtain the register offset by ModRM r/m, SiB base and SiB index. * Improved documentation of functions. * Split patches further for easier review. Changes since V2: * Added new utility functions to decode the memory addresses contained in registers when the 16-bit addressing encodings are used. This includes code to obtain and compute memory addresses using segment selectors for real-mode address translation. * Added support to emulate UMIP-protected instructions for virtual-8086 tasks. * Added self-tests for virtual-8086 mode that contains representative use cases: address represented as a displacement, address in registers and registers as operands. * Instead of maintaining a static variable for the dummy base addresses of the IDT and GDT, a hard-coded value is used. * The emulated SMSW instructions now return the value with which the CR0 register is programmed in head_32/64.S This is: PE | MP | ET | NE | WP | AM. For x86_64, PG is also enabled. * The new file arch/x86/lib/insn-utils.c is now renamed as arch/x86/lib/ insn-kernel.c. It also has its own header. This helps keep in sync the the kernel and objtool instruction decoders. Also, the new insn-kernel.c contains utility functions that are only relevant in a kernel context. * Removed printed warnings for errors that occur when decoding instructions with invalid operands. * Added more comments on fixes in the instruction-decoding MPX functions. * Now user_64bit_mode(regs) is used instead of test_thread_flag(TIF_IA32) to determine if the task is 32-bit or 64-bit. * Found and fixed a bug in insn-decoder in which X86_MODRM_RM was incorrectly used to obtain the mod part of the ModRM byte. * Added more explanatory code in emulation and instruction decoding code. This includes a comment regarding that copy_from_user could fail if there exists a memory protection key in place. * Tested code with CONFIG_X86_DECODER_SELFTEST=y and everything passes now. * Prefixed get_reg_offset_rm with insn_ as this function is exposed via a header file. For clarity, this function was added in a separate patch. Changes since V1: * Virtual-8086 mode tasks are not treated in a special manner. All code for this purpose was removed. * Instead of attempting to disable UMIP during a context switch or when entering virtual-8086 mode, UMIP remains enabled all the time. General protection faults that occur are fixed-up by returning dummy values as detailed above. * Removed umip= kernel parameter in favor of using clearcpuid=514 to disable UMIP. * Removed selftests designed to detect the absence of SIGSEGV signals when running in virtual-8086 mode. * Reused code from MPX to decode instructions operands. For this purpose code was put in a common location. * Fixed two bugs in MPX code that decodes operands. Thanks and BR, Ricardo Ricardo Neri (20): x86/mpx: Use signed variables to compute effective addresses x86/mpx: Do not use SIB index if index points to R/ESP x86/mpx: Do not use R/EBP as base in the SIB byte with Mod = 0 x86/mpx, x86/insn: Relocate insn util functions to a new insn-kernel x86/insn-eval: Add utility functions to get register offsets x86/insn-eval: Add utility functions to get segment selector x86/insn-eval: Add utility function to get segment descriptor x86/insn-eval: Add utility function to get segment descriptor base address x86/insn-eval: Add functions to get default operand and address sizes x86/insn-eval: Do not use R/EBP as base if mod in ModRM is zero insn/eval: Incorporate segment base in address computation x86/insn: Support both signed 32-bit and 64-bit effective addresses x86/insn-eval: Add support to resolve 16-bit addressing encodings x86/insn-eval: Add wrapper function for 16-bit and 32-bit address encodings x86/cpufeature: Add User-Mode Instruction Prevention definitions x86: Add emulation code for UMIP instructions x86/umip: Force a page fault when unable to copy emulated result to user x86/traps: Fixup general protection faults caused by UMIP x86: Enable User-Mode Instruction Prevention selftests/x86: Add tests for User-Mode Instruction Prevention arch/x86/Kconfig | 10 + arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/disabled-features.h | 8 +- arch/x86/include/asm/insn-eval.h | 23 + arch/x86/include/asm/umip.h | 15 + arch/x86/include/uapi/asm/processor-flags.h | 2 + arch/x86/kernel/Makefile | 1 + arch/x86/kernel/cpu/common.c | 16 +- arch/x86/kernel/traps.c | 4 + arch/x86/kernel/umip.c | 305 ++++++++++ arch/x86/lib/Makefile | 2 +- arch/x86/lib/insn-eval.c | 832 ++++++++++++++++++++++++++ arch/x86/mm/mpx.c | 120 +--- tools/testing/selftests/x86/entry_from_vm86.c | 39 +- 14 files changed, 1256 insertions(+), 122 deletions(-) create mode 100644 arch/x86/include/asm/insn-eval.h create mode 100644 arch/x86/include/asm/umip.h create mode 100644 arch/x86/kernel/umip.c create mode 100644 arch/x86/lib/insn-eval.c -- 2.9.3

2 22

Re: [PATCH] makedep: touch the .tab.c file to ensure ordering
by Alexandre Julliard March 6, 2017

March 6, 2017

Marcus Meissner <marcus(a)jet.franken.de> writes: > .tab.c and .tab.h are created too close together to confuse > make. Ensure a bit better ordering by touching mcy.tab.c I don't think that's better. It shouldn't make any difference, except maybe making it a little harder to reproduce. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: Reproducible builds
by wylda＠volny.cz March 6, 2017

March 6, 2017

> Od: André Hentschel <nerv(a)dawncrow.de> >>>>>> Am 2017-02-19 um 10:00 schrieb wylda(a)volny.cz: >>>>>>> Anyway, there are 7 files out of 2750 which _always_ pickup some random >>>>>>> value and use that consistently. Why are these libraries built >>>>>>> inconsitently? Is that intentional? An example is attached. >>>>> >>>>> Thanks. These "random" value come from: >>>>> >>>>> * olepro_t.res >>>>> * std_ole_v2_t.res >>>>> * wuapi_tlb_t.res >>> >>> When i do: >>> >>> export WIDL_TIME_OVERRIDE="0" >>> rm dlls/olepro32/olepro_t.res >>> make -k dlls/olepro32 >>> md5sum -b dlls/olepro32/olepro_t.res >>> >>> then i always get different MD5 checksum. >>> >>> Marcus, could you send me your dlls/olepro32/Makefile please? >>> What is your distro? >> >> I tried above for both 32bit and 64bit directories, always gives the same md5 in those. >> >> openSUSE Tumbleweed, with currently gcc 6.3.1+r245113, binutils-2.27, flex-2.6.1, bison-3.0.4 >> > >I can reproduce the bug with the instructions above >Linux Mint 18.1, gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.4), binutils 2.26.1, flex 2.6, biso 3.0.4 >the first bad value is at offset 269h >Which is datatype2 from MSFT_TypeInfoBase > >So initializing datatype2 in add_typedef_typeinfo or the following patch fixes the problem for me: > >diff --git a/tools/widl/write_msft.c b/tools/widl/write_msft.c >index 137bb2d..7904e45 100644 >--- a/tools/widl/write_msft.c >+++ b/tools/widl/write_msft.c >@@ -798,6 +798,7 @@ static int encode_type( > if (!alignment) alignment = &scratch; > if (!decoded_size) decoded_size = &scratch; > >+ *decoded_size = 0; > > switch (vt) { > case VT_I1: > Thank Stefan / Marcus / Andre. This fixed 6 files. 7th file is libdinput.a, which also has some random stuff inside. This was for 32bit chroot. In 64bit chroot these 6 files are also fixed now. libdinput.a differs again. But 64bit chroot also shows 2 other files: * /bin/wmc * /lib64/wine/msi.dll.so These last two differs in Build-id's SHA1 of the .note.gnu.build-id section. Debian Stretch 9.0 Multiarch: * gcc (Debian 6.3.0-6) 6.3.0 20170205 * binutils 2.27.90.20170124-2 * flex 2.6.1-1.3 * bison 3.0.4.dfsg-1 Regards, W.

3 5

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 6, 2017

March 6, 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 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=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 March 1, 2017

March 1, 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