October 2014 - Wine-devel - list.winehq.org

Re: comctl32: Avoid magic numbers
by Nikolay Sivov 31 Oct '14

31 Oct '14

> /* FIXME: This flag is undocumented and unknown by our CompareString. > - * We need a define for it. > */ > - dwFlags = 0x10000000; > + dwFlags = LOCALE_RETURN_GENITIVE_NAMES; > if (!bCase) dwFlags |= NORM_IGNORECASE; It makes no sense to use this flag from CompareString(). I think it's better to remove it along with this comment.

3 2

Re: [PATCH 2/2] user32/tests: Test exception handling for timer callbacks.
by Marvin 31 Oct '14

31 Oct '14

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=9924 Your paranoid android. === w7u (32 bit msg) === msg.c:12154: Test failed: unexpected hwnd 0014013C msg.c:12176: Test failed: bad time 325e7/325f6

1 0

Re: [2/2] oleaut32: Implement TLB dependencies lookup in resources (resend 3 - last submit before abandon)
by Alexandre Julliard 31 Oct '14

31 Oct '14

Guillaume Charifi <guillaume.charifi(a)sfr.fr> writes: > + strcpyW(szTLBPath, szPath); > + snprintfW(szStringName, sizeof(szStringName)/sizeof(WCHAR), resFormatStrW, PtrToInt(lpszIntName)); > + strcatW(szTLBPath, szStringName); > + > + ret = TLB_PEFile_Open(szPath, PtrToInt(lpszIntName), &pBase, &dwTLBLength, &pFile); > + if (ret == TYPE_E_CANTLOADLIBRARY) > + ret = TLB_NEFile_Open(szPath, PtrToInt(lpszIntName), &pBase, &dwTLBLength, &pFile); > + if (ret == TYPE_E_CANTLOADLIBRARY) > + ret = TLB_Mapping_Open(szPath, &pBase, &dwTLBLength, &pFile); > + if (SUCCEEDED(ret)) > + { > + if (dwTLBLength >= 4) > + { > + DWORD dwSignature = FromLEDWord(*((DWORD*) pBase)); > + if (dwSignature == MSFT_SIGNATURE) > + pTLib = ITypeLib2_Constructor_MSFT(pBase, dwTLBLength); > + else if (dwSignature == SLTG_SIGNATURE) > + pTLib = ITypeLib2_Constructor_SLTG(pBase, dwTLBLength); > + else > + { > + FIXME("Header type magic 0x%08x not supported.\n", dwSignature); > + ret = E_FAIL; > + } > + } > + else > + ret = E_FAIL; > + IUnknown_Release(pFile); > + } You should avoid duplicating all that code. > + if(pTLib) { > + ITypeLibImpl *impl = impl_from_ITypeLib2(pTLib); > + ret = UuidEqual((UUID *)This->guid, (UUID *)TLB_get_guid_null(impl->guid), &rpc_stat); > + ITypeLib_Release((ITypeLib *)pTLib); > + > + ret = (ret == TRUE) ? LoadTypeLibEx(szTLBPath, REGKIND_NONE, (ITypeLib **)&pTLib) : E_FAIL; > + } That's pretty ugly. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 4/5] d3d9/tests: Add a test for ATI1N and ATI2N texture formats.
by Henri Verbeet 30 Oct '14

30 Oct '14

On 29 October 2014 18:01, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > @@ -6145,6 +6145,7 @@ static void test_lockrect_offset(void) > {D3DFMT_DXT3, "D3DFMT_DXT3", 4, 4, 16}, > {D3DFMT_DXT4, "D3DFMT_DXT4", 4, 4, 16}, > {D3DFMT_DXT5, "D3DFMT_DXT5", 4, 4, 16}, > + {MAKEFOURCC('A','T','I','1'), "ATI1N", 1, 1, 1}, > {MAKEFOURCC('A','T','I','2'), "ATI2N", 1, 1, 1}, Mostly just curiosity, but there are other tests in device.c for ATI2N, any reason for not adding ATI1N coverage to those as well? > + static const DWORD expected_colors_ati[] = {0x003f3f3f, 0x003f3f3f, 0x00007fff, 0x007f00ff}; > + static const DWORD expected_colors_nvidia[] = {0x007f0000, 0x007f0000, 0x00007fff, 0x007f00ff}; Conceptually I might have preferred this as static const struct { struct vec2 position; D3DCOLOR amd; D3DCOLOR nvidia; } expected_colors[] { ... } but it's not a big deal. > + for (i = 0; i < 4; ++i) > + { > + color = getPixelColor(device, 80 + 160 * i, 240); > + trace("Got 0x%08x.\n", color); Is this leftover debug code?

2 2

Need feedback on first four UTF-7 patches
by Alex Henrie 30 Oct '14

30 Oct '14

