- wine-devel - winehq.org

Re: [PATCH] winmm: Load MCI drivers from MCI32 key.
by Andrew Eikum 28 Jul '17

28 Jul '17

This looks good, but could you add some tests showing something that was wrong before but is now fixed? Andrew On Tue, Jul 25, 2017 at 09:18:15PM -0500, Zebediah Figura wrote: > MCI key/section is for 16-bit drivers, which can't be loaded by > 32-bit winmm. > > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/winmm/mci.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dlls/winmm/mci.c b/dlls/winmm/mci.c > index 56a537d..d770f6d 100644 > --- a/dlls/winmm/mci.c > +++ b/dlls/winmm/mci.c > @@ -66,10 +66,10 @@ WINE_DEFAULT_DEBUG_CHANNEL(mci); > #define MCI_MAGIC 0x0001 > > /* MCI settings */ > -static const WCHAR wszHklmMci [] = {'S','o','f','t','w','a','r','e','\\','M','i','c','r','o','s','o','f','t','\\','W','i','n','d','o','w','s',' ','N','T','\\','C','u','r','r','e','n','t','V','e','r','s','i','o','n','\\','M','C','I',0}; > +static const WCHAR wszHklmMci [] = {'S','o','f','t','w','a','r','e','\\','M','i','c','r','o','s','o','f','t','\\','W','i','n','d','o','w','s',' ','N','T','\\','C','u','r','r','e','n','t','V','e','r','s','i','o','n','\\','M','C','I','3','2',0}; > static const WCHAR wszNull [] = {0}; > static const WCHAR wszAll [] = {'A','L','L',0}; > -static const WCHAR wszMci [] = {'M','C','I',0}; > +static const WCHAR wszMci [] = {'M','C','I','3','2',0}; > static const WCHAR wszOpen [] = {'o','p','e','n',0}; > static const WCHAR wszSystemIni[] = {'s','y','s','t','e','m','.','i','n','i',0}; > > -- > 2.7.4 > > >

2 3

Re: [v2 PATCH] gdi32: Update metafile world transform immediately in winnt mode
by Alexandre Julliard 27 Jul '17

27 Jul '17

