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