December 2014 - Wine-Devel - list.winehq.org

Re: [PATCH] msvcr120: Implement more C99 math functions.
by Piotr Caban Dec. 8, 2014

Dec. 8, 2014

On 12/06/14 17:07, Martin Storsjo wrote: > +/********************************************************************* > + * cbrt (MSVCR120.@) > + */ > +double CDECL MSVCR120_cbrt(double x) > +{ > +#ifdef HAVE_CBRT > + return cbrt(x); > +#else > + return x < 0 ? -powf(-x, 1.0 / 3.0) : powf(x, 1.0 / 3.0); > +#endif Please use pow instead of powf here. Thanks, Piotr

2 1

Re: [PATCH 05/10] combase: Add stub dll.
by Sebastian Lackner Dec. 8, 2014

Dec. 8, 2014

Hi Martin, a few things where I am not sure about: [PATCH 05/10] [...] > +@ stub COMBASE_63 > +@ stub COMBASE_64 > +@ stub COMBASE_65 > +@ stub COMBASE_66 > +@ stub COMBASE_67 > +@ stub COMBASE_68 [...] Are these really named exports, or exports by ordinal? If they are by ordinal and have no name assigned they are probably internal, and its not worth exporting them in Wine. I also noticed that combase.dll exports a lot of functions (Co*), which in Wine are implemented in ole32. It probably makes sense to sync that, but that can also be added in a later patch in my opinion. I don't think make_specfiles already supports such a situation. [PATCH 06/10 and also following patches] > +/*********************************************************************** > + * RoInitialize (api-ms-win-core-winrt-l1-1-0.@) > + */ Those comments should probably be changed. [PATCH 07/10] Does creating an empty string really return a HSTRING or just NULL (which seems to behave identical)? It probably makes sense to add a test for that. Does WindowsCreateString or WindowsCreateStringReference strip terminating NULL chars? (Embedded NULL chars are allowed obviously, but it might be possible that trailing NULL chars are handled differently). nsivov on #winehackers also suggested that it might be worth trying to find out what the structure is on Windows, but not sure if its worth the effort. If we're not trying at all to be compatible, we could get rid of one of the two memory allocations by allocating (sizeof(hstring_private) + length) as a single chunk, and setting ->buffer according. [PATCH 09/10] I would also suggest adding tests for the content of the buffer. You can use for example memcmp(...) to verify that the content is correct. [PATCH 10/10] Is the buffer initialized? Are terminating NULLs stripped from the buffer? Regards, Sebastian

2 2

Re: [PATCH 3/4] api-ms-win-core-winrt-string: Implement a few HSTRING functions.
by Sebastian Lackner Dec. 7, 2014

Dec. 7, 2014

Hi Martin, most of the patch looks okay, but I think there are a few things you should (or could) change. For patch 3/4: * I would split it up in several patches, and after adding the minimally necessary functions always combine the implementation with the corresponding tests. The huge number of functions in a single patch makes it difficult to review. * Include files should also be added as a separate patch. At the moment the formatting looks a bit ugly (random number of linebreaks?). I would also suggest to rename the function arguments to make sure they don't look exactly like MSDN. * Rename function arguments in the implementation itself, to avoid that they look identical to MSDN. * Internal helper functions (like alloc_string) do not necessarily need to return a HRESULT return value, when there are only two possible results. I would suggest using BOOL instead. * In the specfile use 'wstr' for arguments which are widechar strings. This allows to print them with the +relay debug channel. * Not sure how strict Wine is about the coding style, but generally a slighly different style is preferred for new code. Instead of --- snip --- if (...) { .... } --- snip --- the preferred version is --- snip --- if (...) { ... } --- snip --- The same applies also for for and do-while loops. Moreover cast operations like "(HSTRING)string" should usually have no space inbetween of the type and variable name. For patch 4/4: * C++-Style comments like "// ..." are not allowed in Wine source, instead use "/* ... */" for single-line comments or --- snip --- /* ... * ... */ --- snip --- for multi-line comments. * To avoid crashes on machines where this library is not available, it is recommended to skip the tests in such a case. For example: --- snip --- hmod = LoadLibraryA(...); if (!hmod) { win_skip("Failed to load api-ms-win-core-winrt-string-l1-1-0, skipping tests\n"); return; } --- snip --- * It is okay to use define macros for loading function pointers, but they should be defined directly before, and undefined immediately afterwards, to avoid collision with other definitions. I also don't think that checking for != NULL is really necessary, these api-* dlls have a well-defined set of export functions, so it is basically guaranteed that all functions are available. * check_string should also show the line number because it is used very often. The general way to do that is: --- snip --- #define check_string(str, length, has_null) _check_string(__LINE__, str, length, has_null) static void check_string(int line, HSTRING str, UINT32 length, BOOL has_null) { ... ok_(__FILE__, line)(condition, ...); ... } --- snip --- This allows to determine much easier where something goes wrong. * I would also suggest to split the tests. There is no need to put everything in a single function, especially because api-*-string only implements string functions. ;) I would prefer something like: --- snip --- START_TEST(...) { if (!init_functions()) return; test_CreateString(); test_DuplicateString(); ... } --- snip --- (Of course this doesn't mean that WindowsCreateString can only be used in one function.) Regards, Sebastian

2 4

Re: [PATCH 4/4] api-ms-win-core-winrt-string-l1-1-0: Add tests.
by Marvin Dec. 7, 2014

Dec. 7, 2014

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=10659 Your paranoid android. === build (build) === Can't determine base name

1 0

Re: [PATCH 15/15] api-ms-win-core-winrt-string: Implement a few HSTRING functions.
by Sebastian Lackner Dec. 6, 2014

Dec. 6, 2014

On 01.12.2014 11:27, Martin Storsjo wrote: > + > + buf = malloc(sizeof(*buf)); > + if (!buf) > + return E_OUTOFMEMORY; > + buf->buffer = malloc((length + 1) * sizeof(*buf->buffer)); > + if (!buf->buffer) { > + free(buf); > + return E_OUTOFMEMORY; > + } Some of your previous patches also contain changes to ./configure which is an autogenerated file. Afaik its not allowed to include that when submitting patches. Feedback for this part: * HSTRING and HSTRING_HEADER should probably be defined in some include fine. To ensure that the size of private structure matches you could also use C_ASSERT(...), and maybe even disable align of the structure fields. * You should avoid using malloc(...) in Wine source * http://msdn.microsoft.com/en-us/library/br224632(v=vs.85).aspx mentions: """Calling WindowsDeleteString decrements the reference count of the backing buffer, and if the reference count reaches 0, the Windows Runtime de-allocates the buffer.""" I don't see the concept of refcounted backing buffers anywhere in your code, which is most likely wrong. Its difficult to fix that later. * I would suggest starting with some stub functions, and then add tests before sending the actual implementation. BTW: Michael Müller was also working on some api-ms-win-core-* stubs, but I think he is also fine when your patches get upstream instead: https://github.com/wine-compholio/wine-staging/tree/master/patches/api-ms-w… Regards, Sebastian

3 9

Please help spread the word about the Software Freedom Conservancy
by Jeremy White Dec. 5, 2014

Dec. 5, 2014

Folks, Tony at the Conservancy reached out to us to ask our help in spreading the word about Conservancy. The Conservancy is our sponsoring organization, and they have been a great friend to Wine through the years. As they requested, I'm sending an email blast to our mailing list to spread the word. I'd ask you to scan their email and if you can help them in any of the ways they ask, please help them. They're good people, and good for Wine. Thanks, Jeremy -------- Forwarded Message -------- Subject: Conservancy projects: help us launch the Official Conservancy Supporter program! Date: Wed, 03 Dec 2014 17:17:47 -0500 From: Tony Sebro Hi, all: We at Conservancy are committed to doing whatever we can to support our member projects, and to help you produce great, freely-licensed software for the public's benefit. At this time, we could use the support of you, our member projects, as well. Today, Conservancy launched the official Conservancy Supporter program (http://sfconservancy.org/supporter/) This is our first campaign to build awareness about Conservancy, and our first attempt to really engage the public in a fundraising campaign. While we're glad to help you organize project-specific revenue-generating activities (e.g., project fundraisers, conferences, etc.), we've kept the percentage of revenue we receive from member projects under our FSA so low that it doesn't even pay for a single Conservancy staff member. So, we need our member projects' help to get the word out about who we are, what we do, and how friends of free and open source software can become Conservancy Supporters. Conservancy Supporters will receive: - an "Official Conservancy Supporter" badge for display on their personal websites; - a printable "Official Conservancy Supporter" card; and - a snazzy Conservancy T-Shirt. _ Ways__you can help spread the word_: - Follow @Conservancy on Twitter and/or Identi.ca - Tweet/Dent/Microblog about the Conservancy Supporter program on your social media platform of choice, with a link to the campaign page (please use the #ConservancySupporter hashtag) - If your project has an official social media account, please use that to microblog as well - Send email blasts to your project's mailing lists - Write a blog post or a testimonial about Conservancy (we'd really appreciate it) We thank you in advance for your support -- especially during this giving time of the year! Best, -Tony

1 0

Re: ws2_32/tests: Add SO_BSP_STATE tests
by Marvin Dec. 5, 2014

Dec. 5, 2014

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=10633 Your paranoid android. === wxppro (32 bit sock) === sock.c:610: Test failed: oob_server (190): unexpectedly at the OOB mark: 0

1 0

Re: [PATCH 9/9] wined3d: Pass WM_SYSCOMMAND(SC_RESTORE) to DefWindowProc.
by Marvin Dec. 5, 2014

Dec. 5, 2014

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=10625 Your paranoid android. === w8 (32 bit device) === device.c:1222: Test failed: TestCooperativeLevel failed, hr 0x88760868. device.c:1267: Test failed: Reset failed, hr 0x88760868. device.c:1269: Test failed: TestCooperativeLevel failed, hr 0x88760869. device.c:1273: Test failed: Got unexpected value 0 for D3DRS_LIGHTING. device.c:1280: Test failed: D3DVIEWPORT->X = 10, expected 0. device.c:1281: Test failed: D3DVIEWPORT->Y = 20, expected 0. device.c:1282: Test failed: D3DVIEWPORT->Width = 400, expected 800. device.c:1283: Test failed: D3DVIEWPORT->Height = 300, expected 600. device.c:1284: Test failed: D3DVIEWPORT->MinZ = 2.00000003e-001, expected 0. device.c:1285: Test failed: D3DVIEWPORT->MaxZ = 3.00000012e-001, expected 1. device.c:1290: Test failed: Screen width is 640, expected 800. device.c:1291: Test failed: Screen height is 480, expected 600. device.c:1297: Test failed: Back buffer width is 640, expected 800. device.c:1299: Test failed: Back buffer height is 480, expected 600. device.c:1329: Test failed: Screen width is 640, expected 1024. device.c:1330: Test failed: Screen height is 480, expected 768. device.c:2550: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2555: Test failed: The focus window is active. device.c:2563: Test failed: Got unexpected hr 0x88760869. device.c:2569: Test failed: Got unexpect screen size 800x600. device.c:2584: Test failed: Expected message 0x47 for window 0, but didn't receive it, i=1. device.c:2593: Test failed: Got unexpect screen size 800x600. device.c:2606: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2614: Test failed: Got unexpect width 640. device.c:2615: Test failed: Got unexpect height 480.

1 0

Re: [PATCH 8/9] wined3d: Filter messages generated by focus handling.
by Marvin Dec. 4, 2014

Dec. 4, 2014

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=10623 Your paranoid android. === w8 (32 bit device) === device.c:1222: Test failed: TestCooperativeLevel failed, hr 0x88760868. device.c:1267: Test failed: Reset failed, hr 0x88760868. device.c:1269: Test failed: TestCooperativeLevel failed, hr 0x88760869. device.c:1273: Test failed: Got unexpected value 0 for D3DRS_LIGHTING. device.c:1280: Test failed: D3DVIEWPORT->X = 10, expected 0. device.c:1281: Test failed: D3DVIEWPORT->Y = 20, expected 0. device.c:1282: Test failed: D3DVIEWPORT->Width = 400, expected 800. device.c:1283: Test failed: D3DVIEWPORT->Height = 300, expected 600. device.c:1284: Test failed: D3DVIEWPORT->MinZ = 2.00000003e-001, expected 0. device.c:1285: Test failed: D3DVIEWPORT->MaxZ = 3.00000012e-001, expected 1. device.c:1290: Test failed: Screen width is 640, expected 800. device.c:1291: Test failed: Screen height is 480, expected 600. device.c:1297: Test failed: Back buffer width is 640, expected 800. device.c:1299: Test failed: Back buffer height is 480, expected 600. device.c:1329: Test failed: Screen width is 640, expected 1024. device.c:1330: Test failed: Screen height is 480, expected 768. device.c:2550: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2555: Test failed: The focus window is active. device.c:2563: Test failed: Got unexpected hr 0x88760869. device.c:2569: Test failed: Got unexpect screen size 800x600. device.c:2584: Test failed: Expected message 0x47 for window 0, but didn't receive it, i=1. device.c:2593: Test failed: Got unexpect screen size 800x600. device.c:2606: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2614: Test failed: Got unexpect width 640. device.c:2615: Test failed: Got unexpect height 480.

1 0

Re: [PATCH 5/9] d3d8/tests: Test WM_SYSCOMMAND handling.
by Marvin Dec. 4, 2014

Dec. 4, 2014

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=10620 Your paranoid android. === w8 (32 bit device) === device.c:1222: Test failed: TestCooperativeLevel failed, hr 0x88760868. device.c:1267: Test failed: Reset failed, hr 0x88760868. device.c:1269: Test failed: TestCooperativeLevel failed, hr 0x88760869. device.c:1273: Test failed: Got unexpected value 0 for D3DRS_LIGHTING. device.c:1280: Test failed: D3DVIEWPORT->X = 10, expected 0. device.c:1281: Test failed: D3DVIEWPORT->Y = 20, expected 0. device.c:1282: Test failed: D3DVIEWPORT->Width = 400, expected 800. device.c:1283: Test failed: D3DVIEWPORT->Height = 300, expected 600. device.c:1284: Test failed: D3DVIEWPORT->MinZ = 2.00000003e-001, expected 0. device.c:1285: Test failed: D3DVIEWPORT->MaxZ = 3.00000012e-001, expected 1. device.c:1290: Test failed: Screen width is 640, expected 800. device.c:1291: Test failed: Screen height is 480, expected 600. device.c:1297: Test failed: Back buffer width is 640, expected 800. device.c:1299: Test failed: Back buffer height is 480, expected 600. device.c:1329: Test failed: Screen width is 640, expected 1024. device.c:1330: Test failed: Screen height is 480, expected 768. device.c:2550: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2555: Test failed: The focus window is active. device.c:2563: Test failed: Got unexpected hr 0x88760869. device.c:2569: Test failed: Got unexpect screen size 800x600. device.c:2584: Test failed: Expected message 0x47 for window 0, but didn't receive it, i=1. device.c:2593: Test failed: Got unexpect screen size 800x600. device.c:2606: Test failed: Expected message 0x7e for window 0, but didn't receive it. device.c:2614: Test failed: Got unexpect width 640. device.c:2615: Test failed: Got unexpect height 480.

1 0