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

Re: [5/6] wsdapi: Add support for sending messages via UDP multicast.

by Vincent Povirk

+ delay = rand() % (UDP_MAX_DELAY - UDP_MIN_DELAY + 1) + UDP_MIN_DELAY; Using rand() for this doesn't seem very random, and it could interfere with an application's use of rand(). + /* Get size of buffer for adapters */ + adapterAddresses = (IP_ADAPTER_ADDRESSES *) HeapAlloc(heap, 0, bufferSize); Was this comment meant to be earlier? Sending messages to every adapter in sequence, with a delay for each, doesn't seem right. +void terminate_networking(IWSDiscoveryPublisherImpl *impl) +{ + impl->publisherStarted = FALSE; + + WSACleanup(); +} It seems like this will call WSACleanup even when freeing an object that was never used.

July 27, 2017

Re: [v2 PATCH] gdi32: Update metafile world transform immediately in winnt mode

by Alexandre Julliard

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

July 27, 2017

Re: [PATCH v7 24/26] x86: Enable User-Mode Instruction Prevention

by Borislav Petkov

On Tue, Jul 25, 2017 at 05:44:08PM -0700, Ricardo Neri wrote: > On Fri, 2017-06-09 at 18:10 +0200, Borislav Petkov wrote: > > On Fri, May 05, 2017 at 11:17:22AM -0700, Ricardo Neri wrote: > > > User_mode Instruction Prevention (UMIP) is enabled by setting/clearing a > > > bit in %cr4. > > > > > > It makes sense to enable UMIP at some point while booting, before user > > > spaces come up. Like SMAP and SMEP, is not critical to have it enabled > > > very early during boot. This is because UMIP is relevant only when there is > > > a userspace to be protected from. Given the similarities in relevance, it > > > makes sense to enable UMIP along with SMAP and SMEP. > > > > > > UMIP is enabled by default. It can be disabled by adding clearcpuid=514 > > > to the kernel parameters. ... > So would this become a y when more machines have UMIP? I guess. Stuff which proves reliable and widespread gets automatically enabled with time, in most cases. IMHO, of course. > Why would static_cpu_has() reply wrong if alternatives are not in place? > Because it uses the boot CPU data? When it calls _static_cpu_has() it > would do something equivalent to Nevermind - I forgot that static_cpu_has() now drops to dynamic check before alternatives application. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

July 27, 2017

Re: [PATCH] winmm: Load MCI drivers from MCI32 key.

by Andrew Eikum

On Wed, Jul 26, 2017 at 06:45:32PM -0500, Zebediah Figura wrote: > On 07/26/2017 08:03 AM, Andrew Eikum wrote: > >This looks good, but could you add some tests showing something that > >was wrong before but is now fixed? > > > > The application overwrites the value of MCI/avivideo in system.ini (to > 16-bit mciavi.drv). Is this appropriate for a test? > Maybe I misunderstood the impact of this patch. Won't it affect things like MCI_SYSINFO_QUANTITY, and other places that use those registry keys? Andrew

July 27, 2017

Re: [PATCH v7 16/26] x86/insn-eval: Support both signed 32-bit and 64-bit effective addresses

by Borislav Petkov

On Tue, Jul 25, 2017 at 04:48:13PM -0700, Ricardo Neri wrote: > I meant to say the 4 most significant bytes. In this case, the > 64-address 0xffffffffffff1234 would lie in the kernel memory while > 0xffff1234 would correctly be in the user space memory. That explanation is better. > Yes, perhaps the check above is not needed. I included that check as > part of my argument validation. In a 64-bit kernel, this function could > be called with val with non-zero most significant bytes. So say that in the comment so that it is obvious *why*. > I have looked into this closely and as far as I can see, the 4 least > significant bytes will wrap around when using 64-bit signed numbers as > they would when using 32-bit signed numbers. For instance, for two > positive numbers we have: > > 7fff:ffff + 7000:0000 = efff:ffff. > > The addition above overflows. Yes, MSB changes. > When sign-extended to 64-bit numbers we would have: > > 0000:0000:7fff:ffff + 0000:0000:7000:0000 = 0000:0000:efff:ffff. > > The addition above does not overflow. However, the 4 least significant > bytes overflow as we expect. No they don't - you are simply using 64-bit regs: 0x00005555555546b8 <+8>: movq $0x7fffffff,-0x8(%rbp) 0x00005555555546c0 <+16>: movq $0x70000000,-0x10(%rbp) 0x00005555555546c8 <+24>: mov -0x8(%rbp),%rdx 0x00005555555546cc <+28>: mov -0x10(%rbp),%rax => 0x00005555555546d0 <+32>: add %rdx,%rax rax 0xefffffff 4026531839 rbx 0x0 0 rcx 0x0 0 rdx 0x7fffffff 2147483647 ... eflags 0x206 [ PF IF ] (OF flag is not set). > We can clamp the 4 most significant bytes. > > For a two's complement negative numbers we can have: > > ffff:ffff + 8000:0000 = 7fff:ffff with a carry flag. > > The addition above overflows. Yes. > When sign-extending to 64-bit numbers we would have: > > ffff:ffff:ffff:ffff + ffff:ffff:8000:0000 = ffff:ffff:7fff:ffff with a > carry flag. > > The addition above does not overflow. However, the 4 least significant > bytes overflew and wrapped around as they would when using 32-bit signed > numbers. Right. Ok. And come to think of it now, I'm wondering, whether it would be better/easier/simpler/more straight-forward, to do the 32-bit operations with 32-bit types and separate 32-bit functions and have the hardware do that for you. This way you can save yourself all that ugly and possibly error-prone casting back and forth and have the code much more readable too. Hmmm. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

July 27, 2017

Re: [PATCH] gdi32: Update metafile world transform immediately in winnt mode

by Andrew Eikum

On Thu, Jul 27, 2017 at 01:07:20PM +0800, Dmitry Timoshkov wrote: > 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. > OK, I'll remove them and resend. Andrew

July 27, 2017

Re: [PATCH] gdiplus: Add stub for GdipGraphicsSetAbort

by Andrew Eikum

On Thu, Jul 27, 2017 at 01:01:51PM +0800, Dmitry Timoshkov wrote: > 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. > Thanks for the review, will fix! Andrew

July 27, 2017

Re: [PATCH] wmvcore/tests: add tests for WMReader methods

by Alistair Leslie-Hughes

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

July 27, 2017

Re: [PATCH 2/7] regedit: Use a helper function to allocate memory and check for a valid pointer

by Alexandre Julliard

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > On Thursday, 27 July 2017 2:41 AM, Alexandre Julliard wrote: >> That's not the standard heap_alloc(), so a different name would be >> preferable. I'd suggest heap_xalloc by analogy with xmalloc. > > "Standard" would be a function just returning HeapAlloc(...), right? Yes, the one that we use in many places, and that Michael is going to move to a global header any day now ;-) > By the way, regedit should exit(0), but perhaps exit(1) - as in the patch - > is still more appropriate for allocation failure? Sure. -- Alexandre Julliard julliard(a)winehq.org

July 27, 2017

Re: [PATCH] gdi32: Update metafile world transform immediately in winnt mode

by Dmitry Timoshkov

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.

July 27, 2017