Alexandre, I submitted four UTF-7 patches on October 19 and am still waiting for feedback. I was hoping that these patches would not be controversial because they only add encoding tests. I have six additional patches waiting in https://github.com/alexhenrie/wine/commits/master - four patches for adding decoding tests and two patches to make all the tests pass - but I will not submit them until I get feedback about the first four that I have already submitted. -Alex

8 13

Re: msi: Format registry values before interpreting value type.
by Marvin 30 Oct '14

30 Oct '14

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=9898 Your paranoid android. === wxppro (32 bit action) === The test timed out === w7u (32 bit action) === The test timed out

1 0

Re: [PATCH 3/5] wined3d: Dirtify pixel shader on texture format change.
by Henri Verbeet 30 Oct '14

30 Oct '14

On 29 October 2014 18:01, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > + if (!prev || op->texture->target != prev->target > + || op->texture->resource.format != prev->resource.format) Shouldn't this just check the color_fixup? And in principle I think this would apply to vertex shaders as well, except that we don't track color fixups in struct vs_compile_args at all. The real pain will come with d3d10 of course, since you can access a much larger amount of resources there.

2 1

Re: dplayx: Fix some crashes and wine_todo's ( try 3)
by Marvin 30 Oct '14

30 Oct '14

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=9894 Your paranoid android. === w8 (32 bit dplayx) === Failure running script in VM: timed out waiting for the child process

1 0

Re: [PATCH 1/4] kernel32/tests: Add simple UTF-7 encoding tests.
by Sebastian Lackner 29 Oct '14

29 Oct '14