Andrew Eikum <aeikum(a)codeweavers.com> writes: > The Chrome browser print function implements their own handling for > EMR_MODIFYWORLDTRANSFORM which calls ModifyWorldTransform on the HDC > directly without ever calling PlayEnhMetaFileRecord. In Wine, this > transformation would get discarded when the callback function returned, > causing the page to be printed at the wrong scale. > > Tests show that the transform is updated immediately during > PlayEnhMetaFileRecord. In addition, a modified transform persists > between callbacks until PlayEnhMetaFileRecord is called on a relevant > type of callback, at which point the transform is reverted before > playing back the record. > > Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> > --- > > v2: Get rid of win9x checks in tests, and reformat test code. > > dlls/gdi32/enhmetafile.c | 41 ++++++- > dlls/gdi32/tests/metafile.c | 253 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 289 insertions(+), 5 deletions(-) This breaks the tests: ../../../tools/runtest -q -P wine -T ../../.. -M gdiplus.dll -p gdiplus_test.exe.so metafile && touch metafile.ok wine: Unhandled exception 0xc0000093 in thread a0 at address 0x7e8f10c5 (thread 00a0), starting debugger... Unhandled exception: floating point underflow in 32-bit code (0x7e8f10c7). Register dump: CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b EIP:7e8f10c7 ESP:0033f74c EBP:0033f768 EFLAGS:00010202( R- -- I - - - ) EAX:00000001 EBX:7e99b000 ECX:7e99b000 EDX:0033f7a8 ESI:7e99b000 EDI:0033f7a8 Stack dump: 0x0033f74c: 3f000000 00000000 00000000 3f000000 0x0033f75c: 7e99b000 00140bd0 0033f7a8 0033f7d8 0x0033f76c: 7e92e9f0 0033f7a8 0033f7a8 00140bd0 0x0033f77c: 00000000 3f800000 00000000 002f003e 0x0033f78c: 3f800000 3f800000 00000000 00000000 0x0033f79c: 3f800000 00000000 00000000 3f000000 Backtrace: =>0 0x7e8f10c7 CombineTransform+0x87() in gdi32 (0x0033f768) 1 0x7e92e9f0 EMF_Update_MF_Xform+0x6f(hdc=<is not available>, info=<is not available>) [/home/julliard/wine/wine/dlls/gdi32/enhmetafile.c:545] in gdi32 (0x0033f7d8) 2 0x7e930a4e PlayEnhMetaFileRecord+0x191d(hdc=<is not available>, handletable=<is not available>, mr=<is not available>, handles=<is not available>) [/home/julliard/wine/wine/dlls/gdi32/enhmetafile.c:1276] in gdi32 (0x0033f848) 3 0x7eca4fc1 GdipPlayMetafileRecord+0x170(metafile=<is not available>, recordType=<is not available>, flags=<is not available>, dataSize=<is not available>, data=<is not available>) [/home/julliard/wine/wine/dlls/gdiplus/metafile.c:1428] in gdiplus (0x0033f8c8) 4 0x7ed3887f test_gditransform_cb+0x2e(metafile=0x13f420, record_type=EmfRecordTypeGdiComment, flags=0, dataSize=0xc, pStr=") [/home/julliard/wine/wine/dlls/gdiplus/tests/metafile.c:2278] in gdiplus_test (0x0033f928) 5 0x7ed3881c play_metafile_proc+0x25b(record_type=<couldn't compute location>, flags=<couldn't compute location>, dataSize=<couldn't compute location>, pStr=<couldn't compute location>, userdata=<couldn't compute location>) [/home/julliard/wine/wine/dlls/gdiplus/tests/metafile.c:264] in gdiplus_test (0x0033f988) 6 0x7eca3495 enum_metafile_proc+0x64(hDC=<is not available>, lpHTable=<is not available>, lpEMFR=<is not available>, nObj=<is not available>, lpData=<is not available>) [/home/julliard/wine/wine/dlls/gdiplus/metafile.c:1841] in gdiplus (0x0033f9e8) 7 0x7e931af8 EnumEnhMetaFile+0x497() in gdi32 (0x0033faf8) 8 0x7eca5e56 GdipEnumerateMetafileSrcRectDestPoints+0x505() in gdiplus (0x0033fba8) 9 0x7ed39ea4 play_metafile.constprop+0x43() in gdiplus_test (0x0033fc08) 10 0x7ed43f42 func_metafile+0x2971() [/home/julliard/wine/wine/dlls/gdiplus/tests/metafile.c:2369] in gdiplus_test (0x0033fd48) 11 0x7eced39d main+0x25c(argc=<is not available>, argv=<is not available>) [/home/julliard/wine/wine/dlls/gdiplus/tests/../../../include/wine/test.h:603] in gdiplus_test (0x0033fdf8) 12 0x7ed5947b __wine_spec_exe_entry+0x4a(peb=<couldn't compute location>) [/home/julliard/wine/wine/dlls/winecrt0/exe_entry.c:36] in gdiplus_test (0x0033fe38) 13 0x7b45e85c call_process_entry+0xb() in kernel32 (0x0033fe58) 14 0x7b45f799 start_process+0x68(entry=<couldn't compute location>) [/home/julliard/wine/wine/dlls/kernel32/process.c:1116] in kernel32 (0x0033fe98) 15 0x7bc7ea9c call_thread_func_wrapper+0xb() in ntdll (0x0033feb8) 16 0x7bc81a59 call_thread_func+0xa8(entry=0x7b45f730, arg=0x7ed59430, frame=0x33ffc8) [/home/julliard/wine/wine/dlls/ntdll/signal_i386.c:2912] in ntdll (0x0033ffa8) 17 0x7bc7ea7a call_thread_entry_point+0x11() in ntdll (0x0033ffc8) 18 0x7bc53d43 start_process+0x12(arg=0xff987f08) [/home/julliard/wine/wine/dlls/ntdll/loader.c:3083] in ntdll (0x0033ffe8) 19 0xf753b91d __x86.get_pc_thunk.di+0x2a() in libwine.so.1 (0x00000000) 20 0xf753ba80 wine_switch_to_stack+0x1f(func=0x7bc53d30, arg=0xff987f08, stack=0x340000) [/home/julliard/wine/wine/libs/wine/port.c:77] in libwine.so.1 (0xff987ed8) 21 0x7bc599fe LdrInitializeThunk+0x22d(kernel_start=<couldn't compute location>, unknown2=<couldn't compute location>, unknown3=<couldn't compute location>, unknown4=<couldn't compute location>) [/home/julliard/wine/wine/dlls/ntdll/loader.c:3144] in ntdll (0xff987f28) 22 0x7b4656ff __wine_kernel_init+0xa4e() [/home/julliard/wine/wine/dlls/kernel32/process.c:1310] in kernel32 (0xff988e18) 23 0x7bc5a853 __wine_process_init+0x152() [/home/julliard/wine/wine/dlls/ntdll/loader.c:3353] in ntdll (0xff988e88) 24 0xf7539b83 wine_init+0x2a2(argc=0x3, argv=0xff9893d4, error="", error_size=0x400) [/home/julliard/wine/wine/libs/wine/loader.c:974] in libwine.so.1 (0xff988ed8) 25 0x7c000b58 main+0x97(argc=<is not available>, argv=<is not available>) [/home/julliard/wine/wine/loader/main.c:258] in <wine-loader> (0xff989328) 26 0xf7343276 __libc_start_main+0xf5() in libc.so.6 (0x00000000) 0x7e8f10c7 CombineTransform+0x87 in gdi32: flds 0xfffffff0(%ebp) -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH] gdi32: Update metafile world transform immediately in winnt mode
by Dmitry Timoshkov 27 Jul '17

27 Jul '17

Andrew Eikum <aeikum(a)codeweavers.com> wrote: > +#define IS_WIN9X() (GetVersion() & 0x80000000) ... > + if (IS_WIN9X()) > + /* win9x doesn't support world transforms */ > + return; The tests are supposed to get rid of win9x checks, not add new ones. -- Dmitry.

2 1

Re: [PATCH] gdiplus: Add stub for GdipGraphicsSetAbort
by Dmitry Timoshkov 27 Jul '17

27 Jul '17

Andrew Eikum <aeikum(a)codeweavers.com> wrote: > +static void test_GdipGraphicsSetAbort(void) > +{ > + HDC hdc = GetDC(hwnd); > + GpStatus status; > + GpGraphics *graphics; > + > + if (!pGdipGraphicsSetAbort) > + { > + win_skip("GdipGraphicsSetAbort() is not supported.\n"); > + return; hdc is leaked. > + } > + > + status = GdipCreateFromHDC(hdc, &graphics); > + expect(Ok, status); > + > + status = pGdipGraphicsSetAbort(NULL, NULL); > + expect(InvalidParameter, status); > + > + status = pGdipGraphicsSetAbort(graphics, NULL); > + expect(Ok, status); > + > + ReleaseDC(hwnd, hdc); > +} graphics is leaked. Also, it would be useful to test not invalid input as well. -- Dmitry.

2 1

Re: [PATCH] wmvcore/tests: add tests for WMReader methods
by Alistair Leslie-Hughes 27 Jul '17

27 Jul '17

Hi Jefferson, Thanks for the updated patch. When applying your patch there lot of whitespace errors (mostly from the header). warning: squelched 3569 whitespace errors warning: 3574 lines add whitespace errors. After look at qedit/tests, include the wma directly as a resource. (unless someone else objects) >// MSDN says dwType is a QWORD, but it is actually a DWORD C++ style comments aren't allowed. > ok(dwType == WMT_TYPE_DWORD, "onStatus pValue not WMT_TYPE_QWORD (%d), but %d\n", (int)WMT_TYPE_DWORD, dwType); Again WMT_TYPE_QWORD is incorrect here, and remove the (int)WMT_TYPE_DWORD parameter. TestWMReaderCallback_AddRef/Release could just return 2/1 respectfully. The object doesn't need to be deleted nor is there any reference type checks. wmvcore.c: In function ‘test_wmreader_play_wma’: wmvcore.c:425:8: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] ok((int)(onStatus_event = CreateEventW(NULL, FALSE, FALSE, NULL)), "Could not create event\n"); ^ You dont delete the wma file. Best Regards Alistair Leslie-Hughes

1 0

Re: [PATCH 2/7] regedit: Use a helper function to allocate memory and check for a valid pointer
by Alexandre Julliard 27 Jul '17

27 Jul '17

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > +static void *heap_alloc(size_t size) > +{ > + void *buf; > + > + if (!(buf = HeapAlloc(GetProcessHeap(), 0, size))) > + { > + ERR("Out of memory!\n"); > + exit(1); > + } > + > + return buf; > +} That's not the standard heap_alloc(), so a different name would be preferable. I'd suggest heap_xalloc by analogy with xmalloc. -- Alexandre Julliard julliard(a)winehq.org

2 2

Unicode normalization for Wine
by Artur Świgoń 26 Jul '17

26 Jul '17

Dear All, My name is Artur and I'm participating in Google Summer of Code 2017 for Wine. Under Nikolay's supervision, I'm working on implementation of Unicode normalization. I probably should have introduced myself some time ago to share results of my research and my ideas, but I also wanted to wait until I could illustrate my points with some code. I'm not sure whether everyone is familiar with Unicode normalization forms, so here's a short summary of concepts and terms I may use. For detailed information, please see [1]. - Characters that look the same on the screen may be encoded in a few different ways. Accented Latin letters may constitute a familiar example. The letter 'é' can be U+00E9 (LATIN SMALL LETTER E WITH ACUTE), but it also can be represented as the sequence U+0065 U+0301 (LATIN SMALL LETTER E, COMBINING ACUTE ACCENT). Many of 1114112 (if I remember the number correctly) Unicode characters are decomposable, i.e. they can also be represented by a different sequence of characters. - Text in normalization form NFD ('D' stands for 'decomposition') does not contain any decomposable characters, e.g. there is U+0065 U+0301 instead of U+00E9. The combining characters (such as the above-mentioned acute accent) have a well defined order, using the concept of combining classes. [2 §3.11] - Text in normalization form NFKD ('K' stands for 'compatibility') has the properties of NFD, plus characters that are typographical variants of other characters (e.g. superscripts, ligatures, etc.) are mapped to 'plain' characters. - Text that will be normalized into the form NFC and NFKC ('C' stands for 'composition') has to be fully decomposed first (NFD and NFKD, respectively) and starters (characters with a combining class zero) plus following non- starters (combining class greater than zero) can be composed into single characters provided that they exist in Unicode and are not present in [3]. I've developed a draft implementation of NFD & NFKD (mapping characters to decomposed sequences as well as reordering combining characters) and written a test program that uses official Unicode test data. [4] My goal was to provide support for codepoints greater than 0xFFFF, encoded in UTF-16 as surrogate pairs. More on that below. The attachment [gsoc-v2.patch] is a diff of my working branch against master. Most notable changes include: - Necessary changes to tools/make_unicode. - New function wine_decompose_string(). - Draft implementation of two Windows API functions: NormalizeString() and IsNormalizedString() (the latter temporarily returns TRUE). [5][6] Rationale and comments: - tools/make_unicode did not collect all necessary data (most notably, compatibility mappings), so a new data structure, @decomp_table_full has been added. Its unification with @decomp_table is yet to be resolved. - The procedure dump_decompose_table() in that script has been rewritten to accomodate all aspects described above and below. Two helper subroutines have been added. - However, the C code section embedded within the script has grown substantially, so it may be desirable to move it to a separate file. - Adding wine_decompose_string() makes sense since surrogate pairs behave like a string and the Unicode Canonical Ordering Algorithm (for combining characters) is an in-place bubble sort on the string, per Unicode specification. - While the code in libs/port/decompose.c (and embedded in tools/make_unicode) implements normalization, it currently serves the purpose of confirmation that the generated data is correct. It hasn't been yet optimized or cleaned up, because the organization of the data is open to discussion now, so the code may still change substantially. - It has been previously assumed that a decomposition mapping of any character is two characters long. This is certainly not true for NFKD, where a character like U+FDFA decomposes to a string of length 18. Therefore, the tables in libs/port/decompose.c contain pairs of offsets to null-terminated mappings in another table (decomp_data, first in that file), one entry for NFD and one for NFKD, instead of two-character mappings. Moreover, since many NFD and NFKD are identical, they have been placed in the same table, with identical mappings merged and fully expanded (i.e. any decomposable characters in the data have been replaced by fully decomposed sequences), to minimize runtime overhead. - The original idea of multi-stage tables (using the most significant bits of a codepoint to obtain an offset to another table etc.) has been preserved, although the tables are no longer combined into a single table. This is because some sub-tables can use a narrower data type, which saves space, especially when data for all Unicode codepoints is included. - Tables of combining classes have a similar structure. - Mappings for characters above 0xFFFF are encoded as UTF-16 (using surrogate pairs), but a single codepoint (UTF-32 if you like) is used for table indexing. Setting $utflim in make_unicode to 65536 is the simplest way to disable support for such characters, but supporting surrogate pairs should not affect any text-related Wine component in a negative way. - As composition depends directly on decomposition, it is the next thing to work on. The attachment [unorm_test.c] is my test program. It should build fine with mingw-w64 or winegcc without any specific compiler flags. It looks for [4] in its directory or accepts a path to that file as an argument. Option -d makes it run only decomposition tests, option -s limits the range to 0xFFFF. No tests should fail with the -s option and 80 of 187460 should fail without it due to incomplete support for surrogate pairs in the reordering part. For details of failed tests, please run with the -v option. Is there a place for such a program somewhere in the Wine source tree? I'd be more than happy to answer your questions, address your comments, and listen to your opinions regarding Unicode normalization. Regards, Artur Świgoń [1] http://www.unicode.org/reports/tr15/ [2] http://www.unicode.org/versions/Unicode10.0.0/ch03.pdf [3] http://www.unicode.org/Public/10.0.0/ucd/CompositionExclusions.txt [4] http://www.unicode.org/Public/10.0.0/ucd/NormalizationTest.txt [5] https://msdn.microsoft.com/en-us/library/windows/desktop/dd319093(v=vs.85).… [6] https://msdn.microsoft.com/en-us/library/windows/desktop/dd318671(v=vs.85).…

2 1

Unicode normalization for Wine - new patch
by Artur Świgoń 26 Jul '17

26 Jul '17

I'm not sure whether I've sent the updated patch in my previous email. This one is the one that passes the number of tests I described in my previous message. Sorry for confusion. Artur

1 0

Re: [PATCH] wmvcore/tests: add tests for WMReader methods
by Alistair Leslie-Hughes 26 Jul '17

26 Jul '17

Hi Jefferson, > > +typedef struct { > + IWMReaderCallback IWMReaderCallback_iface; > + LONG ref; > + HRESULT (*onStatusCallback)(WMT_STATUS, HRESULT, WMT_ATTR_DATATYPE, BYTE*, void*, void*); > + HRESULT (*onSampleCallback)(DWORD, QWORD, QWORD, DWORD, INSSBuffer*, void*, void*); > + void *pvCallbackContext; > +} TestWMReaderCallback; I'm still a little confused as to why you need this structure when the IWMReaderCallbackVtbl with a few globals would work just as well. > +static HRESULT onStatus_test_wmreader_play_wma( > + WMT_STATUS status, > + HRESULT hr, > + WMT_ATTR_DATATYPE dwType, > + BYTE *pValue, > + void *pvContext, > + void *pvCallbackContext) > +{ > + struct callback_context_test_wmreader_play_wma *pTestContext = > + (struct callback_context_test_wmreader_play_wma*)pvCallbackContext; > + DWORD *dwordValue; > + > + CHECK_EXPECT(onStatus); > + ok(status == onStatus_expected_statuses_test_wmreader_play_wma[pTestContext->onStatus_call_count], > + "onStatus called with unexpected status: %d\n", status); > + if (status == WMT_OPENED) { > + ok(pvContext == pTestContext->pvContextOpen, > + "onStatus was not passed pvContext: expected %p, actual %p\n", pTestContext->pvContextOpen, pvContext); > + } > + else /* status == WMT_STARTED, or later */ { > + ok(pvContext == pTestContext->pvContextStart, > + "onStatus was not passed pvContext: expected %p, actual %p\n", pTestContext->pvContextStart, pvContext); > + } > + switch (status) { > + case WMT_STARTED: > + // MSDN says dwType is a QWORD, but it is actually a DWORD > + ok(dwType == WMT_TYPE_DWORD, "onStatus pValue not WMT_TYPE_QWORD (%d), but %d\n", (int)WMT_TYPE_DWORD, dwType); WMT_TYPE_QWORD typo? > + dwordValue = (DWORD*)pValue; > + ok(*dwordValue == 0, "onStatus call for WMT_STARTED not passed correct starting timestamp: %u\n", *dwordValue); > + SET_EXPECT(onSample); > + break; > + default: > + break; > + } The switch and if could be merged > + /* Get format count. */ > + formatCount = 0; > + hr = IWMReader_GetOutputFormatCount(reader, 0, &formatCount); > + todo_wine ok(hr == S_OK, "Failed to get output format count: 0x%08x\n", hr); formatCount should be checked Best Regards Alistair Leslie-Hughes

2 2

Re: wined3d: Fix context backup window handling.
by Henri Verbeet 25 Jul '17

25 Jul '17

On 25 July 2017 at 22:35, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > - if (dc == context->hdc && !context->hdc_is_private && WindowFromDC(dc) != context->win_handle) > + if (dc == context->hdc && !private && WindowFromDC(dc) != context->win_handle) While that works, I think I'd feel happier about the code if the "dc", "private" and "format" parameters just went away. I.e., the reason they still exist is mostly the backup window path in context_set_gl_context(), and I don't think there's a reason that particular path shouldn't just set the appropriate (hdc, hdc_is_private, hdc_has_format) fields in the context instead.

1 0

Re: [PATCH] gdiplus: Avoid division by zero in SOFTWARE_GdipDrawThinPath
by Fabian Maurer 25 Jul '17

25 Jul '17

Or maybe we should draw this single pixel? Regards, Fabian Maurer

2 2

Re: [PATCH 4/4 v2] mshtml: Add IElementTraversal::get_firstElementChild implementation
by Marvin 25 Jul '17

25 Jul '17

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=32476 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath htmldoc.c:5092: Test failed: unexpected call {d9befc84-f21e-4166-874b-a8d3010dc764}

1 0

Re: [PATCH 3/4 v2] mshtml: Expose IElementTraversal interface to scripts
by Marvin 25 Jul '17

25 Jul '17

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=32475 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath htmldoc.c:5092: Test failed: unexpected call {d9befc84-f21e-4166-874b-a8d3010dc764}

1 0

Re: [PATCH 3/4 v2] mshtml: Expose IElementTraversal interface to scripts
by Jacek Caban 25 Jul '17

25 Jul '17

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>

1 0

Re: [PATCH 4/4] mshtml: Add IElementTraversal::get_firstElementChild implementation
by Marvin 25 Jul '17

25 Jul '17

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=32474 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath

1 0

Re: [PATCH 3/4] mshtml: Expose IElementTraversal interface to scripts
by Marvin 25 Jul '17

25 Jul '17

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=32473 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath

1 0

Re: [PATCH 1/5] po: Fix a typo in the Norwegian translation
by Kim Malmo 25 Jul '17

25 Jul '17

Thank you kindly! How did you find it? I read through the patch 3 times and missed it. Kim --- Den 24. juli 2017 14:47, skrev Hugh McMaster: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > po/nb_NO.po | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/po/nb_NO.po b/po/nb_NO.po > index 2fa8d89863..0ad494b4f2 100644 > --- a/po/nb_NO.po > +++ b/po/nb_NO.po > @@ -12512,7 +12512,7 @@ msgstr "regedit: Registerverdien \"%1\" vil bli eksportert som binære data.\n" > > #: regedit.rc:427 > msgid "regedit: Invalid system key [%1]\n" > -msgstr "reg: Ugyldig systemnøkkel [%1]\n" > +msgstr "regedit: Ugyldig systemnøkkel [%1]\n" > > #: regedit.rc:428 > msgid ""

1 0

Re: [1/2] gdi32/tests: Add test for SetPixel erase with R2_XORPEN
by Huw Davies 25 Jul '17

25 Jul '17

On Mon, Jul 24, 2017 at 12:57:30AM +1000, Tim Worthington wrote: > diff --git a/dlls/gdi32/tests/dib.c b/dlls/gdi32/tests/dib.c > --- a/dlls/gdi32/tests/dib.c > +++ b/dlls/gdi32/tests/dib.c > @@ -2703,6 +2703,26 @@ > ret = ExtFloodFill( hdc, -1, -1, RGB( 0, 0xff, 0 ), FLOODFILLSURFACE ); > ok (!ret, "got ret %d\n", ret); > > + /* SetPixel erase with R2_XORPEN */ > + if ( ( bmi->bmiHeader.biBitCount >= 16 ) > + || ( ( bmi->bmiHeader.biClrUsed == 256 ) && ( bmi->bmiHeader.biBitCount == 8 ) ) ) > + { > + int rop2; > + COLORREF set, get; > + > + rop2 = GetROP2( hdc ); > + for (i = 0; i < 256; i++) > + { > + SetROP2( hdc, R2_COPYPEN ); > + set = SetPixel( hdc, i * 2, 30, RGB( (i & 0x07) << 5, (i & 0x38) << 2, i & 0xc0 ) ); > + SetROP2( hdc, R2_XORPEN ); > + SetPixel( hdc, i * 2, 30, set ); > + get = GetPixel( hdc, i * 2, 30 ); > + ok( get == 0, "got %08x and %08x\n", set, get ); > + } > + SetROP2( hdc, rop2 ); > + } > + > SelectObject(hdc, orig_brush); > SelectObject(hdc, orig_pen); > DeleteObject(solid_brush); Hi Tim, Thank you for your contribution to Wine. There were a number of issues with this series, which by the time I'd reviewed them meant I'd essentially re-written the patches; so I just sent them in: https://source.winehq.org/patches/data/136093 https://source.winehq.org/patches/data/136094 This seemed more efficient than explaining the problems and suggesting alternative approaches. If you're interested, please free feel to look at my patches and ask if there's anything you're curious about. Huw.

2 1

Re: [PATCH] user32: In CURSORICON_FindBestCursor scale cursor correctly
by Fabian Maurer 24 Jul '17

24 Jul '17

Was too slow, not needed anymore after d0e8170c90ef63363f8389be2619c9332ae2c371. Regards, Fabian Maurer

1 0

Re: [PATCH] wmvcore/tests: add tests for WMReader methods
by Alistair Leslie-Hughes 24 Jul '17

24 Jul '17

Hi Jefferson, On 24/07/17 08:26, Jefferson Carpenter wrote: > Create a sample WMA file, which is saved into a temp directory at the > beginning of the wmvcore tests. > > Added a IWMReaderCallback implementation for use in conformance tests. > > Tested various playback-related WMReader methods. > > Changes in version 2 of this patch: Declared loop variable at top of > function, and using C89-style comments. The callback interface can just be static, see mshtml/tests/htmldoc.c for an example. Maybe using the CHECKED_CALLED syntax to check if something was been called. If the order of called is important then see msxml3/tests/saxreader.c for an example. You cannot use Sleep in tests, no matter how small it has to wait, instead use an Event+WaitForSingleObject. Use HeapAlloc/HeapFree instead of malloc/free, create a worker function if required. + IWMReader *reader; Leaking reader + /* Get output props. */ + hr = IWMReader_GetOutputProps(reader, 0, &props); + todo_wine ok(hr == S_OK, "Failed to get output props: 0x%08x\n", Leaking props here. .... + for (i = 0; i < formatCount; i++) { + /* Get output props for format. */ + hr = IWMReader_GetOutputFormat(reader, 0, i, &props); + todo_wine ok(hr == S_OK, "Failed to get output format: and here. Best Regards Alistair Leslie-Hughes

2 1

Re: mscoree: Update for mono profiler api v2.
by Vincent Povirk 24 Jul '17

24 Jul '17

Please don't commit this. They just changed it again and put in compatibility functions for the old API.

1 0

Some clarification of msi messages for translation and pedantic remarks
by Julian Rüger 24 Jul '17

24 Jul '17

Hi Zebediah (and list), I'd like some clarification of a few terms used in the new msi messages, to make sure I translate the correct meaning. :) #: msi.rc:103 msgid "Searching for related applications" ... #: msi.rc:111 msgid "Migrating feature states from related applications" related as in 'similar' or 'belonging together' or something else? I'd guess #2. #: msi.rc:113 msgid "Publishing assembly information" assembly as in 'binary/executable file/dll', 'the assembler language' or just 'putting together'? I strongly suspect #1, and that is a tough one. There is no fitting translation in German and this is not one of the many, many cases where you would just use the original English term. It sounds totally awkward. (Maybe because words ending in y are usually Germanized to use other suffixes. Just does not work with 'Assembly') EDIT: Never mind, I just looked it up and M$ does indeed use the untranslated original, even in compound words.. Ughhh! #: msi.rc:120 msgid "Registering Class servers" ... #: msi.rc:122 msgid "Registering extension servers" It's 'Server' in German as well for the hardware (machine/host) or applications/services like Apache, but this feels like a different thing. Am I being silly? Maybe this can only be answered by other German speakers. #: msi.rc:130 msi.rc:154 msgid "Updating environment strings" Can I just call this environment variables? Sounds more natural than the compound word with string ("Umgebungszeichenketten")... Thanks for your help and for all your work! Best, Julian PS: Some other questions/comments, unrelated to translation: #: msi.rc:168 msi.rc:171 msi.rc:175 msgid "File: [1], Directory: [9], Size: [6]" ... #: msi.rc:170 msgid "File: [1], Directory: [9], Size: [6]" Are these supposed to be separate, but identical? #: msi.rc:173 msgid "File: [1], Dependencies: [2]" ^^ Two spaces on purpose? #: msi.rc:176 msi.rc:177 msgid "Application Context:[1], Assembly Name:[2]" Missing spaces? #: msi.rc:189 msi.rc:209 msgid "Name: [1], Value: [2], Action [3]" Missing colon? (Action: [3])? #: msi.rc:178 msgid "File: [1], Directory: [2], Size: [3]" ... #: msi.rc:193 msi.rc:210 msgid "File: [1], Section: [2], Key: [3], Value: [4]" Duplicate spaces again?

2 1

Re: [PATCH 4/5] reg: Use a helper function to allocate and re-allocate memory
by Alexandre Julliard 24 Jul '17

24 Jul '17

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > @@ -174,6 +177,26 @@ static BOOL ask_confirm(unsigned int msgid, WCHAR *reg_info) > } > } > > +#define CHECK_ENOUGH_MEMORY(p) \ > +if (!(p)) \ > +{ \ > + output_message(STRING_OUT_OF_MEMORY, __FILE__, __LINE__); \ > + exit(1); \ > +} > + > +static void *resize_buffer(void *buf, size_t count) > +{ > + void *new_buf; > + > + if (buf) > + new_buf = HeapReAlloc(GetProcessHeap(), 0, buf, count); > + else > + new_buf = HeapAlloc(GetProcessHeap(), 0, count); > + > + CHECK_ENOUGH_MEMORY(new_buf); > + return new_buf; > +} The macro is not needed, and the message doesn't need to be localized, since this would require allocating memory. I think you'll also want a separate wrapper for HeapAlloc. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 1/3] msvcrt: Don't set errno when calling trigonometric functions with NAN.
by Piotr Caban 24 Jul '17

24 Jul '17

Hi, I've done some more reading about it and it looks like our implementation is not behaving in similar way as native does. The error handling should call _matherr function (see also __setusermatherr). Thanks to implementing it that way we easily write code that is compatible with older versions of C-runtime (by providing different default implementation of matherr function that prevents setting errno). I think it's best to implement it that way to avoid potential regressions. Thanks, Piotr

2 2

Re: ntdll: Add padding to the stack structure for quad-word alignment on ARM64
by Alexandre Julliard 24 Jul '17

24 Jul '17

André Hentschel <nerv(a)dawncrow.de> writes: > EXCEPTION_RECORD is not correctly aligned and until bd1f7ef1868dd563a82eacd1bf414bf8d1a7254e, CONTEXT wasn't either, > which ended up in a correctly aligned structure, but the mentioned commit changed that. > > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > dlls/ntdll/signal_arm64.c | 1 + > 1 file changed, 1 insertion(+) You should probably handle that in the existing alignment computation instead. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH v2 3/3] msvcr120: Add log1p.
by Piotr Caban 24 Jul '17

24 Jul '17

On 07/14/17 07:14, Alex Henrie wrote: > +/********************************************************************* > + * log1pf (MSVCR120.@) > + */ > +float CDECL MSVCR120_log1pf(float x) > +{ > +#ifdef HAVE_LOG1PF > + return log1pf(x); > +#else > + return log(1 + x); > +#endif > +} Please also set errno in this function. Thanks, Piotr

2 3

Re: [PATCH] wmvcore/tests: add tests for WMReader methods
by Marvin 23 Jul '17

23 Jul '17

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=32460 Your paranoid android. === build (build) === Make failed

1 0

Re: [PATCH] gdiplus: free dash_pattern_scaled (Coverity)
by Vincent Povirk 23 Jul '17

23 Jul '17

Thanks, Marcus. Unfortunately, with my patch for the same issue (caught by valgrind), this no longer applies. Can you rebase this please?

2 1

Re: ntdll: Fix stack alignment in RtlRaiseException assembly code.
by Alexandre Julliard 21 Jul '17

21 Jul '17

Sebastian Lackner <sebastian(a)fds-team.de> writes: > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > > Please review carefully, don't want to break anything shortly before the > release. ;) > > dlls/ntdll/signal_i386.c | 41 ++++++++++++++++++++++++----------------- > 1 file changed, 24 insertions(+), 17 deletions(-) All it does is call other API functions that will align the stack anyway, so I'm not sure it's worth the trouble. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH 1/2 v2] reg: Account for sizeof(WCHAR) when resizing the value name buffer during the query operation
by Alexandre Julliard 21 Jul '17

21 Jul '17

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > Changes since v1: > - Handle HeapReAlloc() failure This is a stand-alone app, so adding a couple of helpers that die on allocation failure would probably be a better choice than checking everywhere. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH] mshtml: Added IHTMLDOMNode3::put_textContent implementation.
by Marvin 21 Jul '17

21 Jul '17

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=32419 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath htmldoc.c:5092: Test failed: unexpected call {d9befc84-f21e-4166-874b-a8d3010dc764}

1 0

Re: [PATCH 1/2] xmllite/reader: Implement MoveToAttributeByName()
by Marvin 21 Jul '17

21 Jul '17

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=32415 Your paranoid android. === wxppro (32 bit reader) === reader.c:2597: Test failed: Unexpected hr 0x1. === w2003std (32 bit reader) === reader.c:2597: Test failed: Unexpected hr 0x1.

1 0

Re: [PATCH 2/8] d3d11: Fix race condition in d3d_blend_state_create().
by Henri Verbeet 21 Jul '17

21 Jul '17

On 20 July 2017 at 14:10, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Arguably we could use a different mutex for "device->blend_states". > Yeah. Generally speaking we'd like wined3d locking to be more fine-grained, but there are probably a bunch of issues with that.

1 0

Re: [PATCH] reg: Account for sizeof(WCHAR) when resizing the value name buffer during the query operation
by Henri Verbeet 21 Jul '17

21 Jul '17

On 21 July 2017 at 10:47, Hugh McMaster <hugh.mcmaster(a)outlook.com> wrote: > - value_name = HeapReAlloc(GetProcessHeap(), 0, value_name, max_value_len); > + value_name = HeapReAlloc(GetProcessHeap(), 0, value_name, max_value_len * sizeof(WCHAR)); Note that this leaks the existing "value_name" allocation if the HeapReAlloc() call should fail.

1 0

Re: [PATCH 3/6] msi: Don't set _BrowseProperty.
by Zebediah Figura 21 Jul '17

21 Jul '17

On 07/20/2017 11:43 PM, Zebediah Figura wrote: > This is a revert of 166a7e993cb0033fac7ec2712c346558fb7bd100, > which was introduced to fix bug #28219. _BrowseProperty is not set > by Windows, but should rather be set by the program in question, and > was not being set due to a bug in Wine, fixed by the previous patch. Sorry, typo—meant to say next patch (4/6).

1 0

Re: [PATCH 2/2] mshtml: Added IHTMLDOMNode3::get_textContent implementation.
by Marvin 20 Jul '17

20 Jul '17

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=32404 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath === w864 (64 bit events) === events.c:987: Test failed: unexpected call img_onerror

1 0

Re: [v3] gdi32: return non-zero v-refresh value for display devices.
by Huw Davies 20 Jul '17

20 Jul '17

On Thu, Jul 20, 2017 at 10:56:40PM +0900, Akihiro Sagawa wrote: > > To avoid division by zero (Bug 43369). > > In version 3, refine a testcase. > The refresh rate value is neither zero nor negative, thus we should test both. > Thanks for reviewing. > > Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com> > --- > dlls/gdi32/driver.c | 2 +- > dlls/gdi32/tests/dc.c | 8 ++++++++ > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/dlls/gdi32/driver.c b/dlls/gdi32/driver.c > index fe7812c..6a3975a 100644 > --- a/dlls/gdi32/driver.c > +++ b/dlls/gdi32/driver.c > @@ -344,7 +344,7 @@ static INT nulldrv_GetDeviceCaps( PHYSDEV dev, INT cap ) > case PHYSICALOFFSETY: return 0; > case SCALINGFACTORX: return 0; > case SCALINGFACTORY: return 0; > - case VREFRESH: return 0; > + case VREFRESH: return GetDeviceCaps( dev->hdc, TECHNOLOGY ) == DT_RASDISPLAY ? 1 : 0; > case DESKTOPVERTRES: return GetDeviceCaps( dev->hdc, VERTRES ); > case DESKTOPHORZRES: return GetDeviceCaps( dev->hdc, HORZRES ); > case BLTALIGNMENT: return 0; > diff --git a/dlls/gdi32/tests/dc.c b/dlls/gdi32/tests/dc.c > index add2c04..d316f3f 100644 > --- a/dlls/gdi32/tests/dc.c > +++ b/dlls/gdi32/tests/dc.c > @@ -388,6 +388,7 @@ static void test_device_caps( HDC hdc, HDC ref_dc, const char *descr, int scale > { > INT precision = 0; > INT hdc_caps = GetDeviceCaps( hdc, caps[i] ); > + INT ref_caps; > > switch (caps[i]) > { > @@ -400,6 +401,13 @@ static void test_device_caps( HDC hdc, HDC ref_dc, const char *descr, int scale > case LOGPIXELSY: > hdc_caps *= scale; > break; > + case VREFRESH: > + ref_caps = GetDeviceCaps( ref_dc, caps[i] ); > + if (GetDeviceCaps( ref_dc, TECHNOLOGY ) == DT_RASDISPLAY) > + ok( ref_caps > 0, "expected a positive value on %s, got %d\n", descr, ref_caps ); > + else > + ok( ref_caps == 0, "expected 0 on %s, got %d\n", descr, ref_caps ); > + break; > } You don't want to use 'ref_dc' here, use 'hdc' instead. Also, note that 'hdc_caps' already contains the thing you want to test, so you don't need the first call to GetDeviceCaps(). Huw.

1 0

Re: [PATCH] widl: Handle C++ aggregate returns in a MSVC compatible way.
by Jacek Caban 20 Jul '17

20 Jul '17

Hi Henri, On 20.07.2017 14:18, Henri Verbeet wrote: > This mainly affects 64-bit winelib applications, and potentially mingw-w64 > usage of the Wine headers. As explained in commit > fabfa59aea5c5b3201142382038beb3e76cb7567, MSVC returns aggregates through an > implicit parameter immediately after the this/interface pointer. GCC's > "ms_abi" attribute is supposed to match this, but unfortunately current > versions of g++ (confirmed up to at least g++ 6.3) place the implicit return > pointer before the this/interface pointer. See also > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52792. FWIW, here is a bug about 32-bit problem: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64384 > This commit takes an approach similar to the earlier commit > fabfa59aea5c5b3201142382038beb3e76cb7567: For ABI compatibility the return > pointer is listed as an explicit parameter in the vtbl entry, while an inline > helper is provided for source code compatibility. While it fixes the problem for C++ callers, it breaks building classes implementing those interfaces. Implementation will try to override inline function instead of virtual one. Given that the workaround has bad side effects, I'd say that we should at least try harder to not use it. For example it could be #ifdefed to not be used by compilers doing the right thing. With that, using our headers would require modifications in compiled code compatible with MSVC, so there would be a point in having a nice to use macro that interface implementations could use to decide if they should apply needed changes. Obviously fixing compilers sounds much better. Jacek

2 3

Re: [PATCH] gdi32: Update enhanced metafile world transform immediately in winnt mode
by Huw Davies 20 Jul '17

20 Jul '17

On Wed, Jul 19, 2017 at 02:06:29PM -0500, Andrew Eikum wrote: > /***************************************************************************** > * PlayEnhMetaFileRecord (GDI32.@) > @@ -1443,14 +1458,20 @@ BOOL WINAPI PlayEnhMetaFileRecord( > info->state.world_transform.eM11 = info->state.world_transform.eM22 = 1; > info->state.world_transform.eM12 = info->state.world_transform.eM21 = 0; > info->state.world_transform.eDx = info->state.world_transform.eDy = 0; > + if (!IS_WIN9X()) > + EMF_Update_MF_Xform(hdc, info); > break; > case MWT_LEFTMULTIPLY: > CombineTransform(&info->state.world_transform, &lpModifyWorldTrans->xform, > &info->state.world_transform); > + if (!IS_WIN9X()) > + ModifyWorldTransform(hdc, &lpModifyWorldTrans->xform, MWT_LEFTMULTIPLY); > break; > case MWT_RIGHTMULTIPLY: > CombineTransform(&info->state.world_transform, &info->state.world_transform, > &lpModifyWorldTrans->xform); > + if (!IS_WIN9X()) > + EMF_Update_MF_Xform(hdc, info); > break; > default: > FIXME("Unknown imode %d\n", lpModifyWorldTrans->iMode); > @@ -2216,6 +2237,13 @@ BOOL WINAPI PlayEnhMetaFileRecord( > LPtoDP(hdc, (POINT*)&tmprc, 2); > TRACE("L:0,0 - 1000,1000 -> D:%s\n", wine_dbgstr_rect(&tmprc)); > > + if (emr_modifies_transform( mr->iType ) && !IS_WIN9X()) > + { > + /* WinNT - update the transform (win9x updates when the next graphics > + output record is played). */ > + EMF_Update_MF_Xform(hdc, info); > + } > + > return TRUE; > } > I think it would be better to call EMF_Update_MF_Xform() in each 'case' block that needs it, just like you are doing for EMR_MODIFYWORLDTRANSFORM. Also, what about EMR_SETMAPMODE? > diff --git a/dlls/gdi32/tests/metafile.c b/dlls/gdi32/tests/metafile.c > index eeec79c800..eaad666361 100644 > --- a/dlls/gdi32/tests/metafile.c > +++ b/dlls/gdi32/tests/metafile.c [snip] > START_TEST(metafile) > { > init_function_pointers(); > @@ -3928,6 +4430,9 @@ START_TEST(metafile) > test_emf_paths(); > test_emf_PolyPolyline(); > test_emf_GradientFill(); > + test_emf_WorldTransform_simple(); > + test_emf_WorldTransform_modify(); > + test_emf_WorldTransform_modified_playback(); > You have a lot of duplicated code here. It should be possible to have a single test_emf_WorldTransform() that uses a table to drive these three tests. Could you also make the matrices a little more interesting? Adding a translation would make them non-commuting which would be good to test. Huw.

1 0

Re: [PATCH 3/3] mshtml: Expose IHTMLDOMNode3 interface to scripts.
by Marvin 19 Jul '17

19 Jul '17

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=32387 Your paranoid android. === w7u (32 bit htmldoc) === htmldoc.c:2682: Test failed: unexpected call GetOverrideKeyPath

1 0

Re: [v2] gdi32: return non-zero v-refresh value for display devices.
by Huw Davies 19 Jul '17

19 Jul '17

On Wed, Jul 19, 2017 at 11:28:58PM +0900, Akihiro Sagawa wrote: > diff --git a/dlls/gdi32/tests/dc.c b/dlls/gdi32/tests/dc.c > index add2c04..2b48542 100644 > --- a/dlls/gdi32/tests/dc.c > +++ b/dlls/gdi32/tests/dc.c > @@ -445,6 +445,7 @@ static void test_device_caps( HDC hdc, HDC ref_dc, const char *descr, int scale > if (type != COMPLEXREGION && type != ERROR) /* region can be complex on multi-monitor setups */ > { > RECT ref_rect; > + int rate = GetDeviceCaps( ref_dc, VREFRESH ); > > ok( type == SIMPLEREGION, "GetClipBox returned %d on %s\n", type, descr ); > if (GetDeviceCaps( ref_dc, TECHNOLOGY ) == DT_RASDISPLAY) > @@ -462,11 +463,15 @@ static void test_device_caps( HDC hdc, HDC ref_dc, const char *descr, int scale > SetRect( &ref_rect, GetSystemMetrics( SM_XVIRTUALSCREEN ), GetSystemMetrics( SM_YVIRTUALSCREEN ), > GetSystemMetrics( SM_XVIRTUALSCREEN ) + GetSystemMetrics( SM_CXVIRTUALSCREEN ), > GetSystemMetrics( SM_YVIRTUALSCREEN ) + GetSystemMetrics( SM_CYVIRTUALSCREEN ) ); > + > + ok( rate >= 1 && rate <= 85, "returned rate is out of range (1 -- 85), got %d\n", rate); Don't check the range, just that it's != 0. Also, please move this to a new VREFRESH case in the switch block higher up in this function. Huw.

1 0

Re: winhttp: Rename host_t to hostdata_t.
by Henri Verbeet 19 Jul '17

19 Jul '17

On 19 July 2017 at 17:22, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > Commit fa2cc9868b7db7a4cbf7d01a48500cb3f31e109a broke the compilation macOS > because host_t conflicts with a system header file: > > [19:31:41] In file included from ../../../wine/dlls/winhttp/session.c:50: > [19:31:41] ../../../wine/dlls/winhttp/winhttp_private.h:106:3: error: typedef redefinition with different types ('struct host_t' vs 'mach_port_t' (aka 'unsigned int')) > [19:31:41] } host_t; > [19:31:41] ^ > [19:31:41] /build/macos-rootfs/usr/include/mach/mach_types.h:120:22: note: previous definition is here > [19:31:41] typedef mach_port_t host_t; > [19:31:41] ^ > It may be too late for winhttp or Wine in general (although Michael does love these kinds of things), but note that the _t suffix is reserved by POSIX and should in principle be avoided on POSIX systems.

1 0

Re: gdi32: return non-zero v-refresh value for display devices.
by Huw Davies 19 Jul '17

19 Jul '17

On Wed, Jul 19, 2017 at 12:11:28AM +0900, Akihiro Sagawa wrote: > > To avoid division by zero (Bug 43369). > > Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com> > --- > dlls/gdi32/driver.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/gdi32/driver.c b/dlls/gdi32/driver.c > index fe7812c..6a3975a 100644 > --- a/dlls/gdi32/driver.c > +++ b/dlls/gdi32/driver.c > @@ -344,7 +344,7 @@ static INT nulldrv_GetDeviceCaps( PHYSDEV dev, INT cap ) > case PHYSICALOFFSETY: return 0; > case SCALINGFACTORX: return 0; > case SCALINGFACTORY: return 0; > - case VREFRESH: return 0; > + case VREFRESH: return GetDeviceCaps( dev->hdc, TECHNOLOGY ) == DT_RASDISPLAY ? 1 : 0; > case DESKTOPVERTRES: return GetDeviceCaps( dev->hdc, VERTRES ); > case DESKTOPHORZRES: return GetDeviceCaps( dev->hdc, HORZRES ); > case BLTALIGNMENT: return 0; Could you add a test (probably in dc.c:test_device_caps())? The values returned by the testbot range from 1 -- 85, so 1 is as good a choice as any. Huw.

2 2

Re: [v2 3/5] d3dx9: Support skip_constants parameter for effect.
by Matteo Bruni 18 Jul '17

18 Jul '17

2017-07-11 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > @@ -6397,24 +6464,75 @@ static HRESULT d3dx9_base_effect_init(struct d3dx9_base_effect *base, > TRACE("Tag: %x\n", tag); > } > > + if (skip_constants_string) > + { > + skip_constants_buffer = HeapAlloc(GetProcessHeap(), 0, > + sizeof(*skip_constants_buffer) * (strlen(skip_constants_string) + 1)); I think you can get rid of the sizeof() in this case. If sizeof(*skip_constants_buffer) is not 1 we have bigger problems anyway... > + if (!skip_constants_buffer) > + { > + if (bytecode) > + ID3D10Blob_Release(bytecode); > + return E_OUTOFMEMORY; > + } > + strcpy(skip_constants_buffer, skip_constants_string); Ideally this one should be a memcpy(), reusing the string length computed above. > @@ -588,7 +588,8 @@ static unsigned int *parse_pres_ins(unsigned int *ptr, unsigned int count, struc > return ptr; > } > > -static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D3DXCONSTANT_DESC *desc) > +static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D3DXCONSTANT_DESC *desc, > + WORD *constantinfo_reserved) > { > const struct ctab_constant *constant = d3dx_shader_get_ctab_constant(ctab, hc); > > @@ -598,10 +599,13 @@ static HRESULT get_ctab_constant_desc(ID3DXConstantTable *ctab, D3DXHANDLE hc, D > return D3DERR_INVALIDCALL; > } > *desc = constant->desc; > + if (constantinfo_reserved) > + *constantinfo_reserved = constant->constantinfo_reserved; > return D3D_OK; > } For future reference: these (and related) changes should probably have been in a separate patch, or even part of the previous patch in the series. Also, as it becomes more obvious here, one could go one step further with this and directly return struct ctab_constant to the caller. That would essentially make the get_ctab_constant_desc() helper redundant so it could be merged into the callers and removed entirely. > + for (j = 0; j < skip_constants_count; ++j) > + { > + if (!strcmp(cdesc[index].Name, skip_constants[j])) > + { > + if (!constantinfo_reserved) > + { > + WARN("skip_constants parameter %s is not register bound.\n", > + cdesc[index].Name); > + hr = D3DERR_INVALIDCALL; > + goto cleanup; > + } > + TRACE("Skipping constant %s.\n", cdesc[index].Name); > + break; > + } > + } > + if (j < skip_constants_count) > + continue; > + ++out->input_count; > + if (inputs_param[index]->class == D3DXPC_OBJECT) > + continue; > + if (FAILED(hr = init_set_constants_param(out, ctab, hc, inputs_param[index]))) > + goto cleanup; > } > - out->input_count = desc.Constants; > if (out->const_set_count) > { > out->const_set = HeapReAlloc(GetProcessHeap(), 0, out->const_set, Not particularly important but we could shrink out->inputs and out->inputs_param to out->input_count if any parameter was skipped.

1 0

Re: [PATCH 7/7] d3dx9: Support skip_constants parameter for effect.
by Matteo Bruni 18 Jul '17

18 Jul '17

2017-07-06 13:15 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > HRESULT d3dx_create_param_eval(struct d3dx9_base_effect *base_effect, void *byte_code, > unsigned int byte_code_size, D3DXPARAMETER_TYPE type, > - struct d3dx_param_eval **peval, ULONG64 *version_counter) DECLSPEC_HIDDEN; > + struct d3dx_param_eval **peval, ULONG64 *version_counter, > + const char **skip_constants_names, unsigned int skip_constant_name_count) DECLSPEC_HIDDEN; I think you can get rid of the "_name" part in those parameters, for brevity. > +static HRESULT parse_skip_constants_string(char *skip_constants, const char ***names, > + unsigned int *name_count) It's a bit a matter of personal taste but triple pointers look quite ugly to me. In this case you could e.g. return "names" in place of HRESULT. > +{ > + const char *name; > + char *s; > + unsigned int size = INITIAL_CONST_NAMES_SIZE; > + > + *names = HeapAlloc(GetProcessHeap(), 0, sizeof(*names) * size); > + if (!*names) > + return E_OUTOFMEMORY; > + > + *name_count = 0; > + s = skip_constants; > + while ((name = next_valid_constant_name(&s))) > + { > + if (*name_count == size) > + { > + size *= 2; > + *names = HeapReAlloc(GetProcessHeap(), 0, *names, sizeof(*names) * size); > + if (!*names) > + return E_OUTOFMEMORY; I think you're leaking memory on failure here. > + } > + (*names)[(*name_count)++] = name; > + } > + *names = HeapReAlloc(GetProcessHeap(), 0, *names, *name_count * sizeof(*names)); > + if (!*names) > + return E_OUTOFMEMORY; And here. > + for (i = 0; i < skip_constants_name_count; ++i) > + { > + struct d3dx_parameter *param; > + > + param = get_parameter_by_name(base, NULL, skip_constants_names[i]); > + if (param) > + { > + for (j = 0; j < base->technique_count; ++j) > + { > + if (is_parameter_used(param, &base->techniques[j])) > + { > + WARN("Parameter %s found in skip_constants is used in technique %u.\n", > + debugstr_a(skip_constants_names[i]), j); > + HeapFree(GetProcessHeap(), 0, skip_constants_buffer); > + HeapFree(GetProcessHeap(), 0, skip_constants_names); > + d3dx9_base_effect_cleanup(base); > + return D3DERR_INVALIDCALL; > + } > + } > + } > + else > + { > + TRACE("Parameter %s found in skip_constants not found.\n", > + debugstr_a(skip_constants_names[i])); This one sounds a bit awkward ("found... not found"). Maybe just rewrite that as "skip_constants parameter %s not found". You could apply the same trick to the previous message too. > + for (j = 0; j < skip_constant_name_count; ++j) > + { > + if (!strcmp(cdesc[index].Name, skip_constants_names[j])) I assume the skip_constants thing isn't used often and, when it is, very few constants (1-3) are in the list. If that's not the case, we might want to use an rbtree for the constant names.

2 2

Re: [PATCH v2] user32: should restore owned window when restoring owner window.
by Andrew Eikum 18 Jul '17

18 Jul '17

On Tue, Jul 18, 2017 at 04:47:05PM +0800, jiang.biao2(a)zte.com.cn wrote: > Is this the right place to commit patches for wine? > > could somebody help to review this patch? > This is the right place to send patches. Your patch needs tests to show that it is correct, and to detect regressions from future changes. Here are some good places to start: https://wiki.winehq.org/Developer_FAQ https://wiki.winehq.org/Submitting_Patches https://wiki.winehq.org/Conformance_Tests Andrew

1 0

Re: [PATCH 4/5] msvcr120: Don't set errno in expm1.
by Piotr Caban 18 Jul '17

18 Jul '17

On 07/18/17 06:33, Alex Henrie wrote: > @@ -2421,7 +2421,6 @@ double CDECL MSVCR120_expm1(double x) > #else > double ret = exp(x) - 1; > #endif > - if (!isfinite(ret)) *MSVCRT__errno() = MSVCRT_ERANGE; > return ret; > } The function behaves differently depending on the reason of overflow. It doesn't set ERANGE errno if x == INFINITY (as your tests show). But it does set it when e.g. x == 10e100. Thanks, Piotr

1 0

Re: [PATCH 5/5] ucrtbase/tests: Add tests for errno with math functions.
by Piotr Caban 18 Jul '17

18 Jul '17

On 07/18/17 06:33, Alex Henrie wrote: > +#define INF 1.0/0.0 Please define infinity in the same way as it's done in other tests. You can use __port_infinity or call p_strtod("inf"...) to obtain it. Thanks, Piotr

1 0

Re: [PATCH 2/8] regedit: Determine the number of values and subkeys before exporting any data
by Alexandre Julliard 18 Jul '17

18 Jul '17

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > programs/regedit/regproc.c | 212 +++++++++++++++++++-------------------------- > 1 file changed, 91 insertions(+), 121 deletions(-) This doesn't seem to be an improvement. The registry can change while you are exporting it, you can't rely on querying things beforehand. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH 4/6 v3] gdiplus: Store newer gdi+ version in created GdipRegions
by Vincent Povirk 17 Jul '17

17 Jul '17

It'd be very surprising to me if native does this and also requires a recognized version number, as that would break compatibility with 1.0 for no good reason. Based on the spec, they might only be checking the top 20 bits.

2 1

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

2004

2003

2002

2001

wine-devel