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

Re: [v2] msvcrt: handle of thread created by _beginthread should not be closed on thread detach.
by Piotr Caban Feb. 1, 2016

Feb. 1, 2016

Hi Paul, On 01/31/16 22:58, Paul Gofman wrote: > + /* _beginthread: handle is not closed on ExitProcess and _endthreadex */ > + hThread = (HANDLE)_beginthread(test_thread_func, 0, (void*)0); > + Sleep(150); > + ret = WaitForSingleObject(hThread, INFINITE); > + ok(ret == WAIT_FAILED, "ret = %d\n", ret); This test may fail if for some reason thread has not finished during 150ms sleep. You can workaround this issue following way: hThread = (HANDLE)_beginthread(test_thread_func, 0, (void*)0); ok(hThread != INVALID_HANDLE_VALUE, "_beginthread failed (%d)\n", errno); WaitForSingleObject(hThread, INFINITE); ret = CloseHandle(hThread); ok(!ret, "ret = %d\n", ret); > @@ -69,6 +69,9 @@ static DWORD CALLBACK _beginthread_trampoline(LPVOID arg) > MSVCRT_free(arg); > > local_trampoline.start_address(local_trampoline.arglist); > + > + CloseHandle(data->handle); > + data->handle = NULL; According to MSDN _endthread should be called here. Because it's documented I think we should implement it in similar way. > @@ -136,8 +139,18 @@ MSVCRT_uintptr_t CDECL _beginthreadex( > */ > void CDECL _endthread(void) > { > + thread_data_t *tls; > + > TRACE("(void)\n"); > > + tls = TlsGetValue(msvcrt_tls_index); > + if (tls && tls->handle) You should check for INVALID_HANDLE_VALUE instead of NULL here. It's doesn't really matter but for sake of consistency tls->handle should be set to INVALID_HANDLE_VALUE after close. Thanks, Piotr

2 1

Re: [PATCH] server: Implement serial port read interval timeouts.
by Alexandre Julliard Feb. 1, 2016

Feb. 1, 2016

Alex Henrie <alexhenrie24(a)gmail.com> writes: > Fixes https://bugs.winehq.org/show_bug.cgi?id=39875 > > There is a test program attached to the bug report. > > Version 3 has all the logic in serial.c, even though this required > adding a getter in async.c and another getter in fd.c. The first > interval timeout no longer trashes subsequent interval timeouts or > the total timeout, and absolute timeouts are used exclusively. You should be able to do this without adding all the extra getter functions. I'd suggest something along these lines (completely untested): diff --git a/server/serial.c b/server/serial.c index 164a4b1fe717..04b517dd347e 100644 --- a/server/serial.c +++ b/server/serial.c @@ -62,12 +62,15 @@ static void serial_destroy(struct object *obj); static enum server_fd_type serial_get_fd_type( struct fd *fd ); static void serial_queue_async( struct fd *fd, const async_data_t *data, int type, int count ); +static void serial_reselect_async( struct fd *fd, struct async_queue *queue ); struct serial { struct object obj; struct fd *fd; + struct timeout_user *read_timer; + /* timeout values */ unsigned int readinterval; unsigned int readconst; @@ -115,7 +118,7 @@ static const struct fd_ops serial_fd_ops = no_fd_flush, /* flush */ default_fd_ioctl, /* ioctl */ serial_queue_async, /* queue_async */ - default_fd_reselect_async, /* reselect_async */ + serial_reselect_async, /* reselect_async */ default_fd_cancel_async /* cancel_async */ }; @@ -134,6 +137,7 @@ struct object *create_serial( struct fd *fd ) if (!(serial = alloc_object( &serial_ops ))) return NULL; + serial->read_timer = NULL; serial->readinterval = 0; serial->readmult = 0; serial->readconst = 0; @@ -157,6 +161,7 @@ static struct fd *serial_get_fd( struct object *obj ) static void serial_destroy( struct object *obj) { struct serial *serial = (struct serial *)obj; + if (serial->read_timer) remove_timeout_user( serial->read_timer ); release_object( serial->fd ); } @@ -203,6 +208,34 @@ static void serial_queue_async( struct fd *fd, const async_data_t *data, int typ } } +static void serial_read_timeout( void *arg ) +{ + struct serial *serial = arg; + + serial->read_timer = NULL; + fd_async_wake_up( serial->fd, ASYNC_TYPE_READ, STATUS_TIMEOUT ); +} + +static void serial_reselect_async( struct fd *fd, struct async_queue *queue ) +{ + struct serial *serial = get_fd_user( fd ); + + if (serial->read_timer) + { + if (!(default_fd_get_poll_events( fd ) & POLLIN)) + { + remove_timeout_user( serial->read_timer ); + serial->read_timer = NULL; + } + } + else if (!serial->read_timer && serial->readinterval && (default_fd_get_poll_events( fd ) & POLLIN)) + { + serial->read_timer = add_timeout_user( (timeout_t)serial->readinterval * -10000, + serial_read_timeout, serial ); + } + default_fd_reselect_async( fd, queue ); +} + DECL_HANDLER(get_serial_info) { struct serial *serial; -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 3/3] user32: A window may be notified with WM_CAPTURECHANGED about itself gaining mouse capture if SetCapture() i
by Marvin Feb. 1, 2016

Feb. 1, 2016

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=20123 Your paranoid android. === w1064 (32 bit msg) === msg.c:6290: Test failed: Update region shouldn't be empty msg.c:6524: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6526: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6296: Test failed: Regions are different msg.c:6296: Test failed: Regions are different msg.c:6296: Test failed: Regions are different === w1064 (64 bit msg) === msg.c:7246: Test failed: destroy child on thread exit: 3: the msg sequence is not complete: expected 0000 - actual 001a

1 0

Re: [PATCH] user32: Allow WM_CAPTURECHANGED indicating SetCapture() parameter
by Ken Thomases Feb. 1, 2016

Feb. 1, 2016

On Jan 31, 2016, at 7:18 PM, Christopher Thielen <cthielen(a)gmail.com> wrote: > > A window may be notified with WM_CAPTURECHANGED about itself > gaining mouse capture if it calls SetCapture() twice. > > Tested on Windows XP and Fedora Workstation 22. > > Revised to simplify based on wine-devel feedback. > Revised to address any side-effects of new WM_CAPTURECHANGED > messages. The changes to comctl32 should be in a separate patch. In general, changes to different DLLs should be in separate patches unless it can't be avoided. The comctl32 patch should come before the change to when WM_CAPTURECHANGED is sent so that things aren't broken part-way through the patch series. You might also put the button.c changes in a separate patch, too. > case WM_CAPTURECHANGED: > - return TOOLBAR_CaptureChanged(infoPtr); > + if(hwnd != lParam) > + { > + return TOOLBAR_CaptureChanged(infoPtr); > + } > + else > + { > + return 0; > + } The second return can be hoisted out of the else since the true branch can't fall through. Also, you can reverse the logic, so the code can look like: case WM_CAPTURECHANGED: if(hwnd == lParam) return 0; return TOOLBAR_CaptureChanged(infoPtr); I think that's clearer and also a smaller diff. Same for the other files. Cheers, Ken

2 1

Re: [PATCH] user32: Allow WM_CAPTURECHANGED indicating SetCapture() parameter
by Marvin Feb. 1, 2016

Feb. 1, 2016

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=20122 Your paranoid android. === w1064 (32 bit msg) === msg.c:6290: Test failed: Update region shouldn't be empty msg.c:6524: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6526: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6296: Test failed: Regions are different msg.c:6296: Test failed: Regions are different msg.c:6296: Test failed: Regions are different

1 0