On 20.10.2014 06:41, Alex Henrie wrote: > --- > dlls/kernel32/tests/codepage.c | 134 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 134 insertions(+) > > diff --git a/dlls/kernel32/tests/codepage.c b/dlls/kernel32/tests/codepage.c > index 8423c75..684ede1 100644 > --- a/dlls/kernel32/tests/codepage.c > +++ b/dlls/kernel32/tests/codepage.c > @@ -412,6 +412,138 @@ static void test_string_conversion(LPBOOL bUsedDefaultChar) > ok(GetLastError() == 0xdeadbeef, "GetLastError() is %u\n", GetLastError()); > } > > +static void test_utf7_encoding(void) > +{ > + struct simple_test > + { > + WCHAR utf16[1024]; > + int utf16_len; > + char utf7[1024]; > + int utf7_len; 1024 is unnecessary big. A couple of bytes would be sufficient for all these tests. > + }; > + > + static const struct simple_test simple_tests[] = { No need to give that struct a name. You can simply do: static const struct { ... } simple_tests[] = { ... } > + /* tests some valid UTF-16 */ > + { > + {0x4F60,0x597D,0x5417,0}, > + 4, > + "+T2BZfVQX-", > + 11 > + }, > + /* tests some invalid UTF-16 */ > + /* (stray lead surrogate) */ That looks ugly, either put it in a single line or use: /* line1 * line2 */ > + { > + {0xD801,0}, > + 2, > + "+2AE-", > + 6 > + }, > + /* tests some more invalid UTF-16 */ > + /* (codepoint does not exist) */ > + { > + {0xFF00,0}, > + 2, > + "+/wA-", > + 6 > + } > + }; > + > + int i; > + > + for (i = 0; i < sizeof(simple_tests) / sizeof(simple_tests[0]); i++) > + { > + char c_buffer[1024]; > + WCHAR w_buffer[1024]; Same as above, way too big. > + int len; > + > + c_buffer[sizeof(c_buffer) - 1] = 0; > + w_buffer[sizeof(w_buffer) / sizeof(WCHAR) - 1] = 0; It probably makes more sense to initialize the buffers before each test, instead of one time at the beginning. > + > + /* test string conversion with srclen=-1 */ > + memset(c_buffer, '#', sizeof(c_buffer) - 1); > + SetLastError(0xdeadbeef); > + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, c_buffer, sizeof(c_buffer), NULL, NULL); > + if (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) > + { > + skip("UTF-7 encoding not implemented\n"); > + return; > + } At my opinion it would be better to do one basic test at the beginning, and then abort immediately, instead of doing that inside of the for loop, if its not too complicated. > + ok(len == simple_tests[i].utf7_len && > + strcmp(c_buffer, simple_tests[i].utf7) == 0 && > + c_buffer[len] == '#', > + "simple_test failure i=%i dst=\"%s\" len=%i\n", i, c_buffer, len); > + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError()); > + > + /* test string conversion with srclen=-2 */ > + memset(c_buffer, '#', sizeof(c_buffer) - 1); > + SetLastError(0xdeadbeef); > + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -2, c_buffer, sizeof(c_buffer), NULL, NULL); > + ok(len == simple_tests[i].utf7_len && > + strcmp(c_buffer, simple_tests[i].utf7) == 0 && > + c_buffer[len] == '#', > + "simple_test failure i=%i dst=\"%s\" len=%i\n", i, c_buffer, len); > + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError()); You are testing a lot of things in a single ok(...) block. It will be very difficult to find out whats wrong if we get test failures in the future. I think it would be better to split that into multiple checks, probably with some more meaningful message about what is wrong (wrong len, wrong content, ...) > + > + /* test string conversion with dstlen=len-1 */ > + memset(c_buffer, '#', sizeof(c_buffer) - 1); > + SetLastError(0xdeadbeef); > + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, c_buffer, simple_tests[i].utf7_len - 1, NULL, NULL); > + ok(len == 0 && > + memcmp(c_buffer, simple_tests[i].utf7, simple_tests[i].utf7_len - 1) == 0 && > + c_buffer[simple_tests[i].utf7_len - 1] == '#', > + "simple_test failure i=%i dst=\"%s\" len=%i\n", i, c_buffer, len); > + ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "error=%x\n", GetLastError()); > + > + /* test string conversion with dstlen=len */ > + memset(c_buffer, '#', sizeof(c_buffer) - 1); > + SetLastError(0xdeadbeef); > + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, c_buffer, simple_tests[i].utf7_len, NULL, NULL); > + ok(len == simple_tests[i].utf7_len && > + strcmp(c_buffer, simple_tests[i].utf7) == 0 && > + c_buffer[len] == '#', > + "simple_test failure i=%i dst=\"%s\" len=%i\n", i, c_buffer, len); > + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError()); > + > + /* test string conversion with dstlen=len+1 */ > + memset(c_buffer, '#', sizeof(c_buffer) - 1); > + SetLastError(0xdeadbeef); > + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, c_buffer, simple_tests[i].utf7_len + 1, NULL, NULL); > + ok(len == simple_tests[i].utf7_len && > + strcmp(c_buffer, simple_tests[i].utf7) == 0 && > + c_buffer[len] == '#', > + "simple_test failure i=%i dst=\"%s\" len=%i\n", i, c_buffer, len); > + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError()); > + > + /* test dry run with dst=NULL and dstlen=0 */ > + memset(c_buffer, '#', sizeof(c_buffer)); > + SetLastError(0xdeadbeef); > + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, NULL, 0, NULL, NULL); > + ok(len == simple_tests[i].utf7_len && > + c_buffer[0] == '#', > + "simple_test failure i=%i len=%i\n", i, len); > + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError()); > + > + /* test dry run with dst!=NULL and dstlen=0 */ > + memset(c_buffer, '#', sizeof(c_buffer) - 1); > + SetLastError(0xdeadbeef); > + len = WideCharToMultiByte(CP_UTF7, 0, simple_tests[i].utf16, -1, c_buffer, 0, NULL, NULL); > + ok(len == simple_tests[i].utf7_len && > + c_buffer[0] == '#', > + "simple_test failure i=%i len=%i\n", i, len); > + ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError()); > + > + /* all simple utf16-to-utf7 tests can be reversed to make utf7-to-utf16 tests */ > + memset(w_buffer, '#', sizeof(w_buffer) - sizeof(WCHAR)); > + SetLastError(0xdeadbeef); > + len = MultiByteToWideChar(CP_UTF7, 0, simple_tests[i].utf7, -1, w_buffer, sizeof(w_buffer) / sizeof(WCHAR)); > + todo_wine ok(len == simple_tests[i].utf16_len && > + memcmp(w_buffer, simple_tests[i].utf16, len * sizeof(WCHAR)) == 0 && > + w_buffer[len] == 0x2323, > + "simple_test failure i=%i dst=%s len=%i\n", i, wine_dbgstr_w(w_buffer), len); > + todo_wine ok(GetLastError() == 0xdeadbeef, "error=%x\n", GetLastError()); Why do you use todo_wine when the whole test block is skipped? Moreover, it should probably go into the later patches which add the MultibyteToWideChar tests. > + } > +} > + > static void test_undefined_byte_char(void) > { > static const struct tag_testset { > @@ -618,6 +750,8 @@ START_TEST(codepage) > test_string_conversion(NULL); > test_string_conversion(&bUsedDefaultChar); > > + test_utf7_encoding(); > + > test_undefined_byte_char(); > test_threadcp(); > } >

1 0

Re: [2/2] oleaut32: Implement TLB dependencies lookup in resources (resend 3 - last submit before abandon)
by Andrew Eikum 29 Oct '14

29 Oct '14

On Sat, Sep 27, 2014 at 08:10:28PM +0200, Guillaume Charifi wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=34184 > > This is the last time without a review before I give up. > I'm not sure why this is hung up, but to reiterate, it seems acceptable to me. Andrew

1 0