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

Re: [PATCH 3/6] d3d9: Validate adapter format in CheckDeviceFormat().

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=31280 Your paranoid android. === wvistau64 (32 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4. === w2008s64 (32 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4. === w7u (32 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4. === w7pro64 (32 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4. === w8 (32 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4. === w864 (32 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4. === w1064 (32 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4. === wvistau64 (64 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4. === w2008s64 (64 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4. === w7pro64 (64 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4. === w864 (64 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4. === w1064 (64 bit device) === device.c:11206: Test failed: Got unexpected hr 0, device type 0x4. device.c:11209: Test failed: Got unexpected hr 0, device type 0x4. device.c:11212: Test failed: Got unexpected hr 0, device type 0x4. device.c:11215: Test failed: Got unexpected hr 0, device type 0x4.

May 31, 2017

Re: [PATCH 2/6] d3d8: Validate adapter format in CheckDeviceFormat().

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=31279 Your paranoid android. === w8 (32 bit device) === device.c:2945: Test failed: Received WM_WINDOWPOSCHANGED but did not expect it.

May 31, 2017

Re: [PATCH 2/7] msvcrt: Clean up registered C++ object in handler.

by Piotr Caban

On 05/31/17 00:59, Daniel Lehman wrote: > > On 05/24/17 02:55, Daniel Lehman wrote: >> +static DWORD cxx_catch_cleanup(EXCEPTION_RECORD *rec, EXCEPTION_REGISTRATION_RECORD *frame, >> + CONTEXT *context, >> +EXCEPTION_REGISTRATION_RECORD **pdispatcher) { >> + if (rec->ExceptionFlags & (EH_UNWINDING | EH_EXIT_UNWIND)) >> + { >> + thread_data_t *data = msvcrt_get_thread_data(); >> + frame_info *cur; >> + >> + if (cxx_is_consolidate(rec)) >> Is this condition really needed? Shouldn't we clean the object no matter what's the reason of unwind? > > Yeah. That's covered by patch 7/7. The original code only cleaned up if consolidating. > > Since this 2/7 patch was somewhat of a refactoring for the later patches, I kept the consolidate-only cleanup logic. I can merge them if you want Yes, it would be preferable to merge the patches. >>> + if ((ULONG64)cur <= (ULONG64)frame) >> This condition is not working. It's making assumption about order of catch_frame and frame_info variables on stack while they are declared this way: > > I see what you mean. If I forcefully reverse them, my tests crash; my version of gcc was always putting them in the same place on the stack, regardless of where they were declared > >>> + EXCEPTION_REGISTRATION_RECORD catch_frame; >>> cxx_frame_info frame_info; >> Shouldn't the cxx_catch_cleanup just unregister the object that was registered in call_catch_block > > I didn't find a way to be call __CxxUnregisterObject on that one specifically (I'll try suggestions if you got em) I think that the easiest solution would be to define something like: struct catch_cleanup_frame { EXCEPTION_REGISTRATION_RECORD frame; cxx_frame_info frame_info; } and just call __CxxUnregisterObject with passed catch_cleanup_frame->frame_info in handler. >> Here's a test case that demonstrate the problem with cur <= frame >> comparison: >> >> try { >> try { int *p = NULL; *p = 0x42; } >> catch (klass x) { throw 1; } >> } catch (int i) { } >> >> try { throw 1; } >> catch(...) {} > > This crashes for me even on Windows because the SEGV is uncaught. It 'works' if I set an seh translator that throws int, but I get identical results on Wine with my series applied. Do I need to add something? It was meant to be run with seh translator set. It might be compiler/optimization level specific. On my computer it leads to incorrect stack usage caused by "cur <= frame" condition. The __CxxUnregisterObject is not called in my case and the registered objects list points to invalid stack space. Thanks, Piotr

May 31, 2017

Re: [v2 PATCH 2/3] mfplat: Implement MFCreateMediaType

by Henri Verbeet

On 31 May 2017 at 04:47, Nikolay Sivov <bunglehead(a)gmail.com> wrote: > On 31.05.2017 4:09, Alistair Leslie-Hughes wrote: >> +typedef struct _mfmedaitype >> +{ >> + IMFMediaType IMFMediaType_iface; >> + LONG ref; >> +} mfmedaitype; > > You got a typo in structure name. > If everyone is going to comment, I might as well join in. What is the purpose of the typedef?

May 31, 2017

Re: [PATCH 7/7] regedit: Do not get text for items with an index less than one

by Hugh McMaster

On Wednesday, 31 May 2017 3:55 PM, Nikolay Sivov wrote: > 'item' is unsigned, so '< 1' and '== 0' should be equivalent. Yes, they are. But -1 gets wrapped into a positive number, which means the proposed patch doesn't work anyway. We'd have to change the UINT to int, but it's not worth it.

May 31, 2017

Re: [PATCH 7/7] regedit: Do not get text for items with an index less than one

by Nikolay Sivov

On 31.05.2017 8:49, Hugh McMaster wrote: > On Wednesday, 31 May 2017 5:23 AM, Alexandre Julliard wrote: >> Hugh McMaster writes: >> >>> @@ -59,7 +59,7 @@ LPWSTR GetItemText(HWND hwndLV, UINT item) >>> { >>> LPWSTR newStr, curStr; >>> unsigned int maxLen = 128; >>> - if (item == 0) return NULL; /* first item is ALWAYS a default */ >>> + if (item < 1) return NULL; /* first item is ALWAYS a default */ >> >> How is this different? > > Some of the listview API functions return -1 on failure. If we > were to pass in -1 as the item integer to GetItemText(), the function > proceeds to allocate memory before attempting to get text for that > non-existent item. > > That memory is not freed in the function and needs to be freed by > the caller. > > We don't currently use the function in this way, but it is quite easy > to do so without realizing. So I thought it would be easier to prevent any > such use of the function. > > The commit message probably didn't explain this well at all. > > I can resend if you feel this is needed, otherwise I'm happy to let it go. > 'item' is unsigned, so '< 1' and '== 0' should be equivalent.

May 31, 2017

Re: [PATCH 7/7] regedit: Do not get text for items with an index less than one

by Hugh McMaster

On Wednesday, 31 May 2017 5:23 AM, Alexandre Julliard wrote: > Hugh McMaster writes: > >> @@ -59,7 +59,7 @@ LPWSTR GetItemText(HWND hwndLV, UINT item) >> { >> LPWSTR newStr, curStr; >> unsigned int maxLen = 128; >> - if (item == 0) return NULL; /* first item is ALWAYS a default */ >> + if (item < 1) return NULL; /* first item is ALWAYS a default */ > > How is this different? Some of the listview API functions return -1 on failure. If we were to pass in -1 as the item integer to GetItemText(), the function proceeds to allocate memory before attempting to get text for that non-existent item. That memory is not freed in the function and needs to be freed by the caller. We don't currently use the function in this way, but it is quite easy to do so without realizing. So I thought it would be easier to prevent any such use of the function. The commit message probably didn't explain this well at all. I can resend if you feel this is needed, otherwise I'm happy to let it go.

May 31, 2017

Re: [v2 PATCH 2/3] mfplat: Implement MFCreateMediaType

by Nikolay Sivov

On 31.05.2017 4:09, Alistair Leslie-Hughes wrote: > +typedef struct _mfmedaitype > +{ > + IMFMediaType IMFMediaType_iface; > + LONG ref; > +} mfmedaitype; You got a typo in structure name.

May 31, 2017

Re: [PATCH 2/3] mfplat: Implement MFCreateMediaType

by Dmitry Timoshkov

Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > +struct IMFMediaTypeVtbl mediatype_vtbl = > +{ It should be 'static const', and without 'struct'. -- Dmitry.

May 30, 2017

RE: [PATCH 2/7] msvcrt: Clean up registered C++ object in handler.

by Daniel Lehman

On 05/24/17 02:55, Daniel Lehman wrote: > +static DWORD cxx_catch_cleanup(EXCEPTION_RECORD *rec, EXCEPTION_REGISTRATION_RECORD *frame, > + CONTEXT *context, > +EXCEPTION_REGISTRATION_RECORD **pdispatcher) { > + if (rec->ExceptionFlags & (EH_UNWINDING | EH_EXIT_UNWIND)) > + { > + thread_data_t *data = msvcrt_get_thread_data(); > + frame_info *cur; > + > + if (cxx_is_consolidate(rec)) > Is this condition really needed? Shouldn't we clean the object no matter what's the reason of unwind? Yeah. That's covered by patch 7/7. The original code only cleaned up if consolidating. Since this 2/7 patch was somewhat of a refactoring for the later patches, I kept the consolidate-only cleanup logic. I can merge them if you want >> + if ((ULONG64)cur <= (ULONG64)frame) > This condition is not working. It's making assumption about order of catch_frame and frame_info variables on stack while they are declared this way: I see what you mean. If I forcefully reverse them, my tests crash; my version of gcc was always putting them in the same place on the stack, regardless of where they were declared >> + EXCEPTION_REGISTRATION_RECORD catch_frame; >> cxx_frame_info frame_info; > Shouldn't the cxx_catch_cleanup just unregister the object that was registered in call_catch_block? I didn't find a way to be call __CxxUnregisterObject on that one specifically (I'll try suggestions if you got em) but the loop in cxx_catch_cleanup effective does this. when unwinding a nested exception, cxx_catch_cleanup will be called when unwinding call_catch_block, so all objects registered in call_catch_block and below will be cleaned up at that point. Unless the catch block itself explicitly calls __CxxRegisterObject, the only object on the list up to that frame will be the one from call_catch_block > Here's a test case that demonstrate the problem with cur <= frame > comparison: > > try { > try { int *p = NULL; *p = 0x42; } > catch (klass x) { throw 1; } > } catch (int i) { } > > try { throw 1; } > catch(...) {} This crashes for me even on Windows because the SEGV is uncaught. It 'works' if I set an seh translator that throws int, but I get identical results on Wine with my series applied. Do I need to add something? Thanks daniel

May 30, 2017