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

Error handling in RpcBindingVectorFree (dlls/rpcrt4/rpc_binding.c)
by Gerald Pfeifer Nov. 22, 2020

Nov. 22, 2020

Looking at RPC_STATUS WINAPI RpcBindingVectorFree( RPC_BINDING_VECTOR** BindingVector ) { RPC_STATUS status; ULONG c; TRACE("(%p)\n", BindingVector); for (c=0; c<(*BindingVector)->Count; c++) { status = RpcBindingFree(&(*BindingVector)->BindingH[c]); } HeapFree(GetProcessHeap(), 0, *BindingVector); *BindingVector = NULL; return RPC_S_OK; } we currently always ignore the outcome of RpcBindingFree and return RPC_S_OK. However, there is one case where RpcBindingFree returns something different (which is if *Binding is null when RPC_S_INVALID_BINDING is returned). What is the proper way of handling this? Just keeping the code as is and removing the unused status variable? Breaking the loop once RpcBindingFree returns something different from RPC_S_OK? Continuing and returning the first / the last status different from RPC_S_OK? Gerald

1 1

ntoskrnl.exe: unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces
by Qian Hong Sept. 28, 2018

Sept. 28, 2018

Dear all, While test another online bank with wine ActiveX, I got an unimplemented fuction of ntoskrnl: IoGetDeviceInterfaces, I found it listed in http://source.winehq.org/WineAPI/ntoskrnl.html as a stup, so I can't understand this log: wine: Unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces called at address 0x7b839552 (thread 0022), starting debugger... Grateful for any explain! env: wine1.3.12 on Ubuntu 10.04 Here are the steps: 1. install an ActiveX from https://e.bank.ecitic.com/perbank5/plugs/CNCBSecPkg_EN.exe $ rm -rf ~/.wine $ winetricks -q mfc42 $ wine CNCBSecPkg_EN.exe fixme:ole:DllRegisterServer stub fixme:win:DisableProcessWindowsGhosting : stub fixme:msg:ChangeWindowMessageFilter c057 00000001 fixme:msg:ChangeWindowMessageFilter c057 00000001 fixme:msg:ChangeWindowMessageFilter c057 00000001 fixme:ole:CoCreateInstance no instance created for interface {ea1afb91-9e28-4b86-90e9-9e9f8a5eefaf} of class {56fdf344-fd6d-11d0-958a-006097c9a090}, hres is 0x80004002 fixme:sfc:SfcIsFileProtected ((nil), L"C:\\Program Files\\\4e2d\4fe1\94f6\884c\7f51\94f6\5b89\5168\63a7\4ef6\\unins000.exe") stub fixme:win:WINNLSEnableIME hUnknown1 0x1011a bUnknown2 0: stub! fixme:win:WINNLSEnableIME hUnknown1 0x1011a bUnknown2 -1: stub! fixme:win:WINNLSEnableIME hUnknown1 0x1011a bUnknown2 0: stub! wine: Call from 0x7b839552 to unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces, aborting wine: Unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces called at address 0x7b839552 (thread 002b), starting debugger... wine: Call from 0x7b839552 to unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces, aborting wine: Call from 0x7b839552 to unimplemented function ntoskrnl.exe.IoGetDeviceInterfaces, aborting 2. open the online bank entry with wine builtin IE, then IE will crash: $ wine iexplore https://e.bank.ecitic.com/perbank5/signIn.do Please checkout the full log here: http://pastebin.com/rbAg7gwj Should I file a singel bug in ntoskrnl component , or separate bugs, one for ntoskrnl and one for the IE crashing? Generalliy what component should I switch while file a bug about IE crashing? Many thanks! -- Regards, Qian Hong - Sent from Ubuntu http://www.ubuntu.com/

5 6

COM tracing ...
by Michael Meeks Aug. 21, 2018

Aug. 21, 2018

Hi guys, Rather a peripheral question; apologies for that - but I'd imagine there are experts here that may be able to help. I have a black-box problem - a Windows app dealing with confidential data that I can't easily touch (and thus can't get to run under Wine) - which does some small but critical automation of MS Office - using VBS / COM scripting. I'd -really- love some input on how best to trace that series of COM method calls on Windows itself ie. the (remote) service activation, and the RPC beyond that - particularly the method names, parameters etc. of the COM/OLE Office API. I've tried (so far): http://www.rohitab.com/apimonitor which, while closed, looks interesting; but traces a lot in a hard-to-search way and doesn't appear to do the trick. https://support.microsoft.com/en-us/help/926098/how-to-enable-com-and-com-d… Sounds useful: using Event Tracing for Windows (ETW) but has this rather unhelpful property: "The binary file must be formatted by Microsoft so that it can be analyzed. Please forward the .etl files to your support contact. ..." ;-) Component models used to be all the rage in my youth ;-) surely someone solved the "strace for COM calls" problem elegantly some-when ! Crazily - might it be possible to instrument, interpose and use Wine's COM impl. on Windows [ which sounds a bit 'exciting' ;-], or ? Anyhow - help much appreciated & sorry for the noise ! Regards, Michael. -- michael.meeks(a)collabora.com <><, Pseudo Engineer, itinerant idiot

2 4

ODBC Databases: Jet/Access success
by Barry Bird June 14, 2018

June 14, 2018

Good Afternoon. In section 3.3.6.2 of your User Guide you ask readers to report successes with databases other than MS SQL. Well here's one (I know Access 2000 can work with Wine, but this doesn't require Access): How to set up Wine to enable Windows programs that read and write to Jet (Access) databases using ODBC. I write such programs in C and use the API defined in ODBC API Reference http://msdn.microsoft.com/en-us/library/ms714562%28VS.85%29.aspx They work fine on Windows and only require Mingw to be installed to compile and link them, both for console programs and those using Windows SDK. You then can write interactive programs in C which use a full-featured SQL database which comes bundled with Windows. No need to purchase Access. With the set-up below they will also run on Linux (Kubuntu 9.04) and Wine 1.1.26. You need to update the registry to install the Access drivers. Under Windows, export from the Registry to *.reg files the registry entries - HKLM\Software\ODBC - HKLM\Software\Microsoft\Jet all subsidiary keys and values come with them. You can carefully edit these files to remove drivers and DSNs you don't need. Import these files using the registry editor in wine: wine regedit.exe. You need to bring across from Windows to Windows\System32 under wine: clbcatq.dll comres.dll expsrv.dll msjet40.dll mswstr10.dll msjter40.dll msjint40.dll msjtes40.dll msrd3x40.dll odbc32.dll odbccp32.dll odbcji32.dll odbcjt32.dll odbcad32.exe odbcint.dll odbctrac.dll vbajet32.dll Register this server in wine: wine regsvr32.exe msjtes40.dll No need to install MDAC. To use Windows ODBC drivers, you have to override Wine's odbccp32.dll and odbc32.dll with the native versions because the Wine versions are currently wired directly to Linux's unixodbc. This can be done by setting up the ODBC Data Source Administrator odbcad32.exe using winecfg: - Add the program to the Applications tab - then in the libraries tab, pick from 'New override for library' drop-down odbc32.dll and odbccp32.dll add them and edit them to be Native for Windows. Then if you do wine odbcad32.exe this brings up the ODBC Data Source Administrator window as in the Windows Control Panel If needed, set up (System) DSNs using this program. Bring across the programs and *.mdb database files from Windows. Using winecfg, you need to set up each program you want to run with overrides for odbc32.dll and odbccp32.dll (Applications and Libraries tabs) as above. The programs should then run as they did in Windows but perhaps a bit slower with: wine Odbc-prog.exe This has been tested twice on a clean .wine install. I cannot vouch for support of all API and SQL facilities however. I hope someone finds this useful. Barry Bird

5 4

Re: [PATCH] server_ioctl_file() - Prefix printed hex values with '0x'
by Alexandre Julliard Oct. 12, 2017

Oct. 12, 2017

Chris Morgan <chmorgan(a)gmail.com> writes: > Avoids the ambiguity with non-alphanumeric hex values. > > Signed-off-by: Chris Morgan <chmorgan(a)gmail.com> > --- > dlls/ntdll/file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c > index fd7f3dd..4d081b7 100644 > --- a/dlls/ntdll/file.c > +++ b/dlls/ntdll/file.c > @@ -1567,7 +1567,7 @@ static NTSTATUS server_ioctl_file( HANDLE handle, HANDLE event, > SERVER_END_REQ; > > if (status == STATUS_NOT_SUPPORTED) > - FIXME("Unsupported ioctl %x (device=%x access=%x func=%x method=%x)\n", > + FIXME("Unsupported ioctl 0x%x (device=0x%x access=0x%x func=0x%x method=0x%x)\n", That's mostly a matter of taste, so I don't think it's necessary to change it. -- Alexandre Julliard julliard(a)winehq.org

3 2

Wine Swag Sticker Poll
by Aric Stewart Oct. 10, 2017

Oct. 10, 2017

Hello! So all the way back at wineconf I volunteered to be swag master for winehq. The idea is to have something that can be given away at trade shows, presentations, LUG visits and the like. Just something to help promote the visibility of the project. Jana here, CodeWeavers marketing director, has put together a sampling of designs to do as our first sticker and so now we are looking for some feedback on what people prefer. https://docs.google.com/forms/d/e/1FAIpQLSd5X_fXe9KSFAppRHz8kUo3XKeODyH5KeX… Rosanne? Could you direct me as to where this should be put up for the users as well? I am unfamiliar with how to communicate to our user base. The poll will run for as long as it feels like it needs to get some sort of winner. Comments and suggestions are always welcome! -aric

7 10

Re: wrc: Allow quotes in resource nameID.
by Alexandre Julliard May 16, 2017

May 16, 2017

Artur Świgoń <artur.swigon(a)gmail.com> writes: > @@ -138,6 +142,14 @@ static int wbufalloc = 0; > > static int current_codepage = -1; /* use language default */ > > +/* Used for differentiating between single- and double-quoted nameID's */ > +static int id_dblquote = 0; > + > +/* Used for catching first (quoted) nameID before wanted_id is set. > + * This variable is reset to 0 once the first ID (quoted or not), string or > + * data has been encountered */ > +static int id_initial = 1; That's a bit hackish. Couldn't you simply set the initial state appropriately instead of introducing a separate variable? > @@ -402,8 +414,30 @@ static unsigned long xstrtoul(const char *nptr, char **endptr, int base) > 0[oO][0-7]+[lL]? { parser_lval.num = xstrtoul(yytext+2, 0, 8); > return (yytext[yyleng-1] == 'L' || yytext[yyleng-1] == 'l') ? tLNUMBER : tNUMBER; } > > +<tkid_dbl>[A-Za-z_0-9./\\\']+\" | > +<tkid_sgl>[A-Za-z_0-9./\\\"]+\' { There are more characters that should be allowed. Nested quotes are also supposed to work. -- Alexandre Julliard julliard(a)winehq.org

2 2

winetricks alldlls=builtin update based on wine-2.7
by wylda＠volny.cz May 16, 2017

May 16, 2017

______________________________________________________________ > Od: Austin English <austinenglish(a)gmail.com> > Komu: wylda(a)volny.cz > Datum: 20.02.2017 07:07 > Předmět: Re: winetricks alldlls=builtin update based on wine-2.2 > > CC: <wine-devel(a)winehq.org> On Sun, Feb 19, 2017 at 7:51 AM, <wylda(a)volny.cz> wrote: > Hi, > in case you find it useful... > > W. Hi Wylda, Please send a pull request to https://github.com/Winetricks/winetricks/ -- -Austin GPG: 14FB D7EA A041 937B ______________________________________________________________ Hi Austin, i updated alldlls=builtin function for wine-2.7 and also fixed few things: - generates properly indentioned library list - removed nonsensical library names like "./dlls" - script generates override list matching 1:1 to wine-2.7 available list Unfortunately i not able to create pull request in github even though i have created github account right now. It's still grey out and i don't know different way to upload a patch. Anyway is there a different way to get it in, so i don't need to maintain locally? (spent whole day on this and i have no plans to learn git "details" in near futures). Best regards, W.

2 1

[v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention
by Ricardo Neri May 15, 2017

May 15, 2017

This is v6 of this series. The five previous submissions can be found here [1], here [2], here[3], here[4], and here[5]. This version addresses the comments received in v4 plus improvements of the handling of emulation in 64-bit builds. Please see details in the change log. === What is UMIP? User-Mode Instruction Prevention (UMIP) is a security feature present in new Intel Processors. If enabled, it prevents the execution of certain instructions if the Current Privilege Level (CPL) is greater than 0. If these instructions were executed while in CPL > 0, user space applications could have access to system-wide settings such as the global and local descriptor tables, the segment selectors to the current task state and the local descriptor table. These are the instructions covered by UMIP: * SGDT - Store Global Descriptor Table * SIDT - Store Interrupt Descriptor Table * SLDT - Store Local Descriptor Table * SMSW - Store Machine Status Word * STR - Store Task Register If any of these instructions is executed with CPL > 0, a general protection exception is issued when UMIP is enabled. === How does it impact applications? There is a caveat, however. Certain applications rely on some of these instructions to function. An example of this are applications that use WineHQ[6]. For instance, these applications rely on sidt returning a non- accessible memory location[8]. During the discussions, it was proposed that the fault could be relied to the user-space and perform the emulation in user-mode. However, this would break existing applications until, for instance, they update to a new WineHQ version. However, this approach would require UMIP to be disabled by default. The consensus in this forum is to always enable it. This patchset initially treated tasks running in virtual-8086 mode as a special case. However, I received clarification that DOSEMU[8] does not support applications that use these instructions. It relies on WineHQ for this [9]. Furthermore, the applications for which the concern was raised run in protected mode [8]. Please note that UMIP is always enabled for both 64-bit and 32-bit Linux builds. However, emulation of the UMIP-protected instructions is not done for 64-bit processes. 64-bit user space applications will receive the SIGSEGV signal when UMIP instructions causes a general protection fault. === How are UMIP-protected instructions emulated? This version keeps UMIP enabled at all times and by default. If a general protection fault caused by the instructions protected by UMIP is detected, such fault will be fixed-up by returning dummy values as follows: * SGDT and SIDT return hard-coded dummy values as the base of the global descriptor and interrupt descriptor tables. These hard-coded values correspond to memory addresses that are near the end of the kernel memory map. This is also the case for virtual-8086 mode tasks. In all my experiments in x86_32, the base of GDT and IDT was always a 4-byte address, even for 16-bit operands. Thus, my emulation code does the same. In all cases, the limit of the table is set to 0. * STR and SLDT return 0 as the segment selector. This looks appropriate since we are providing a dummy value as the base address of the global descriptor table. * SMSW returns the value with which the CR0 register is programmed in head_32/64.S at boot time. This is, the following bits are enabled: CR0.0 for Protection Enable, CR.1 for Monitor Coprocessor, CR.4 for Extension Type, which will always be 1 in recent processors with UMIP; CR.5 for Numeric Error, CR0.16 for Write Protect, CR0.18 for Alignment Mask. As per the Intel 64 and IA-32 Architectures Software Developer's Manual, SMSW returns a 16-bit results for memory operands. However, when the operand is a register, the results can be up to CR0[63:0]. Since the emulation code only kicks-in in x86_32, we return up to CR[31:0]. * The proposed emulation code is handles faults that happens in both protected and virtual-8086 mode. === How is this series laid out? ++ Fix bugs in MPX address evaluator I found very useful the code for Intel MPX (Memory Protection Extensions) used to parse opcodes and the memory locations contained in the general purpose registers when used as operands. I put some of this code in a separate library file that both MPX and UMIP can access and avoid code duplication. Before creating the new library, I fixed a couple of bugs that I found in how MPX determines the address contained in the instruction and operands. ++ Provide a new x86 instruction evaluating library With bugs fixed, the MPX evaluating code is relocated in a new insn-eval.c library. The basic functionality of this library is extended to obtain the segment descriptor selected by either segment override prefixes or the default segment by the involved registers in the calculation of the effective address. It was also extended to obtain the default address and operand sizes as well as the segment base address. Also, support to process 16-bit address encodings. Armed with this arsenal, it is now possible to determine the linear address onto which the emulated results shall be copied. This code supports Normal 32-bit and 64-bit (i.e., __USER32_CS and/or __USER_CS) protected mode, virtual-8086 mode, 16-bit protected mode with 32-bit base address. ++ Emulate UMIP instructions A new fixup_umip_exception functions inspect the instruction at the instruction pointer. If it is an UMIP-protected instruction, it executes the emulation code. This uses all the address-computing code of the previous section. ++ Add self-tests Lastly, self-tests are added to entry_from_v86.c to exercise the most typical use cases of UMIP-protected instructions in a virtual-8086 mode. ++ Extensive tests Extensive tests were performed to test all the combinations of ModRM, SiB and displacements for 16-bit and 32-bit encodings for the ss, ds, es, fs and gs segments. Tests also include a 64-bit program that uses segmentation via fs and gs. For this purpose, I temporarily, and not as part of this patchset, enabled UMIP support for 64-bit process with the intention to test the computations of linear addresses in 64-bit mode, including the extra R8-R15 registers. Extensive test is also implemented for virtual-8086 tasks. Code of these tests can be found here [10] and here [11]. [1]. https://lwn.net/Articles/705877/ [2]. https://lkml.org/lkml/2016/12/23/265 [3]. https://lkml.org/lkml/2017/1/25/622 [4]. https://lkml.org/lkml/2017/2/23/40 [5]. https://lkml.org/lkml/2017/3/3/678 [7]. https://www.winehq.org/ [8]. https://www.winehq.org/pipermail/wine-devel/2016-November/115320.html [9]. http://www.dosemu.org/ [9]. http://marc.info/?l=linux-kernel&m=147876798717927&w=2 [10]. https://github.com/01org/luv-yocto/tree/rneri/umip/meta-luv/recipes-core/um… [11]. https://github.com/01org/luv-yocto/commit/a72a7fe7d68693c0f4100ad86de6ecabd… Thanks and BR, Ricardo Changes since V5: * Relocate the page fault error code enumerations to traps.h Changes since V4: * Audited patches to use braces in all the branches of conditional. statements, except those in which the conditional action only takes one line. * Implemented support in 64-builds for both 32-bit and 64-bit tasks in the instruction evaluating library. * Split segment selector function in the instruction evaluating library into two functions to resolve the segment type by instruction override or default and a separate function to actually read the segment selector. * Fixed a bug when evaluating 32-bit effective addresses with 64-bit kernels. * Split patches further for for easier review. * Use signed variables for computation of effective address. * Fixed issue with a spurious static modifier in function insn_get_addr_ref found by kbuild test bot. * Removed comparison between true and fixup_umip_exception. * Reworked check logic when identifying erroneous vs invalid values of the SiB base and index. Changes since V3: * Limited emulation to 32-bit and 16-bit modes. For 64-bit mode, a general protection fault is still issued when UMIP-protected instructions are executed with CPL > 0. * Expanded instruction-evaluating code to obtain segment descriptor along with their attributes such as base address and default address and operand sizes. Also, support for 16-bit encodings in protected mode was implemented. * When getting a segment descriptor, this include support to obtain those of a local descriptor table. * Now the instruction-evaluating code returns -EDOM when the value of registers should not be used in calculating the effective address. The value -EINVAL is left for errors. * Incorporate the value of the segment base address in the computation of linear addresses. * Renamed new instruction evaluation library from insn-kernel.c to insn-eval.c * Exported functions insn_get_reg_offset_* to obtain the register offset by ModRM r/m, SiB base and SiB index. * Improved documentation of functions. * Split patches further for easier review. Changes since V2: * Added new utility functions to decode the memory addresses contained in registers when the 16-bit addressing encodings are used. This includes code to obtain and compute memory addresses using segment selectors for real-mode address translation. * Added support to emulate UMIP-protected instructions for virtual-8086 tasks. * Added self-tests for virtual-8086 mode that contains representative use cases: address represented as a displacement, address in registers and registers as operands. * Instead of maintaining a static variable for the dummy base addresses of the IDT and GDT, a hard-coded value is used. * The emulated SMSW instructions now return the value with which the CR0 register is programmed in head_32/64.S This is: PE | MP | ET | NE | WP | AM. For x86_64, PG is also enabled. * The new file arch/x86/lib/insn-utils.c is now renamed as arch/x86/lib/ insn-kernel.c. It also has its own header. This helps keep in sync the the kernel and objtool instruction decoders. Also, the new insn-kernel.c contains utility functions that are only relevant in a kernel context. * Removed printed warnings for errors that occur when decoding instructions with invalid operands. * Added more comments on fixes in the instruction-decoding MPX functions. * Now user_64bit_mode(regs) is used instead of test_thread_flag(TIF_IA32) to determine if the task is 32-bit or 64-bit. * Found and fixed a bug in insn-decoder in which X86_MODRM_RM was incorrectly used to obtain the mod part of the ModRM byte. * Added more explanatory code in emulation and instruction decoding code. This includes a comment regarding that copy_from_user could fail if there exists a memory protection key in place. * Tested code with CONFIG_X86_DECODER_SELFTEST=y and everything passes now. * Prefixed get_reg_offset_rm with insn_ as this function is exposed via a header file. For clarity, this function was added in a separate patch. Changes since V1: * Virtual-8086 mode tasks are not treated in a special manner. All code for this purpose was removed. * Instead of attempting to disable UMIP during a context switch or when entering virtual-8086 mode, UMIP remains enabled all the time. General protection faults that occur are fixed-up by returning dummy values as detailed above. * Removed umip= kernel parameter in favor of using clearcpuid=514 to disable UMIP. * Removed selftests designed to detect the absence of SIGSEGV signals when running in virtual-8086 mode. * Reused code from MPX to decode instructions operands. For this purpose code was put in a common location. * Fixed two bugs in MPX code that decodes operands. Ricardo Neri (21): x86/mpx: Use signed variables to compute effective addresses x86/mpx: Do not use SIB index if index points to R/ESP x86/mpx: Do not use R/EBP as base in the SIB byte with Mod = 0 x86/mpx, x86/insn: Relocate insn util functions to a new insn-kernel x86/insn-eval: Add utility functions to get register offsets x86/insn-eval: Add utility functions to get segment selector x86/insn-eval: Add utility function to get segment descriptor x86/insn-eval: Add utility function to get segment descriptor base address x86/insn-eval: Add functions to get default operand and address sizes x86/insn-eval: Do not use R/EBP as base if mod in ModRM is zero insn/eval: Incorporate segment base in address computation x86/insn: Support both signed 32-bit and 64-bit effective addresses x86/insn-eval: Add support to resolve 16-bit addressing encodings x86/insn-eval: Add wrapper function for 16-bit and 32-bit address encodings x86/mm: Relocate page fault error codes to traps.h x86/cpufeature: Add User-Mode Instruction Prevention definitions x86: Add emulation code for UMIP instructions x86/umip: Force a page fault when unable to copy emulated result to user x86/traps: Fixup general protection faults caused by UMIP x86: Enable User-Mode Instruction Prevention selftests/x86: Add tests for User-Mode Instruction Prevention arch/x86/Kconfig | 10 + arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/disabled-features.h | 8 +- arch/x86/include/asm/insn-eval.h | 23 + arch/x86/include/asm/traps.h | 18 + arch/x86/include/asm/umip.h | 15 + arch/x86/include/uapi/asm/processor-flags.h | 2 + arch/x86/kernel/Makefile | 1 + arch/x86/kernel/cpu/common.c | 16 +- arch/x86/kernel/traps.c | 4 + arch/x86/kernel/umip.c | 298 +++++++++ arch/x86/lib/Makefile | 2 +- arch/x86/lib/insn-eval.c | 832 ++++++++++++++++++++++++++ arch/x86/mm/fault.c | 88 ++- arch/x86/mm/mpx.c | 120 +--- tools/testing/selftests/x86/entry_from_vm86.c | 39 +- 16 files changed, 1301 insertions(+), 176 deletions(-) create mode 100644 arch/x86/include/asm/insn-eval.h create mode 100644 arch/x86/include/asm/umip.h create mode 100644 arch/x86/kernel/umip.c create mode 100644 arch/x86/lib/insn-eval.c -- 2.9.3

7 109

Re: [PATCH 2/9] winemac.drv: EnumDisplayDevices placeholder implementation
by Ken Thomases May 8, 2017

May 8, 2017

Hi, Thanks for taking this up again… On Apr 24, 2017, at 3:48 PM, Donat Enikeev <donat(a)enikeev.net> wrote: > > Based on Ken Thomases <ken(a)codeweavers.com> staging patches > > Signed-off-by: Donat Enikeev <donat(a)enikeev.net> > --- > dlls/winemac.drv/display.c | 51 +++++++++++++++++++++++++++++++++++++++ > dlls/winemac.drv/winemac.drv.spec | 1 + > 2 files changed, 52 insertions(+) This doesn't build: display.c:1007:13: error: use of undeclared identifier 'monitors' if (monitors[iDevNum].dwFlags & MONITORINFOF_PRIMARY) ^ display.c:1010:80: error: use of undeclared identifier 'monitors' memcpy(lpDisplayDevice->DeviceName, monitors[iDevNum].szDevice, sizeof(monitors[iDevNum].szDevice)); ^ display.c:1010:45: error: use of undeclared identifier 'monitors' memcpy(lpDisplayDevice->DeviceName, monitors[iDevNum].szDevice, sizeof(monitors[iDevNum].szDevice)); ^ Is this patch written on the assumption that some other staging patches have already been applied? Beyond that, can you explain the rationale of moving the implementation of EnumDisplayDevices to the drivers? My patches implemented it in user32 based on EnumDisplayMonitors() and GetMonitorInfoW(). It may be somewhat clunky, but has the advantage that it ensures all three functions return mutually consistent results. Since you appear to have dropped my patches which caused GetMonitorInfoW() to return unique names for the displays, they are no longer consistent. GetMonitorInfoW() always uses "\\.\DISPLAY1" for all monitors while this implementation of EnumDisplayDevices() attempts to use "\\.\DISPLAY2", etc., for secondary and subsequent monitors. For what it's worth, my original patches were rejected because Alexandre wanted the tracking and management of displays and display modes centralized in the explorer process[1]. I have my reservations about that approach, mind you. ;) [1] https://www.winehq.org/pipermail/wine-devel/2014-March/103680.html Issues with other patches in the series: Patch 1: \ No newline at end of file Patches 1 and 2 ought to come after 3. In the current order, they introduce dead code. -Ken

2 3

Wine native fps overlay
by CARLOS RODRIGUEZ May 3, 2017

May 3, 2017

Hi Could be possible add native fps overlay, maybe can usefull for many apps and without forget steam overlay give performance issues in certain apps (steam overlay dont work since wine 2.6) Thanks

3 2

pulse/alsa option in wine cfg audio tab
by CARLOS RODRIGUEZ May 3, 2017

May 3, 2017

Hi Seeking donat enikeev patch for add csmt option to graphics tab Could be possible add option for select which audio driver can be used aka alsa o pulse in winecfg audio tab ? Because various apps still needed alsa case tales of zestiria, alan wake and many others Thanks

2 1

Re: [PATCH] user32: Do not call SendMessage() to show a window that is already visible.
by Dmitry Timoshkov May 1, 2017

May 1, 2017

Kimmo Myllyvirta <kimmo.myllyvirta(a)gmail.com> wrote: > diff --git a/dlls/user32/winpos.c b/dlls/user32/winpos.c > index c2b35ec..64fdfe0 100644 > --- a/dlls/user32/winpos.c > +++ b/dlls/user32/winpos.c > @@ -1239,6 +1239,9 @@ BOOL WINAPI ShowWindow( HWND hwnd, INT cmd ) > if ((cmd == SW_HIDE) && !(GetWindowLongW( hwnd, GWL_STYLE ) & WS_VISIBLE)) > return FALSE; > > + if ((cmd == SW_SHOW) && (GetWindowLongW( hwnd, GWL_STYLE ) & WS_VISIBLE)) > + return TRUE; > + > return SendMessageW( hwnd, WM_WINE_SHOWWINDOW, cmd, 0 ); > } Please have a look at the show_window() implementation and try to cover all cases that trigger SWP_SHOWWINDOW flag passed to SetWindowPos() in order to actually show a window. In fact it's only SW_HIDE that leads to that, so perhaps just adding an 'else' to the SW_HIDE case is enough? -- Dmitry.

2 1

Wine native fps overlay
by CARLOS RODRIGUEZ May 1, 2017

May 1, 2017

Hi Yes +fps option works but not all time, many times wine gives priority to some errors messages than fps counter However native fps overlay will be better because overlay appears independent wine message errors Without forget will be better for new users but need to some option for enable in winecfg Thanks

2 1

Re: version: Fix error in win 9x mode when a file was not found in GetFileVersionInfoSize.
by Nikolay Sivov April 30, 2017

April 30, 2017

On 28.04.2017 18:03, Sebastian Lackner wrote: > From: Michael Müller <michael(a)fds-team.de> > > Signed-off-by: Michael Müller <michael(a)fds-team.de> > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > dlls/version/version.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/dlls/version/version.c b/dlls/version/version.c > index ed060c82b6..3ab53026e8 100644 > --- a/dlls/version/version.c > +++ b/dlls/version/version.c > @@ -705,7 +705,12 @@ DWORD WINAPI GetFileVersionInfoSizeExW( DWORD flags, LPCWSTR filename, LPDWORD h > return (len * 2) + 4; > > default: > - SetLastError( lzfd == HFILE_ERROR ? ofs.nErrCode : ERROR_RESOURCE_DATA_NOT_FOUND ); > + if (lzfd == HFILE_ERROR) > + SetLastError(ofs.nErrCode); > + else if (GetVersion() & 0x80000000) > + SetLastError(ERROR_FILE_NOT_FOUND); > + else > + SetLastError(ERROR_RESOURCE_DATA_NOT_FOUND); > return 0; > } > } > What depends on this? Is this function actually exported on Win9x? If not this fixup should happen in actual affected export I think.

2 1

Re: [PATCH v2] taskmgr/endproc.c: Implement ProcessPage_OnEndProcessTree()
by Bruno Jesus April 30, 2017

April 30, 2017

Hi, just a variable type change and minor style suggestions, some you can accept or ignore. > ... > > +static BOOL process_list_append(process_list *list, DWORD id) > +{ > + INT *temp; > + if (list->count == list->size) > { > - GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); > - MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP); > - return; > + list->size *= 2; > + temp = HeapReAlloc(GetProcessHeap(), 0, list->pid, list->size * sizeof(*list->pid)); > + if(!temp) > + return FALSE; > + > + list->pid = temp; > } > + list->pid[list->count++] = id; > + return TRUE; > +} Please use an empty line to separate variable declaration from code. Change "INT *temp" to "DWORD *temp" so it matches the same type from list->pid. You can also reduce the scope of the temp variable. There are 4 uses of "if(", please change to "if (". > - if (!TerminateProcess(hProcess, 0)) > +static void free_process_list(process_list *list) > +{ > + HeapFree(GetProcessHeap(), 0, list->pid); > +} > + > +static void enum_process_children(HANDLE snapshot, process_list *list, DWORD pid) > +{ > + PROCESSENTRY32 entry; > + SIZE_T start, end, i; > + > + start = list->count; > + entry.dwSize = sizeof(entry); > + > + if(!Process32First(snapshot, &entry)) > + return; > + > + do > + { > + if(entry.th32ParentProcessID == pid) > + { > + if(!process_list_append(list, entry.th32ProcessID)) > + { > + free_process_list(list); > + return; > + } > + } > + } while (Process32Next(snapshot, &entry)); > + > + end = list->count; > + for (i = start; i < end; ++i) > { > - GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); > - MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP); > + enum_process_children(snapshot, list, list->pid[i]); > } You can drop the { } since there is a single line now, but it is just a suggestion. Other style suggestion is to merge the end attribution inside the for like "for (i = start, end = list-> count; ...". > - > - CloseHandle(hProcess); > } > > void ProcessPage_OnEndProcessTree(void) > @@ -100,11 +171,14 @@ void ProcessPage_OnEndProcessTree(void) > DWORD dwProcessId; > HANDLE hProcess; > WCHAR wstrErrorText[256]; > + process_list list; > + SIZE_T i; > + HANDLE snapshot; > > load_message_strings(); > > Count = SendMessageW(hProcessPageListCtrl, LVM_GETITEMCOUNT, 0, 0); > - for (Index=0; Index<Count; Index++) > + for (Index = 0; Index < Count; Index++) > { > lvitem.mask = LVIF_STATE; > lvitem.stateMask = LVIS_SELECTED; > @@ -116,7 +190,6 @@ void ProcessPage_OnEndProcessTree(void) > if (lvitem.state & LVIS_SELECTED) > break; > } > - > Count = SendMessageW(hProcessPageListCtrl, LVM_GETSELECTEDCOUNT, 0, 0); Please don't remove the empty line to separate the code from the "for" block. > ... On Sat, Apr 29, 2017 at 7:08 PM, Akarsha Sehwag <akarsha15010(a)iiitd.ac.in> wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=39640 > > Implemented ProcessPage_OnEndProcessTree(). It now kills all the child processes along with the specified process. > > Signed-off-by: Akarsha Sehwag <akarsha15010(a)iiitd.ac.in> > --- > programs/taskmgr/endproc.c | 125 +++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 105 insertions(+), 20 deletions(-) > > diff --git a/programs/taskmgr/endproc.c b/programs/taskmgr/endproc.c > index 89c2d7b..60df0d7 100644 > --- a/programs/taskmgr/endproc.c > +++ b/programs/taskmgr/endproc.c > @@ -31,11 +31,18 @@ > #include "wine/unicode.h" > #include "taskmgr.h" > #include "perfdata.h" > +#include "tlhelp32.h" > > static WCHAR wszWarnMsg[511]; > static WCHAR wszWarnTitle[255]; > static WCHAR wszUnable2Terminate[255]; > > +typedef struct process_list { > + DWORD *pid; > + SIZE_T count; /* index to maintain the last entry of the array */ > + SIZE_T size; > +} process_list; > + > static void load_message_strings(void) > { > LoadStringW(hInst, IDS_TERMINATE_MESSAGE, wszWarnMsg, sizeof(wszWarnMsg)/sizeof(WCHAR)); > @@ -43,6 +50,22 @@ static void load_message_strings(void) > LoadStringW(hInst, IDS_WARNING_TITLE, wszWarnTitle, sizeof(wszWarnTitle)/sizeof(WCHAR)); > } > > +static void kill_process(HANDLE hProcess, WCHAR *wstrErrorText) > +{ > + if (!hProcess) > + { > + GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); > + MessageBoxW(hMainWnd, wstrErrorText, wszUnable2Terminate, MB_OK|MB_ICONSTOP); > + return; > + } > + > + if (!TerminateProcess(hProcess, 0)) > + { > + GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); > + MessageBoxW(hMainWnd, wstrErrorText, wszUnable2Terminate, MB_OK|MB_ICONSTOP); > + } > +} > + > void ProcessPage_OnEndProcess(void) > { > LVITEMW lvitem; > @@ -54,7 +77,7 @@ void ProcessPage_OnEndProcess(void) > load_message_strings(); > > Count = SendMessageW(hProcessPageListCtrl, LVM_GETITEMCOUNT, 0, 0); > - for (Index=0; Index<Count; Index++) > + for (Index=0; Index < Count; Index++) > { > lvitem.mask = LVIF_STATE; > lvitem.stateMask = LVIS_SELECTED; > @@ -77,20 +100,68 @@ void ProcessPage_OnEndProcess(void) > > hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, dwProcessId); > > - if (!hProcess) > + kill_process(hProcess, wstrErrorText); > + CloseHandle(hProcess); > +} > + > +static BOOL init_process_list(process_list *list) > +{ > + list->size = 4; /* initialise size with 4. Will increase if necessary */ > + list->pid = HeapAlloc(GetProcessHeap(), 0, list->size * sizeof(*list->pid)); > + list->count = 0; > + > + return list->pid != NULL; > +} > + > +static BOOL process_list_append(process_list *list, DWORD id) > +{ > + INT *temp; > + if (list->count == list->size) > { > - GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); > - MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP); > - return; > + list->size *= 2; > + temp = HeapReAlloc(GetProcessHeap(), 0, list->pid, list->size * sizeof(*list->pid)); > + if(!temp) > + return FALSE; > + > + list->pid = temp; > } > + list->pid[list->count++] = id; > + return TRUE; > +} > > - if (!TerminateProcess(hProcess, 0)) > +static void free_process_list(process_list *list) > +{ > + HeapFree(GetProcessHeap(), 0, list->pid); > +} > + > +static void enum_process_children(HANDLE snapshot, process_list *list, DWORD pid) > +{ > + PROCESSENTRY32 entry; > + SIZE_T start, end, i; > + > + start = list->count; > + entry.dwSize = sizeof(entry); > + > + if(!Process32First(snapshot, &entry)) > + return; > + > + do > + { > + if(entry.th32ParentProcessID == pid) > + { > + if(!process_list_append(list, entry.th32ProcessID)) > + { > + free_process_list(list); > + return; > + } > + } > + } while (Process32Next(snapshot, &entry)); > + > + end = list->count; > + for (i = start; i < end; ++i) > { > - GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); > - MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP); > + enum_process_children(snapshot, list, list->pid[i]); > } > - > - CloseHandle(hProcess); > } > > void ProcessPage_OnEndProcessTree(void) > @@ -100,11 +171,14 @@ void ProcessPage_OnEndProcessTree(void) > DWORD dwProcessId; > HANDLE hProcess; > WCHAR wstrErrorText[256]; > + process_list list; > + SIZE_T i; > + HANDLE snapshot; > > load_message_strings(); > > Count = SendMessageW(hProcessPageListCtrl, LVM_GETITEMCOUNT, 0, 0); > - for (Index=0; Index<Count; Index++) > + for (Index = 0; Index < Count; Index++) > { > lvitem.mask = LVIF_STATE; > lvitem.stateMask = LVIS_SELECTED; > @@ -116,7 +190,6 @@ void ProcessPage_OnEndProcessTree(void) > if (lvitem.state & LVIS_SELECTED) > break; > } > - > Count = SendMessageW(hProcessPageListCtrl, LVM_GETSELECTEDCOUNT, 0, 0); > dwProcessId = PerfDataGetProcessId(Index); > if ((Count != 1) || (dwProcessId == 0)) > @@ -125,20 +198,32 @@ void ProcessPage_OnEndProcessTree(void) > if (MessageBoxW(hMainWnd, wszWarnMsg, wszWarnTitle, MB_YESNO|MB_ICONWARNING) != IDYES) > return; > > - hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, dwProcessId); > + if (!init_process_list(&list)) > + return; > > - if (!hProcess) > + if (!process_list_append(&list, dwProcessId)) > { > - GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); > - MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP); > + free_process_list(&list); > return; > } > > - if (!TerminateProcess(hProcess, 0)) > + snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); > + > + if (snapshot == INVALID_HANDLE_VALUE) > { > - GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); > - MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP); > + free_process_list(&list); > + return; > } > + enum_process_children(snapshot, &list, dwProcessId); > + > + CloseHandle(snapshot); > > - CloseHandle(hProcess); > + for (i = 0; i < list.count; ++i) > + { > + hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, list.pid[i]); > + > + kill_process(hProcess, wstrErrorText); > + CloseHandle(hProcess); > + } > + free_process_list(&list); > } > -- > 2.9.3 > > >

1 0

Re: [v2 PATCH 1/4] mfplat: Implement MFTRegister
by Nikolay Sivov April 30, 2017

April 30, 2017

On 30.04.2017 6:29, Alistair Leslie-Hughes wrote: > + for (i = 0; i < cinput; i++) > + { > + memcpy(&types[2 * i], &input_types[i].guidMajorType, sizeof(GUID)); > + memcpy(&types[2 * i + 1], &input_types[i].guidSubtype, sizeof(GUID)); > + } This probably could be simplified even more, you can use MFT_REGISTER_TYPE_INFO for 'types' (that would make it 'type' actually), and do types[i] = input_types[i];, or even copy whole input_types array at once. Same applies to MFTEnum().

1 0

Re: [v2 PATCH 3/4] mfplat: Implement MFTEnum.
by Nikolay Sivov April 30, 2017

April 30, 2017

On 30.04.2017 6:29, Alistair Leslie-Hughes wrote: > + for (i = 0; i < size / sizeof(GUID); i += 2) > + { > + if (!IsEqualGUID(&guids[i], &type->guidMajorType) && > + !IsEqualGUID(&guids[i+1], &type->guidSubtype)) > + { > + ret = TRUE; > + break; > + } > + } This is confusing. Shouldn't it look for a match?

1 0

Re: [PATCH] user32: Do not call SendMessage() to show a window that is already visible.
by Marvin April 29, 2017

April 29, 2017

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=30597 Your paranoid android. === w7pro64 (32 bit msg) === msg.c:5274: Test failed: RedrawWindow:show_popup_extreme_location: 12: the msg 0x001c was expected, but got msg 0x0085 instead

2 1

Re: [PATCH] glu32: avoid assert side effect (Coverity)
by Nikolay Sivov April 29, 2017

April 29, 2017

On 29.04.2017 20:34, Marcus Meissner wrote: > CID 401313 > > Signed-off-by: Marcus Meissner <marcus(a)jet.franken.de> > --- > dlls/glu32/sweep.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/dlls/glu32/sweep.c b/dlls/glu32/sweep.c > index a909e2be33..1248e9f050 100644 > --- a/dlls/glu32/sweep.c > +++ b/dlls/glu32/sweep.c > @@ -1287,7 +1287,8 @@ static void DoneEdgeDict( GLUtesselator *tess ) > */ > if( ! reg->sentinel ) { > assert( reg->fixUpperEdge ); > - assert( ++fixedEdges == 1 ); > + ++fixedEdges; > + assert( fixedEdges == 1 ); > } > assert( reg->windingNumber == 0 ); > DeleteRegion( tess, reg ); > This particular one has no side effects. I don't know what's the plan for glu, maybe it's a better idea to report/fix anything the comes up in mesa, and then import again.

2 1

Re: [4/6] secur32: Return PRF algorithm value when using GCM. (resend)
by Michael Müller April 29, 2017

April 29, 2017

This patch broke the compilation on Debian Wheezy and Ubuntu Trusty: schannel_gnutls.c: In function schannel_get_mac_algid: schannel_gnutls.c:329:10: error: ‘GNUTLS_MAC_AEAD undeclared (first use in this function) schannel_gnutls.c:329:10: note: each undeclared identifier is reported only once for each function it appears in Build overview: https://dev.wine-staging.com/builder/group/60/ (Click Downloads for build logs) Debian Wheezy build log: https://builds.wine-staging.com/2017-04-27-21-23-02-job-10597/build.log Regards, Michael

5 5

Re: [PATCH 4/4] dmloader/tests: Add some IDirectMusicLoader directory tests.
by Sebastian Lackner April 28, 2017

April 28, 2017

On 27.04.2017 19:15, Michael Stefaniuc wrote: > Signed-off-by: Michael Stefaniuc <mstefani(a)winehq.org> > --- > dlls/dmloader/tests/loader.c | 41 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > Your patch introduced a bunch of test failures here: loader.c:69: Test succeeded inside todo block: SetSearchDirectory failed with 0 loader.c:71: Test failed: Second SetSearchDirectory failed with 0 loader.c:73: Test failed: SetSearchDirectory failed with 0x1

2 1

Re: Review | endproc.c: Implement ProcessPage_OnEndProcessTree
by Aaryaman Vasishta April 28, 2017

April 28, 2017

Forgot to CC wine-devel.. On Fri, Apr 21, 2017 at 12:47 AM, Aaryaman Vasishta < jem456.vasishta(a)gmail.com> wrote: > Hi, > > On Thu, Apr 20, 2017 at 6:51 AM, Akarsha Sehwag <akarsha15010(a)iiitd.ac.in> > wrote: > >> Hi >> Could someone take out a little time to review the code once? >> Bug I intend to solve: https://bugs.winehq.org/show_bug.cgi?id=39640 >> >> I am new to wine. So, I would appreciate suggestions regarding my coding >> style if that's not-so-perfect according to wine standards. Or any other >> problems with the code I have written. I apologize for submitting an >> untested code earlier. I hope now it works fine. >> > No worries! your efforts in contributing is always appreciated :) > > > /*-------------------------------------------CODE----------- >> ---------------------------------------*/ >> > Try using diffs/patches instead of manually copying them in this format. > Diffs are easier to apply and test with using git. That's how Stefan was > able to detect compilation errors in the patches you sent over on > wine-patches. I suggest sending another email with the changes in a diif > format if possible. > >> *//DOUBT: *Would sizeof(list->pid) be better or sizeof(int)? >> //Since in the former case, since list->pid is actually a pointer to >> DWORD, so //sizeof(list->pid) would return 8 bytes and not 4 bytes. >> >> } >> > Probably sizeof(DWORD) would be fine as well. In general, try following > what the surrounding, previously committed code is doing. > FWIW, pointers don't necessarily have to be 8 bytes in size. The size of a > pointer depends on whether your application is compiled as 32 or 64 bit. > It's possible to e.g. compile a program as 32 bit on a 64 bit machine. > For wine in 64 bit: https://wiki.winehq.org/FAQ#Is_there_a_64_bit_Wine.3F > > Cheers, > Aaryaman >

4 5

Re: [PATCH 4/4] mfplat/tests: Add tests.
by Andrew Eikum April 28, 2017

April 28, 2017

On Fri, Apr 28, 2017 at 04:29:07AM +0000, Alistair Leslie-Hughes wrote: > +static void test_register(void) > +{ > + static WCHAR name[] = {'W','i','n','e',' ','t','e','s','t',0}; > + MFT_REGISTER_TYPE_INFO input; > + MFT_REGISTER_TYPE_INFO output; > + CLSID *clsids; > + UINT32 count; > + HRESULT ret; > + > + memcpy(&input.guidMajorType, &DUMMY_GUID1, sizeof(GUID)); > + memcpy(&input.guidMajorType, &DUMMY_GUID2, sizeof(GUID)); > + memcpy(&output.guidSubtype, &DUMMY_GUID1, sizeof(GUID)); > + memcpy(&output.guidSubtype, &DUMMY_GUID3, sizeof(GUID)); > + Looks like some copy-paste errors here. Andrew

1 0

Re: ntdll: Use environment variable "WINESERVER" first on search for wineserver to start.
by Ralf Habacker April 28, 2017

April 28, 2017

Am 07.03.2017 um 15:20 schrieb Sebastian Lackner: > Was it intentional that you did not CC the wine-devel mailing list? no, I Just replyed to all; fixed that > > On 07.03.2017 10:22, Ralf Habacker wrote: >> In the opposite having wineserver environment variable set in users >> environment will make it sure to always starts wineserver with the >> requested settings. > Well, you could also change $PATH globally to let "wine" point to your > special wrapper executable. This would not really be different from using > a global WINESERVER variable. Really ? Adding an additional path looks more invasive in terms of security and maintenance as using a dedicated environment variable affecting only one executable. > > I think there is a misunderstanding here, each wineprefix uses its own > wineserver instance. Wineservers started for other prefixes will never > inherit any timeout settings. Thanks for this pointer, I wasn't aware of that. >> [...] >> >> With the mentioned patch applied to a local wine build (or with a >> renamed wineserver) the site now runs about two weeks and one noticable >> change is that the initial >> mentioned communication issue did not happens. This let me think that >> there may be a long time running issue with wineserver, which affects >> the dde interface. > It is nice that you have found a workaround, however we would also like > to fix such issues of course. If you find any way to trigger this issue, > please file a bug report. I will do but unfortunally I'm not at this point yet. Observations from the last weeks shows that the error happened two times: 1. wineserver is running 12 days and not restarted because one application showed a message box, therefore did not quit and blocked wineserver restart, which let other wine applications dde communication fail 20170309-061102.523831920: starting '/srv/www/gauszweb-prod/tools/usr/bin/wineserver --foreground --persistent=3600 ' -> last wineserver restart 20170321-150057.967117185: starting '/srv/www/gauszweb-prod/tools/usr/bin/wineserver --foreground --persistent=3600 ' -> manual restarted because of application freeze 2. wineserver is running 3 days and applications dde communications fails because wineserver has been restarted immediatly 20170322-061003.064358739: starting '/srv/www/gauszweb-prod/tools/usr/bin/wineserver --foreground --persistent=3600 ' -> last restart .... 20170325-220751 -> print job starts 20170325-220753.202518273: starting '/srv/www/gauszweb-prod/tools/usr/bin/wineserver --foreground --persistent=3600 ' -> wineserver has been restarted by timeout 20170325-220805 applications dde communication fails and wine reports wine: chdir to /tmp/.wine-37002/server-fd0a-9602cwine: : No such file or directory chdir to /tmp/.wine-37002/server-fd0a-9602c : No such file or directoryThis is expected and not a bug. It can happen when multiple wine > instances try to start up wineserver at the same time. Only the first > wineserver instance will be able to acquire the lock, all others will > exit and then connect to the existing wineserver. Sound reasonable Best regards Ralf

1 1

Re: [1/5] wsdapi: Implement WSD LinkedMemory functions [try 2]
by Alexandre Julliard April 28, 2017

April 28, 2017

Owen Rudge <orudge(a)codeweavers.com> writes: > +struct memory_allocation > +{ > + struct list entry; > + void *ptr; > + > + struct list children; > +}; > + > +static struct list memory_allocations = LIST_INIT(memory_allocations); > + > +static struct memory_allocation *find_allocation(struct list *searchList, void *parent) > +{ > + struct memory_allocation *allocation, *childAllocation; > + > + LIST_FOR_EACH_ENTRY(allocation, searchList, struct memory_allocation, entry) > + { > + if (allocation->ptr == parent) > + { > + return allocation; > + } > + > + /* Search the children */ > + childAllocation = find_allocation(&allocation->children, parent); > + > + if (childAllocation != NULL) > + { > + return childAllocation; > + } > + } > + > + return NULL; > +} You should probably store the child list at the start of the allocated block or something like that, you don't want to go through the entire list on every operation. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 4/4] mfplat/tests: Add tests.
by Nikolay Sivov April 28, 2017

April 28, 2017

On 28.04.2017 7:29, Alistair Leslie-Hughes wrote: > +BOOL init_function_ptrs(void) > +{ > + HMODULE mfplat = LoadLibraryA("mfplat.dll"); > + if (!mfplat) > + { > + win_skip("Could not load mfplat.dll\n"); > + return FALSE; > + } > + > + #define LOAD_FUNCPTR(f) p##f = (void*)GetProcAddress(mfplat, #f) > + LOAD_FUNCPTR(MFTEnum); > + LOAD_FUNCPTR(MFTRegister); > + LOAD_FUNCPTR(MFTUnregister); > + #undef LOAD_FUNCPTR > + > + return TRUE; > +} This looks redundant, why is it not statically linked?

1 0

Re: [PATCH 3/4] mfplat: Implement MFTEnum.
by Nikolay Sivov April 28, 2017

April 28, 2017

On 28.04.2017 7:28, Alistair Leslie-Hughes wrote: > + for (i = 0; i < size / sizeof(GUID); i += 2) > + { > + if (!memcmp(&guids[i], &type->guidMajorType, sizeof(GUID)) && > + !memcmp(&guids[i+1], &type->guidSubtype, sizeof(GUID))) > + { > + ret = TRUE; > + break; > + } > + } IsEqualGUID could be used instead. > +HRESULT WINAPI MFTEnum(GUID category, UINT32 flags, MFT_REGISTER_TYPE_INFO *input_type, > + MFT_REGISTER_TYPE_INFO *output_type, IMFAttributes *attributes, > + CLSID **pclsids, UINT32 *pcount) I think it's better to print a fixme for non-zero flags and attributes, and use TRACE otherwise. > + if (RegOpenKeyW(HKEY_LOCAL_MACHINE, categories_keyW, &hcategory)) > + return E_FAIL; > + > + GUIDToString(buffer, &category); > + > + ret = RegOpenKeyW(hcategory, buffer, &hlist); > + RegCloseKey(hcategory); > + if (ret) return E_FAIL; This probably could be replaced with a single RegOpenKeyW(). > + PVOID tmp; Why void*?

1 0

Re: [PATCH 2/4] mfplat: Implement MFTUnregister.
by Nikolay Sivov April 28, 2017

April 28, 2017

On 28.04.2017 7:28, Alistair Leslie-Hughes wrote: > + FIXME("(%s)\n", debugstr_guid(&clsid)); Same, why fixme?

1 0

Re: [PATCH 1/4] mfplat: Implement MFTRegister
by Nikolay Sivov April 28, 2017

April 28, 2017

On 28.04.2017 7:28, Alistair Leslie-Hughes wrote: > From: Michael Müller <michael(a)fds-team.de> > > Changes > - Added pointer check output_types, input_types > > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/mfplat/Makefile.in | 1 + > dlls/mfplat/main.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++++ > dlls/mfplat/mfplat.spec | 2 +- > loader/wine.inf.in | 4 ++ > 4 files changed, 150 insertions(+), 1 deletion(-) > > diff --git a/dlls/mfplat/Makefile.in b/dlls/mfplat/Makefile.in > index 2b5bd24..9679f53 100644 > --- a/dlls/mfplat/Makefile.in > +++ b/dlls/mfplat/Makefile.in > @@ -1,4 +1,5 @@ > MODULE = mfplat.dll > +IMPORTS = user32 advapi32 > > C_SRCS = \ > main.c > diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c > index aae81f8..424c7f4 100644 > --- a/dlls/mfplat/main.c > +++ b/dlls/mfplat/main.c > @@ -23,15 +23,44 @@ > > #include "windef.h" > #include "winbase.h" > +#include "winuser.h" > +#include "winreg.h" > > #include "initguid.h" > #include "mfapi.h" > #include "mferror.h" > > #include "wine/debug.h" > +#include "wine/unicode.h" > > WINE_DEFAULT_DEBUG_CHANNEL(mfplat); > > +static WCHAR transform_keyW[] = {'S','o','f','t','w','a','r','e','\\','C','l','a','s','s','e','s', > + '\\','M','e','d','i','a','F','o','u','n','d','a','t','i','o','n','\\', > + 'T','r','a','n','s','f','o','r','m','s',0}; > +static WCHAR categories_keyW[] = {'S','o','f','t','w','a','r','e','\\','C','l','a','s','s','e','s', > + '\\','M','e','d','i','a','F','o','u','n','d','a','t','i','o','n','\\', > + 'T','r','a','n','s','f','o','r','m','s','\\', > + 'C','a','t','e','g','o','r','i','e','s',0}; > +static WCHAR inputtypesW[] = {'I','n','p','u','t','T','y','p','e','s',0}; > +static WCHAR outputtypesW[] = {'O','u','t','p','u','t','T','y','p','e','s',0}; Shouldn't those be constants too? > +static const WCHAR szGUIDFmt[] = > +{ > + '%','0','8','x','-','%','0','4','x','-','%','0','4','x','-','%','0', > + '2','x','%','0','2','x','-','%','0','2','x','%','0','2','x','%','0','2', > + 'x','%','0','2','x','%','0','2','x','%','0','2','x',0 > +}; > + > +static LPWSTR GUIDToString(LPWSTR lpwstr, REFGUID lpcguid) > +{ > + wsprintfW(lpwstr, szGUIDFmt, lpcguid->Data1, lpcguid->Data2, > + lpcguid->Data3, lpcguid->Data4[0], lpcguid->Data4[1], > + lpcguid->Data4[2], lpcguid->Data4[3], lpcguid->Data4[4], > + lpcguid->Data4[5], lpcguid->Data4[6], lpcguid->Data4[7]); > + > + return lpwstr; > +} It's easier to use sprintfW() here. Please for consistency remove the rest of LP* types and prefixed names. > + > BOOL WINAPI DllMain(HINSTANCE instance, DWORD reason, LPVOID reserved) > { > switch (reason) > @@ -46,6 +75,121 @@ BOOL WINAPI DllMain(HINSTANCE instance, DWORD reason, LPVOID reserved) > return TRUE; > } > > +static HRESULT register_transform(CLSID *clsid, WCHAR *name, > + UINT32 cinput, MFT_REGISTER_TYPE_INFO *input_types, > + UINT32 coutput, MFT_REGISTER_TYPE_INFO *output_types) > +{ > + HKEY htransform, hclsid = 0; > + WCHAR buffer[64]; > + GUID *types; > + DWORD size; > + LONG ret; > + UINT32 i; > + > + if (RegOpenKeyW(HKEY_LOCAL_MACHINE, transform_keyW, &htransform)) > + return E_FAIL; > + > + GUIDToString(buffer, clsid); > + ret = RegCreateKeyW(htransform, buffer, &hclsid); > + RegCloseKey(htransform); > + if (ret) return E_FAIL; > + > + size = (strlenW(name) + 1) * sizeof(WCHAR); > + if (RegSetValueExW(hclsid, NULL, 0, REG_SZ, (BYTE *)name, size)) > + goto err; > + > + if (cinput && input_types) > + { > + size = 2 * cinput * sizeof(GUID); > + types = HeapAlloc(GetProcessHeap(), 0, size); > + if (!types) goto err; > + > + for (i = 0; i < cinput; i++) > + { > + memcpy(&types[2 * i], &input_types[i].guidMajorType, sizeof(GUID)); > + memcpy(&types[2 * i + 1], &input_types[i].guidSubtype, sizeof(GUID)); > + } > + > + ret = RegSetValueExW(hclsid, inputtypesW, 0, REG_BINARY, (BYTE *)types, size); > + HeapFree(GetProcessHeap(), 0, types); > + if (ret) goto err; > + } > + > + if (coutput && output_types) > + { > + size = 2 * coutput * sizeof(GUID); > + types = HeapAlloc(GetProcessHeap(), 0, size); > + if (!types) goto err; > + > + for (i = 0; i < coutput; i++) > + { > + memcpy(&types[2 * i], &output_types[i].guidMajorType, sizeof(GUID)); > + memcpy(&types[2 * i + 1], &output_types[i].guidSubtype, sizeof(GUID)); > + } > + > + ret = RegSetValueExW(hclsid, outputtypesW, 0, REG_BINARY, (BYTE *)types, size); > + HeapFree(GetProcessHeap(), 0, types); > + if (ret) goto err; > + } > + > + RegCloseKey(hclsid); > + return S_OK; > + > +err: > + RegCloseKey(hclsid); > + return E_FAIL; > +} > + > +static HRESULT register_category(CLSID *clsid, GUID *category) > +{ > + HKEY hcategory, htmp1, htmp2; > + WCHAR buffer[64]; > + DWORD ret; > + > + if (RegOpenKeyW(HKEY_LOCAL_MACHINE, categories_keyW, &hcategory)) > + return E_FAIL; > + > + GUIDToString(buffer, category); > + ret = RegCreateKeyW(hcategory, buffer, &htmp1); > + RegCloseKey(hcategory); > + if (ret) return E_FAIL; > + > + GUIDToString(buffer, clsid); > + ret = RegCreateKeyW(htmp1, buffer, &htmp2); > + RegCloseKey(htmp1); > + if (ret) return E_FAIL; > + > + RegCloseKey(htmp2); > + return S_OK; > +} > + > +/*********************************************************************** > + * MFTRegister (mfplat.@) > + */ > +HRESULT WINAPI MFTRegister(CLSID clsid, GUID category, LPWSTR name, UINT32 flags, UINT32 cinput, > + MFT_REGISTER_TYPE_INFO *input_types, UINT32 coutput, > + MFT_REGISTER_TYPE_INFO *output_types, void *attributes) This should use proper type for last argument. > +{ > + HRESULT hr; > + > + FIXME("(%s, %s, %s, %x, %u, %p, %u, %p, %p)\n", debugstr_guid(&clsid), debugstr_guid(&category), > + debugstr_w(name), flags, cinput, input_types, > + coutput, output_types, attributes); > + Why FIXME? > + if (attributes) > + FIXME("attributes not yet supported.\n"); > + > + if (flags) > + FIXME("flags not yet supported.\n"); > + > + hr = register_transform(&clsid, name, cinput, input_types, coutput, output_types); > + > + if (SUCCEEDED(hr)) > + hr = register_category(&clsid, &category); > + > + return hr; > +}

1 0

Re: [PATCH 5/9] d3dx9: Store effect creation flags in struct d3dx_base_effect.
by Matteo Bruni April 27, 2017

April 27, 2017

2017-04-27 13:05 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/effect.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c > index c2fb1d0..7885241 100644 > --- a/dlls/d3dx9_36/effect.c > +++ b/dlls/d3dx9_36/effect.c > @@ -154,6 +154,7 @@ struct d3dx9_base_effect > struct d3dx_object *objects; > > struct d3dx_effect_pool *pool; > + DWORD flags; > }; > > struct ID3DXEffectImpl > @@ -169,7 +170,7 @@ struct ID3DXEffectImpl > struct d3dx_technique *active_technique; > struct d3dx_pass *active_pass; > BOOL started; > - DWORD flags; > + DWORD begin_flags; begin_flags is quite ugly IMO. If you really want to change it from just flags, maybe call it creation_flags.

2 2

Re: [PATCH 4/9] d3dx9/tests: Add test for D3DXFX_LARGEADDRESSAWARE effect creation flag.
by Matteo Bruni April 27, 2017

April 27, 2017

2017-04-27 13:05 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/tests/effect.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index 997c1fc..a58352e 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -5743,6 +5743,30 @@ static void test_cross_effect_handle(IDirect3DDevice9 *device) > > effect2->lpVtbl->Release(effect2); > effect1->lpVtbl->Release(effect1); > + > + hr = D3DXCreateEffect(device, test_effect_preshader_effect_blob, sizeof(test_effect_preshader_effect_blob), > + NULL, NULL, D3DXFX_LARGEADDRESSAWARE, NULL, &effect1, NULL); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + param1 = effect1->lpVtbl->GetParameterByName(effect1, NULL, "g_iVect"); > + ok(!!param1, "GetParameterByName failed.\n"); > + > + hr = effect1->lpVtbl->SetValue(effect1, param1, expected_ivect, sizeof(expected_ivect)); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = effect1->lpVtbl->GetValue(effect1, param1, ivect, sizeof(ivect)); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + ok(!memcmp(ivect, expected_ivect, sizeof(expected_ivect)), "Vector value mismatch.\n"); > + > + if (0) > + { > + /* Native d3dx crashes in GetValue(). */ > + hr = effect1->lpVtbl->GetValue(effect1, "g_iVect", ivect, sizeof(ivect)); > + ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); > + } > + > + effect1->lpVtbl->Release(effect1); > } The test is okay but it doesn't feel right to put it in test_cross_effect_handle(). Probably just make a separate function.

1 0

Re: [PATCH 2/3] dwrite: Fix name section strings lookup
by Nikolay Sivov April 27, 2017

April 27, 2017

On 27.04.2017 17:03, Piotr Caban wrote: > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/dwrite/opentype.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > The problem with this patch is that we want to continue and add as many strings for different locales as possible. So the condition should set 'exists' to TRUE after first pair was added successfully and never touch it after.

2 3

Re: [PATCH 1/3] dwrite: Don't crash on uninitialized cached and factory fields in dwritefontface_Release
by Nikolay Sivov April 27, 2017

April 27, 2017

On 27.04.2017 17:03, Piotr Caban wrote: > > The function may be called with the fields uninitialized from > create_fontface when init_font_data fails. > > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/dwrite/font.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > > - factory_release_cached_fontface(This->cached); > - IDWriteFactory4_Release(This->factory); > + if (This->cached) > + factory_release_cached_fontface(This->cached); > + if (This->factory) > + IDWriteFactory4_Release(This->factory); Yes, this makes sense. The problem with this change is that 'cached' and 'factory' are not necessarily NULL on this failure path. I think at this point it would be better to use heap_alloc_zero(), and remove all explicit 0 initialization from create_fontface(). By the way, did you check why init_font_data() fails? If it's because opentype_get_font_familyname() fails, I'm interested in a log, or font file that causes it.

2 3

Re: [PATCH 3/3] dwrite: Fix EUDC font collection use after free issue
by Nikolay Sivov April 27, 2017

April 27, 2017

On 27.04.2017 17:04, Piotr Caban wrote: > > Without this patch factory_detach_fontcollection incorrectly releases > factory reference. > > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/dwrite/dwrite_private.h | 2 +- > dlls/dwrite/font.c | 6 ++++-- > dlls/dwrite/main.c | 15 ++++++++------- > 3 files changed, 13 insertions(+), 10 deletions(-) > This makes sense, thanks. Please add some test for that in test_GetEudc*, similar to what test_object_lifetime() does in the beginning - testing that collection gets factory reference, but only once.

1 0

Re: [PATCH 2/3] dwrite: Fix name section strings lookup
by Nikolay Sivov April 27, 2017

April 27, 2017

On 27.04.2017 17:03, Piotr Caban wrote: > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/dwrite/opentype.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > > - if (!(exists = opentype_decode_namerecord(header, storage_area, i, *strings))) > - continue; > + if ((exists = opentype_decode_namerecord(header, storage_area, i, *strings))) > + break; What happens exactly? I guess I'll need a font file for that too.

1 0

Re: [PATCH 1/13] msidb: Add stub tool for manipulating MSI databases.
by Hans Leidekker April 27, 2017

April 27, 2017

On Sun, 2017-04-09 at 13:12 -0600, Erich E. Hoover wrote: > +static int open_database( struct msidb_state *state ) > +{ > + LPCWSTR db_mode = state->create_database ? MSIDBOPEN_CREATEDIRECT : MSIDBOPEN_DIRECT; > + UINT ret; > + > + ret = MsiOpenDatabaseW( state->database_file, db_mode, &state->database_handle ); Databases opened in direct mode should always be committed. You should either do that in following patches, or open them in transacted mode.

2 2

Re: [4/4] ole32: Fix IDataObject_GetDataHere may not implement
by Huw Davies April 27, 2017

April 27, 2017

On Wed, Apr 26, 2017 at 03:45:50PM +0800, Wei Xie wrote: > Fix cann't insert any HBITMAP in RichEdit control > From 2c5a9b4df394c6d50bcc24bcc7b489a864224155 Mon Sep 17 00:00:00 2001 > From: Wei xie <xiewei(a)linuxdeepin.com> > Date: Wed, 26 Apr 2017 15:24:00 +0800 > Subject: [PATCH 4/4] ole32: Fix IDataObject_GetDataHere may not implement > > Fix cann't insert any HBITMAP in RichEdit control > > Signed-off-by: Wei xie <xiewei(a)linuxdeepin.com> > --- > dlls/ole32/ole2impl.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/dlls/ole32/ole2impl.c b/dlls/ole32/ole2impl.c > index a0c4445..5ee031e 100644 > --- a/dlls/ole32/ole2impl.c > +++ b/dlls/ole32/ole2impl.c > @@ -115,6 +115,10 @@ static HRESULT get_storage(IDataObject *data, IStorage *stg, UINT *src_cf, BOOL > med.u.pstg = stg; > med.pUnkForRelease = NULL; > hr = IDataObject_GetDataHere(data, &fmt, &med); > + if (FAILED(hr)) > + { > + hr = IDataObject_GetData(data, &fmt, &med); > + } This can't work. The STGMEDIUM is an [out]-only parameter of _GetData(), meaning that the callee will return a new stg in the STGMEDIUM. If this is indeed called (we need tests to check), then you'd have to copy the returned stg into the original one. Huw.

1 0

Re: [3/4] ole32: Add init cache code after OleLoad in OleCreateFromDataEx
by Huw Davies April 27, 2017

April 27, 2017

On Wed, Apr 26, 2017 at 03:46:23PM +0800, Wei Xie wrote: > Fix cann't insert any HBITMAP in RichEdit control > From a96e73f270745f0694ea340eb8119566d75f1117 Mon Sep 17 00:00:00 2001 > From: Wei xie <xiewei(a)linuxdeepin.com> > Date: Wed, 26 Apr 2017 15:14:36 +0800 > Subject: [PATCH 3/4] ole32: Add init cache code after OleLoad in > OleCreateFromDataEx > > Fix cann't insert any HBITMAP in RichEdit control > > Signed-off-by: Wei xie <xiewei(a)linuxdeepin.com> > --- > dlls/ole32/ole2impl.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/dlls/ole32/ole2impl.c b/dlls/ole32/ole2impl.c > index ccf31fa..a0c4445 100644 > --- a/dlls/ole32/ole2impl.c > +++ b/dlls/ole32/ole2impl.c > @@ -181,6 +181,7 @@ HRESULT WINAPI OleCreateFromDataEx(IDataObject *data, REFIID iid, DWORD flags, > { > HRESULT hr; > UINT src_cf; > + IOleCache* ole_cache; > > FIXME("(%p, %s, %08x, %08x, %d, %p, %p, %p, %p, %p, %p, %p): stub\n", > data, debugstr_guid(iid), flags, renderopt, num_cache_fmts, adv_flags, cache_fmts, > @@ -193,6 +194,11 @@ HRESULT WINAPI OleCreateFromDataEx(IDataObject *data, REFIID iid, DWORD flags, > if(FAILED(hr)) return hr; > > /* FIXME: Init cache */ > + hr = IUnknown_QueryInterface((IUnknown*)*obj, &IID_IOleCache, (void**)&ole_cache); > + if (SUCCEEDED(hr)) > + { > + hr = IOleCache_InitCache(ole_cache, data); > + } You'll need to release the ole_cache. Again we'll need some tests. Huw.

1 0

Re: [2/4] ole32: Support HBITMAP format in DataCache_GetExtent and DataCache_Draw
by Huw Davies April 27, 2017

April 27, 2017

On Wed, Apr 26, 2017 at 03:44:01PM +0800, Wei Xie wrote: > Fix cann't insert any HBITMAP in RichEdit control > From 91b46bc9da357b76b8028d058a191743e9a31a32 Mon Sep 17 00:00:00 2001 > From: Wei xie <xiewei(a)linuxdeepin.com> > Date: Wed, 26 Apr 2017 15:05:59 +0800 > Subject: [PATCH 2/4] ole32: Support HBITMAP format in DataCache_GetExtent and > DataCache_Draw > > Fix cann't insert any HBITMAP in RichEdit control > > Signed-off-by: Wei xie <xiewei(a)linuxdeepin.com> > --- > dlls/ole32/datacache.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 52 insertions(+) > > @@ -1908,6 +1926,40 @@ static HRESULT WINAPI DataCache_GetExtent( > > return S_OK; > } > + case CF_BITMAP: > + { > + HDC hdc; > + char dst_bmibuf[FIELD_OFFSET( BITMAPINFO, bmiColors[256] )]; > + BITMAPINFO *dst_info = (BITMAPINFO *)dst_bmibuf; > + BITMAPINFOHEADER *info; > + LONG x_pels_m, y_pels_m; > + > + if (cache_entry->stgmedium.tymed != TYMED_GDI) > + continue; > + > + hdc = GetDC(NULL); > + memset(dst_info, 0, sizeof(char)*FIELD_OFFSET( BITMAPINFO, bmiColors[256] )); > + dst_info->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); > + > + GetDIBits(hdc, cache_entry->stgmedium.u.hBitmap, 0, 1, NULL, dst_info, DIB_RGB_COLORS); > + > + info = &dst_info->bmiHeader; I'm not convinced adding the info variable helps here. Just use dst_info->bmiHeader throughout. Better still drop the 'dst_' and use BITMAPINFO *info = (BITMAPINFO *)bmibuf; > + x_pels_m = info->biXPelsPerMeter; > + y_pels_m = info->biYPelsPerMeter; > + > + /* Size in units of 0.01mm (ie. MM_HIMETRIC) */ > + if (x_pels_m != 0 && y_pels_m != 0) > + { > + lpsizel->cx = info->biWidth * 100000 / x_pels_m; > + lpsizel->cy = info->biHeight * 100000 / y_pels_m; > + } > + else > + { > + lpsizel->cx = info->biWidth * 2540 / GetDeviceCaps( hdc, LOGPIXELSX ); > + lpsizel->cy = info->biHeight * 2540 / GetDeviceCaps( hdc, LOGPIXELSY ); > + } You need a ReleaseDC(0, hdc); > + return S_OK; > + } > } > } > > -- > 2.10.2 > >

1 0

Re: [1/4] ole32: Implement DataCache_InitCache
by Huw Davies April 27, 2017

April 27, 2017

On Wed, Apr 26, 2017 at 03:42:59PM +0800, Wei Xie wrote: > Fix cann't insert any HBITMAP in RichEdit control Hi, Thanks for working on this. In general this series in going to need some unit tests to be submitted along with it. There are already some for the data-cache in dlls/ole32/tests/ole2.c. The documentation for IOleCache_InitCache() implies that it supposed to be used after first calling IOleCache_Cache() to specify which formats are to be cached, rather than the way you have implemented it. I'd imagine that OleCache_InitCache() would call IDataObject_GetData() on each of the wanted formats and pass that off to IOleCache_SetData(). Anyway, some tests will confirm this. I'll add a couple of comments to the other patches, but the tests need writing first. [Note, it's "can't" or "cannot", not "cann't"]. Huw.

1 0

Re: [PATCH 5/5] user32/tests: Try to read the dpi from the user key first
by Marvin April 27, 2017

April 27, 2017

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=30508 Your paranoid android. === w8 (32 bit sysparams) === sysparams.c:2517: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2528: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 === w1064 (32 bit sysparams) === sysparams.c:204: Test failed: Unexpected WM_DISPLAYCHANGE message

2 1

Re: [PATCH v2 08/11] d3d11/tests: Add test for state refcounting.
by Marvin April 27, 2017

April 27, 2017

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=30506 Your paranoid android. === build (build) === Patch failed to apply

1 0

Re: [PATCH 1/5] d3dx9/tests: Get rid of compare_rotation.
by Marvin April 27, 2017

April 27, 2017

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=30486 Your paranoid android. === build (build) === Patch failed to apply

2 1

Re: [PATCH 5/5] d3dx9/tests: Get rid of compare_planes.
by Marvin April 26, 2017

April 26, 2017

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=30490 Your paranoid android. === build (build) === Patch failed to apply

1 0

Re: [PATCH 4/5] d3dx9/tests: Cleanup compare_vectors.
by Marvin April 26, 2017

April 26, 2017

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=30489 Your paranoid android. === build (build) === Patch failed to apply

1 0

Re: [PATCH 3/5] d3dx9/tests: Get rid of compare_translation.
by Marvin April 26, 2017

April 26, 2017

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=30488 Your paranoid android. === build (build) === Patch failed to apply

1 0

Re: [PATCH 2/5] d3dx9/tests: Get rid of compare_scale.
by Marvin April 26, 2017

April 26, 2017

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=30487 Your paranoid android. === build (build) === Patch failed to apply

1 0

Re: [PATCH] d3dx9: Support parameters sharing in effect.
by Marvin April 26, 2017

April 26, 2017

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=30483 Your paranoid android. === build (build) === Patch failed to apply

1 0

Re: [PATCH] d3dx9/tests: Cleanup expect_vec3.
by Marvin April 26, 2017

April 26, 2017

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=30482 Your paranoid android. === build (build) === Patch failed to apply

1 0

Re: [PATCH 1/13] msidb: Add stub tool for manipulating MSI databases.
by Hans Leidekker April 26, 2017

April 26, 2017

On Sun, 2017-04-09 at 13:12 -0600, Erich E. Hoover wrote: > + /* process and validate all the command line flags */ > + for (i = 1; n && i < argc; i += n) > + n = process_argument( &state, i, argc, argv ); > + if (!valid_state( &state )) > + return 1; > + > + /* perform the requested operations */ > + if (!open_database( &state )) > + { > + ERR( "Failed to open database '%s'.\n", wine_dbgstr_w(state.database_file) ); For programs it should be WINE_ERR, etc.

2 1

Re: po: Update Simplified Chinese translation.
by Alexandre Julliard April 26, 2017

April 26, 2017

Jactry Zeng <jzeng(a)codeweavers.com> writes: > diff --git a/po/zh_CN.po b/po/zh_CN.po > index 45103bf3e4..f2addad1a5 100644 > --- a/po/zh_CN.po > +++ b/po/zh_CN.po > @@ -27,7 +27,7 @@ msgstr "Š»œçŒ¡ç®£çºŒ¯æ'®£'" > > #: appwiz.rc:62 > msgid "&Install..." > -msgstr "®£(&I)" > +msgstr "®£(&I)..." Is it really necessary to change all the ellipsis characters? They were changed the other way around not so long ago. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [v4 PATCH] odbccp32: Handle ODBC_CONFIG_DRIVER request in SQLConfigDriver/W
by Huw Davies April 26, 2017

April 26, 2017

On Wed, Apr 26, 2017 at 12:46:49AM +0000, Alistair Leslie-Hughes wrote: > diff --git a/dlls/odbccp32/odbccp32.c b/dlls/odbccp32/odbccp32.c > index f28d464..475b74e 100644 > --- a/dlls/odbccp32/odbccp32.c > +++ b/dlls/odbccp32/odbccp32.c > @@ -65,6 +65,7 @@ static const WCHAR odbc_error_invalid_param_string[] = {'I','n','v','a','l','i', > static const WCHAR odbc_error_invalid_dsn[] = {'I','n','v','a','l','i','d',' ','D','S','N',0}; > static const WCHAR odbc_error_load_lib_failed[] = {'L','o','a','d',' ','L','i','b','r','a','r','y',' ','F','a','i','l','e','d',0}; > static const WCHAR odbc_error_request_failed[] = {'R','e','q','u','e','s','t',' ','F','a','i','l','e','d',0}; > +static const WCHAR odbc_error_invalid_keyword[] = {'I','n','v','a','l','i','d',' ','k','e','y','w','o','r','d',' ','v','a','l','u','e',0}; > > /* Push an error onto the error stack, taking care of ranges etc. */ > static void push_error(int code, LPCWSTR msg) > @@ -303,6 +304,64 @@ static HMODULE load_config_driver(const WCHAR *driver) > return hmod; > } > > +static BOOL write_config_value(const WCHAR *driver, const WCHAR *args) > +{ > + long ret; > + HKEY hkey, hkeydriver; > + WCHAR *name = NULL; > + > + if (!args) > + return FALSE; > + > + if ((ret = RegOpenKeyW(HKEY_LOCAL_MACHINE, odbcini, &hkey)) == ERROR_SUCCESS) > + { > + if ((ret = RegOpenKeyW(hkey, driver, &hkeydriver)) == ERROR_SUCCESS) > + { > + WCHAR *divider, *value; > + > + name = heap_alloc( (strlenW(args) + 1) * sizeof(WCHAR)); > + if(!name) > + { > + push_error(ODBC_ERROR_OUT_OF_MEM, odbc_error_out_of_mem); > + goto fail; > + } > + lstrcpyW(name, args); > + > + divider = strchrW(name,'='); > + if(!divider) > + { > + push_error(ODBC_ERROR_INVALID_KEYWORD_VALUE, odbc_error_invalid_keyword); > + goto fail; > + } > + > + value = divider + 1; > + *divider = '\0'; > + > + TRACE("Write pair: %s = %s\n", debugstr_w(name), debugstr_w(value)); > + if(RegSetValueExW(hkeydriver, name, 0, REG_SZ, (BYTE*)value, > + strlenW(value) * sizeof(WCHAR)) != ERROR_SUCCESS) The length should be (strlenW(value) + 1) * sizeof(WCHAR). Sorry, I missed that last time. While you're at it, decide whether you're going to use a space after 'if' or not, and then be consistent. Otherwise, this looks good. Huw.

1 0

Re: [PATCH v2 1/2] mountmgr: Create devices and registry entries for serial ports.
by Alexandre Julliard April 26, 2017

April 26, 2017

Alex Henrie <alexhenrie24(a)gmail.com> writes: > + if (driver == serial_driver) > + { > + dos_name_format = comW; > + nt_name_format = device_serialW; > + reg_value_format = comW; > + } > + else > + { > + /* TODO: support parallel ports */ > + } Please avoid introducing dead code. You can add the if() in the second patch. > + /* remove old symlinks */ > + for (n = 1; n <= MAX_PORTS; n++) > + { > + sprintf( p, "%u", n ); > + if (unlink( dosdevices_path ) != 0 && !errno) > + break; > + } Why would errno ever be 0 here? -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: dmscript/tests: Use a void* instead of void** outer IUnknown argument
by Marvin April 25, 2017

April 25, 2017

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=30436 Your paranoid android. === wxppro (32 bit dmscript) === dmscript.c:146: Test failed: DirectMusicScriptTrack create failed: 80004003, expected CLASS_E_NOAGGREGATION === wvistau64 (32 bit dmscript) === 01f4:dmscript: unhandled exception c0000005 at 73E47D60 === w7u (32 bit dmscript) === 0ac8:dmscript: unhandled exception c0000005 at 72157EA8 === w7pro64 (32 bit dmscript) === 0978:dmscript: unhandled exception c0000005 at 73907EA8 === w8 (32 bit dmscript) === 0938:dmscript: unhandled exception c0000005 at 69E4804C === w864 (32 bit dmscript) === 0f8c:dmscript: unhandled exception c0000005 at 7451804C === w1064 (32 bit dmscript) === 02bc:dmscript: unhandled exception c0000005 at 7078945E

2 1

Re: [v2 6/6] d3dx9: Support parameters sharing in effect.
by Matteo Bruni April 25, 2017

April 25, 2017

2017-04-24 13:36 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +struct d3dx_shared_data > +{ > + void *data; > + LONG refcount; > + struct d3dx_parameter **parameters; > + unsigned int table_size; > +}; A bit nitpicky but generally for those dynamic arrays we call "refcount" "count" and "table_size" "size". > + new_refcount = InterlockedIncrement(&pool->shared_data[i].refcount); I think you don't need the Interlocked functions for this count given that adding / removing multiple effects to a pool is expected to be racy. > + if (new_refcount >= pool->shared_data[i].table_size) > + { > + if (!pool->shared_data[i].table_size) > + { > + new_size = INITIAL_SHARED_DATA_TABLE_SIZE; > + pool->shared_data[i].parameters = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > + sizeof(*pool->shared_data[i].parameters) * INITIAL_SHARED_DATA_TABLE_SIZE); > + } > + else > + { > + new_size = pool->shared_data[i].table_size * 2; > + pool->shared_data[i].parameters = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > + pool->shared_data[i].parameters, > + sizeof(*pool->shared_data[i].parameters) * new_size); > + } > + pool->shared_data[i].table_size = new_size; > + } > + > + param->shared_data = &pool->shared_data[i]; > + pool->shared_data[i].parameters[new_refcount - 1] = param; > + > + TRACE("name %s, parameter idx %u, new refcount %u.\n", param->name, i, > + new_refcount); > + return; > +} Unnecessary return. > +static BOOL param_zero_data_func(void *dummy, struct d3dx_parameter *param) > +{ > + param->data = NULL; > + return FALSE; > +} > + > +static void d3dx_pool_release_shared_parameter(struct d3dx_parameter *param) > +{ > + LONG new_refcount; > + > + if (!(param->flags & PARAMETER_FLAG_SHARED) || !param->shared_data) > + return; No need to check for the PARAMETER_FLAG_SHARED flag. Maybe it makes sense to assert that the flag is there if param->shared_data is not NULL. > static void free_effect_pool(struct d3dx_effect_pool *pool) > { > + unsigned int i; > + > + for (i = 0; i < pool->table_size; ++i) > + { > + if (pool->shared_data[i].refcount) > + ERR("Releasing pool with referenced parameters, shared data is not freed.\n"); > + } This can be triggered by an application so it should be a WARN(). BTW, this probably needs a test. On pool destruction, the effects still using it might be "unshared" i.e. data from the pool might be moved back to the effects. Or maybe the effects still using the pool are simply broken and accessing parameter data after the pool is destroyed crashes.

2 6

Re: [PATCH 2/5] ddraw: Ignore unsupported blit flags.
by Matteo Bruni April 25, 2017

April 25, 2017

2017-04-24 23:31 GMT+02:00 Henri Verbeet <hverbeet(a)codeweavers.com>: > Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com> > --- > dlls/ddraw/surface.c | 24 +++++++++++++++++++++--- > dlls/ddraw/tests/ddraw1.c | 4 ++-- > dlls/ddraw/tests/ddraw2.c | 4 ++-- > dlls/ddraw/tests/ddraw4.c | 4 ++-- > dlls/ddraw/tests/ddraw7.c | 4 ++-- > 5 files changed, 29 insertions(+), 11 deletions(-) > > diff --git a/dlls/ddraw/surface.c b/dlls/ddraw/surface.c > index 6c4ed11..90731f0 100644 > --- a/dlls/ddraw/surface.c > +++ b/dlls/ddraw/surface.c > @@ -1570,6 +1570,7 @@ static HRESULT WINAPI DECLSPEC_HOTPATCH ddraw_surface7_Blt(IDirectDrawSurface7 * > struct ddraw_surface *dst_impl = impl_from_IDirectDrawSurface7(iface); > struct ddraw_surface *src_impl = unsafe_impl_from_IDirectDrawSurface7(src_surface); > struct wined3d_blt_fx wined3d_fx; > + DWORD unsupported_flags; > DWORD fill_colour = 0; > HRESULT hr = DD_OK; > DDBLTFX rop_fx; > @@ -1577,9 +1578,26 @@ static HRESULT WINAPI DECLSPEC_HOTPATCH ddraw_surface7_Blt(IDirectDrawSurface7 * > TRACE("iface %p, dst_rect %s, src_surface %p, src_rect %s, flags %#x, fx %p.\n", > iface, wine_dbgstr_rect(dst_rect), src_surface, wine_dbgstr_rect(src_rect), flags, fx); > > - /* Check for validity of the flags here. WineD3D Has the software-opengl selection path and would have > - * to check at 2 places, and sometimes do double checks. This also saves the call to wined3d :-) > - */ > + unsupported_flags = DDBLT_ALPHADEST > + | DDBLT_ALPHADESTCONSTOVERRIDE > + | DDBLT_ALPHADESTNEG > + | DDBLT_ALPHADESTSURFACEOVERRIDE > + | DDBLT_ALPHAEDGEBLEND > + | DDBLT_ALPHASRC > + | DDBLT_ALPHASRCCONSTOVERRIDE > + | DDBLT_ALPHASRCNEG > + | DDBLT_ALPHASRCSURFACEOVERRIDE > + | DDBLT_ZBUFFER > + | DDBLT_ZBUFFERDESTCONSTOVERRIDE > + | DDBLT_ZBUFFERDESTOVERRIDE > + | DDBLT_ZBUFFERSRCCONSTOVERRIDE > + | DDBLT_ZBUFFERSRCOVERRIDE; Hardly going to matter but wouldn't it be better as a static const?

2 1

Re: [v4 PATCH 2/2] wined3d: Update only dirty regions in wined3d_device_update_texture().
by Marvin April 25, 2017

April 25, 2017

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=30446 Your paranoid android. === w1064 (64 bit visual) === visual.c:7941: Test failed: Input test: Quad 3(2crd-wrongidx) returned color 0x00ff00ff, expected 0x00ff0080 visual.c:7989: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors).

1 0

Re: [v2 4/6] d3dx9: Reference effect pool implementation in d3dx9_base_effect structure.
by Matteo Bruni April 25, 2017

April 25, 2017

2017-04-24 13:36 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > /*** IUnknown methods ***/ > -static HRESULT WINAPI ID3DXEffectPoolImpl_QueryInterface(ID3DXEffectPool *iface, REFIID riid, void **object) > +static HRESULT WINAPI d3dx_effect_pool_QueryInterface(ID3DXEffectPool *iface, REFIID riid, void **object) > { > TRACE("(%p)->(%s, %p)\n", iface, debugstr_guid(riid), object); Please update / cleanup this TRACE too. > -static ULONG WINAPI ID3DXEffectPoolImpl_AddRef(ID3DXEffectPool *iface) > +static ULONG WINAPI d3dx_effect_pool_AddRef(ID3DXEffectPool *iface) > { > - struct ID3DXEffectPoolImpl *This = impl_from_ID3DXEffectPool(iface); > + struct d3dx_effect_pool *pool = impl_from_ID3DXEffectPool(iface); > > - TRACE("(%p)->(): AddRef from %u\n", This, This->ref); > + TRACE("iface %p, pool %p, ref %u.\n", iface, pool, pool->ref); Tracing both iface and pool doesn't seem useful and I don't think I've ever seen it anywhere else. Also for AddRef() we usually go with a TRACE in the form: TRACE("%p increasing refcount to %u.\n", iface, refcount); Similarly for Release(). Have a look at recent implementations of the IUnknown functions like e.g. d3d11_hull_shader_* in dlls/d3d11/shader.c.

1 0

Re: [PATCH 1/4] comdlg32: Release the returned imagelist.
by Huw Davies April 25, 2017

April 25, 2017

On Tue, Apr 25, 2017 at 09:04:56AM +0100, Huw Davies wrote: > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > --- > dlls/comdlg32/filedlg.c | 2 ++ > 1 file changed, 2 insertions(+) Actually, while this is correct, could you skip it for now? I'm going to have to make other changes to this area, so it would be better to hold off committing this one. The other patches in the series are still good. Huw.

1 0

Re: [PATCH 2/4] regedit: Simplify parseKeyName()
by Alexandre Julliard April 25, 2017

April 25, 2017

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > - > - for (i = 0; i < ARRAY_SIZE(reg_class_keys); i++) { > - if (CompareStringW(LOCALE_USER_DEFAULT, 0, lpKeyName, len, reg_class_namesW[i], -1) == CSTR_EQUAL && > - len == lstrlenW(reg_class_namesW[i])) { > + num_class_keys = ARRAY_SIZE(reg_class_keys); > + for (i = 0; i < num_class_keys; i++) > + { > + if (!strncmpW(lpKeyName, reg_class_namesW[i], lstrlenW(reg_class_namesW[i]))) > + { You still need to check that the lengths match. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH] wined3d: Check pointer before use
by Józef Kucia April 25, 2017

April 25, 2017

On Tue, Apr 25, 2017 at 8:01 AM, Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > Test was provided by Józef Kucia. > > Fixes https://bugs.winehq.org/show_bug.cgi?id=42873 > > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> > --- > dlls/d3d11/tests/d3d11.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++ > dlls/wined3d/context.c | 2 +- > 2 files changed, 59 insertions(+), 1 deletion(-) > > diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c > index dfe50c6287..2961cc06cb 100644 > --- a/dlls/d3d11/tests/d3d11.c > +++ b/dlls/d3d11/tests/d3d11.c > @@ -17567,6 +17567,63 @@ done: > release_test_context(&test_context); > } > > +static void test_destroyed_wined3d_context_query_poll(void) FWIW, I'm not sure if we want to use this name for the test. I'm sorry that I haven't mentioned this earlier in the bug report. > --- a/dlls/wined3d/context.c > +++ b/dlls/wined3d/context.c > @@ -3968,7 +3968,7 @@ struct wined3d_context *context_reacquire(const struct wined3d_device *device, > { > struct wined3d_context *current_context; > > - if (context->tid != GetCurrentThreadId()) > + if (!context || context->tid != GetCurrentThreadId()) > return NULL; > > current_context = context_acquire(device, context->current_rt.texture, I think it's fine to do a NULL check in context_reacquire(), and it should be good enough in practice (i.e. this corner case shouldn't matter much). However we may want to handle this correctly in the future. I think it may be good idea to add a similar test for D3D11_QUERY_OCCLUSION (or replace this one). For the occlusion query we want to additionally test the result of the query which is expected to be incorrect (todo_wine).

1 0

Re: [PATCH] programs/winecfg: enable CSMT checkbox on Graphics tab
by Henri Verbeet April 25, 2017

April 25, 2017

On 24 April 2017 at 23:01, Donat Enikeev <donat(a)enikeev.net> wrote: > @@ -181,6 +181,9 @@ BEGIN > EDITTEXT IDC_RES_DPIEDIT,188,105,23,13,ES_NUMBER|WS_TABSTOP > LTEXT "#msgctxt#unit: dots/inch#dpi",IDC_STATIC,215,107,30,8 > LTEXT "This is a sample text using 10 point Tahoma",IDC_RES_FONT_PREVIEW,15,124,230,49 > + > + GROUPBOX "CSMT",IDC_STATIC,8,186,244,32 > + CONTROL "Enable multi-threaded command stream in Direct3D",IDC_ENABLE_CSMT,"Button",BS_AUTOCHECKBOX | WS_TABSTOP,15,202,230,8 > END > > IDD_DLLCFG DIALOG 0, 0, 260, 220 I don't think so, this is not a setting regular users should be modifying.

2 3

`setupapi` patch from staging
by Donat Enikeev April 24, 2017

April 24, 2017

Hi Guys, Number of recent well-known games are based on Starling game engine (Adobe Air+Stage3D for gpu rendering), such as: The Banner Saga 2, Samorost 3, The Inner World, Angry Birds. They are are falling into Software rendering mode on plain Wine and thus barely playable (bug #41635) Thanks to Béla Gyebrószki finding @ bug #41653, Wine makes these games consider d3d9 rendering with the following single patch from wine-staging https://github.com/wine-compholio/wine-staging/blob/master/patches/setupapi… This patch implements support of the case when a full driver path is passed to SetupDiGetClassDevs (trace successfully reaching AddDeviceToSet for a game below [1]). I was unable to find @wine-devel discussion around this patch, I would be grateful if one could kindly remind why it was staged, probably it just requires refactoring around the change or additional tests? I am asking this, because Adobe stopped Linux support for the Air years ago and we could never see such games ported, and therefore plain Wine's support of gpu acceleration for such games looks important. Thanks in advance Best regards, Donnie [1] 0009:trace:setupapi:SetupDiGetClassDevsExW {4d36e968-e325-11ce-bfc1-08002be10318} L"PCI\\VEN_0000&DEV_0000" (nil) 0x0000000a (nil) (null) (nil) 0009:warn:setupapi:SetupDiGetClassDevsExW unsupported flags 0000000a 0009:trace:setupapi:SetupDiCreateDeviceInfoListExW {4d36e968-e325-11ce-bfc1-08002be10318} (nil) (null) (nil) 0009:trace:setupapi:SETUPDI_EnumerateDevices 0x464ac90, {4d36e968-e325-11ce-bfc1-08002be10318}, L"PCI\\VEN_0000&DEV_0000", 0000000a 0009:trace:setupapi:SETUPDI_EnumerateMatchingDeviceInstances L"PCI" L"VEN_0000&DEV_0000" 0009:trace:setupapi:SETUPDI_AddDeviceToSet 0x464ac90, {4d36e968-e325-11ce-bfc1-08002be10318}, 0, L"PCI\\VEN_0000&DEV_0000\\13&12345&0", 0

3 6

Re: [PATCH] wined3d: Add Nvidia TitanX (Pascal)
by Henri Verbeet April 24, 2017

April 24, 2017

On 23 April 2017 at 21:12, Christopher Berner <christopherberner(a)gmail.com> wrote: > + {HW_VENDOR_NVIDIA, CARD_NVIDIA_TITANX_PASCAL, "NVIDIA TITAN X (Pascal)", DRIVER_NVIDIA_GEFORCE8, 12288}, Is that really what it's called by the Windows driver?

2 1

Re: [PATCH] shell32/ebrowser: Added IOleWindow stub for IExplorerBrowser
by Huw Davies April 24, 2017

April 24, 2017

On Mon, Apr 24, 2017 at 08:50:09AM +0300, Nikolay Sivov wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/shell32/ebrowser.c | 56 +++++++++++++++++++++++++++++++++++++++++++ > dlls/shell32/tests/ebrowser.c | 2 +- > 2 files changed, 57 insertions(+), 1 deletion(-) > > diff --git a/dlls/shell32/ebrowser.c b/dlls/shell32/ebrowser.c > index bbccf88814..77c727a5ec 100644 > --- a/dlls/shell32/ebrowser.c > +++ b/dlls/shell32/ebrowser.c > @@ -56,6 +56,7 @@ typedef struct _ExplorerBrowserImpl { > IShellBrowser IShellBrowser_iface; > ICommDlgBrowser3 ICommDlgBrowser3_iface; > IObjectWithSite IObjectWithSite_iface; > + IOleWindow IOleWindow_iface; > INameSpaceTreeControlEvents INameSpaceTreeControlEvents_iface; > IInputObject IInputObject_iface; > LONG ref; IShellBrowser inherits IOleWindow, so you don't need this. Huw.

2 1

Re: [PATCH] comctl32/taskdialog: Implement buttons.
by Nikolay Sivov April 24, 2017

April 24, 2017

On 23.03.2017 21:22, Fabian Maurer wrote: > Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de> > --- > dlls/comctl32/taskdialog.c | 224 ++++++++++++++++++++++++++++++++------------- > 1 file changed, 161 insertions(+), 63 deletions(-) > > diff --git a/dlls/comctl32/taskdialog.c b/dlls/comctl32/taskdialog.c > index 6b8bcfcae6..037b85e082 100644 > --- a/dlls/comctl32/taskdialog.c > +++ b/dlls/comctl32/taskdialog.c > @@ -41,10 +41,13 @@ WINE_DEFAULT_DEBUG_CHANNEL(taskdialog); > #define ALIGN_LENGTH(_Len, _Align) _Len = ALIGNED_LENGTH(_Len, _Align) > #define ALIGN_POINTER(_Ptr, _Align) _Ptr = ALIGNED_POINTER(_Ptr, _Align) > > -static const UINT DIALOG_MIN_WIDTH = 180; > +static const UINT DIALOG_MIN_WIDTH = 240; > static const UINT DIALOG_SPACING = 5; > -static const UINT DIALOG_BUTTON_WIDTH = 50; > -static const UINT DIALOG_BUTTON_HEIGHT = 14; > +static const UINT DIALOG_BUTTON_WIDTH_MIN = 45; > +static const UINT DIALOG_BUTTON_HEIGHT = 13; This was taken from MSGBOX template, what's a point in making it different? > +static const UINT DIALOG_SPACING_BUTTONS_LEFT = 30; /* minimum distance from the left dialog border */ > +static const UINT DIALOG_SPACING_BUTTON_H = 1; /* Distance between buttons */ > +static const UINT DIALOG_SPACING_BUTTON_W = 4; /* Distance between buttons */ > > static const UINT ID_MAIN_INSTRUCTION = 0xf000; > > @@ -67,6 +70,15 @@ struct taskdialog_template_desc > HFONT font; > }; > > +typedef struct > +{ > + int id; > + const WCHAR *text; > + UINT width; > + UINT x; > + UINT y; > +} button_info; > + > static void pixels_to_dialogunits(const struct taskdialog_template_desc *desc, LONG *width, LONG *height) > { > if (width) > @@ -83,6 +95,44 @@ static void dialogunits_to_pixels(const struct taskdialog_template_desc *desc, L > *height = MulDiv(*height, desc->y_baseunit, 8); > } > > +/* used to calculate size for the controls */ > +static RECT text_get_rect(const struct taskdialog_template_desc *desc, const WCHAR *text) > +{ > + const WCHAR *textW = NULL; > + unsigned int length; > + HFONT oldfont; > + HDC hdc; > + static const WCHAR nulW; > + RECT rect = { 0, 0, desc->dialog_width - DIALOG_SPACING * 2, 0}; /* padding left and right of the control */ > + > + if (IS_INTRESOURCE(text)) > + { > + if (!(length = LoadStringW(desc->taskconfig->hInstance, (UINT_PTR)text, (WCHAR *)&textW, 0))) > + { > + WARN("Failed to load text\n"); > + textW = &nulW; > + length = 0; > + } > + } > + else > + { > + textW = text; > + length = strlenW(textW); > + } > + > + hdc = GetDC(0); > + oldfont = SelectObject(hdc, desc->font); > + > + dialogunits_to_pixels(desc, &rect.right, NULL); > + DrawTextW(hdc, textW, length, &rect, DT_LEFT | DT_EXPANDTABS | DT_CALCRECT | DT_WORDBREAK); > + pixels_to_dialogunits(desc, &rect.right, &rect.bottom); > + > + SelectObject(hdc, oldfont); > + ReleaseDC(0, hdc); > + > + return rect; > +} > + Did you test that buttons are supposed to support multiple lines of text? > static void template_write_data(char **ptr, const void *src, unsigned int size) > { > memcpy(*ptr, src, size); > @@ -90,7 +140,7 @@ static void template_write_data(char **ptr, const void *src, unsigned int size) > } > > static unsigned int taskdialog_add_control(struct taskdialog_template_desc *desc, WORD id, const WCHAR *class, > - HINSTANCE hInstance, const WCHAR *text, short x, short y, short cx, short cy) > + DWORD style, HINSTANCE hInstance, const WCHAR *text, short x, short y, short cx, short cy) > { > struct taskdialog_control *control = Alloc(sizeof(*control)); > unsigned int size, class_size, text_size; > @@ -117,7 +167,7 @@ static unsigned int taskdialog_add_control(struct taskdialog_template_desc *desc > control->template = template = Alloc(size); > control->template_size = size; > > - template->style = WS_VISIBLE; > + template->style = WS_VISIBLE | WS_CHILD | style; You don't use styles in this patch, this is unnecessary change. > template->dwExtendedStyle = 0; > template->x = x; > template->y = y; > @@ -136,78 +186,126 @@ static unsigned int taskdialog_add_control(struct taskdialog_template_desc *desc > > static unsigned int taskdialog_add_main_instruction(struct taskdialog_template_desc *desc) > { > - RECT rect = { 0, 0, desc->dialog_width - DIALOG_SPACING * 2, 0}; /* padding left and right of the control */ > - const WCHAR *textW = NULL; > - unsigned int size, length; > - HFONT oldfont; > - HDC hdc; > + unsigned int size; > + RECT rect; > > if (!desc->taskconfig->pszMainInstruction) > return 0; > > - if (IS_INTRESOURCE(desc->taskconfig->pszMainInstruction)) > - { > - if (!(length = LoadStringW(desc->taskconfig->hInstance, (UINT_PTR)desc->taskconfig->pszMainInstruction, > - (WCHAR *)&textW, 0))) > - { > - WARN("Failed to load main instruction text\n"); > - return 0; > - } > - } > - else > - { > - textW = desc->taskconfig->pszMainInstruction; > - length = strlenW(textW); > - } > - > - hdc = GetDC(0); > - oldfont = SelectObject(hdc, desc->font); > - > - dialogunits_to_pixels(desc, &rect.right, NULL); > - DrawTextW(hdc, textW, length, &rect, DT_LEFT | DT_EXPANDTABS | DT_CALCRECT | DT_WORDBREAK); > - pixels_to_dialogunits(desc, &rect.right, &rect.bottom); > - > - SelectObject(hdc, oldfont); > - ReleaseDC(0, hdc); > + rect = text_get_rect(desc, desc->taskconfig->pszMainInstruction); > > - size = taskdialog_add_control(desc, ID_MAIN_INSTRUCTION, WC_STATICW, desc->taskconfig->hInstance, > + size = taskdialog_add_control(desc, ID_MAIN_INSTRUCTION, WC_STATICW, 0, desc->taskconfig->hInstance, > desc->taskconfig->pszMainInstruction, DIALOG_SPACING, desc->dialog_height, > rect.right, rect.bottom); > - desc->dialog_height += rect.bottom; > + desc->dialog_height += rect.bottom + DIALOG_SPACING; > return size; > } > > -static unsigned int taskdialog_add_common_buttons(struct taskdialog_template_desc *desc) > +static button_info make_button(struct taskdialog_template_desc *desc, int id, const WCHAR *text) > +{ > + RECT rect; > + button_info button; > + > + button.id = id; > + button.text = text; > + rect = text_get_rect(desc, text); > + button.width = rect.right + 10; > + if (button.width < DIALOG_BUTTON_WIDTH_MIN) > + button.width = DIALOG_BUTTON_WIDTH_MIN; > + > + return button; > +} This does not handle "text" correctly. > + > +static unsigned int taskdialog_add_buttons(struct taskdialog_template_desc *desc) > { > - short button_x = desc->dialog_width - DIALOG_BUTTON_WIDTH - DIALOG_SPACING; > DWORD flags = desc->taskconfig->dwCommonButtons; > + const TASKDIALOGCONFIG *taskconfig = desc->taskconfig; > + UINT alignment = DIALOG_SPACING_BUTTONS_LEFT; /* minimum distance from the left dialog border */ > + UINT location_x; > + BOOL first_row = TRUE; > + button_info *buttons; > + int count = 0; > + int i; > unsigned int size = 0; > > -#define TASKDIALOG_ADD_COMMON_BUTTON(id) \ > - do { \ > - size += taskdialog_add_control(desc, ID##id, WC_BUTTONW, COMCTL32_hModule, MAKEINTRESOURCEW(IDS_BUTTON_##id), \ > - button_x, desc->dialog_height + DIALOG_SPACING, DIALOG_BUTTON_WIDTH, DIALOG_BUTTON_HEIGHT); \ > - button_x -= DIALOG_BUTTON_WIDTH + DIALOG_SPACING; \ > - } while(0) > - if (flags & TDCBF_CLOSE_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(CLOSE); > - if (flags & TDCBF_CANCEL_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(CANCEL); > - if (flags & TDCBF_RETRY_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(RETRY); > - if (flags & TDCBF_NO_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(NO); > - if (flags & TDCBF_YES_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(YES); > + /* Allocate enough memory for the custom and the default buttons */ > + if (taskconfig->cButtons && taskconfig->pButtons) > + buttons = HeapAlloc(GetProcessHeap(), 0, (taskconfig->cButtons + 6) * sizeof(button_info)); > + else > + { > + buttons = HeapAlloc(GetProcessHeap(), 0, 6 * sizeof(button_info)); > + } > + > + /* Custom buttons */ > + if (taskconfig->cButtons && taskconfig->pButtons) > + { > + for (i = 0; i < taskconfig->cButtons; i++) > + { > + buttons[count++] = make_button(desc, taskconfig->pButtons[i].nButtonID, taskconfig->pButtons[i].pszButtonText); > + } > + } > + > + /* Default buttons */ > if (flags & TDCBF_OK_BUTTON) > - TASKDIALOG_ADD_COMMON_BUTTON(OK); > - /* Always add OK button */ > - if (list_empty(&desc->controls)) > - TASKDIALOG_ADD_COMMON_BUTTON(OK); > -#undef TASKDIALOG_ADD_COMMON_BUTTON > - > - /* make room for common buttons row */ > - desc->dialog_height += DIALOG_BUTTON_HEIGHT + 2 * DIALOG_SPACING; > + buttons[count++] = make_button(desc, IDOK, MAKEINTRESOURCEW(IDS_BUTTON_OK)); > + if (flags & TDCBF_YES_BUTTON) > + buttons[count++] = make_button(desc, IDYES, MAKEINTRESOURCEW(IDS_BUTTON_YES)); > + if (flags & TDCBF_NO_BUTTON) > + buttons[count++] = make_button(desc, IDNO, MAKEINTRESOURCEW(IDS_BUTTON_NO)); > + if (flags & TDCBF_RETRY_BUTTON) > + buttons[count++] = make_button(desc, IDRETRY, MAKEINTRESOURCEW(IDS_BUTTON_RETRY)); > + if (flags & TDCBF_CANCEL_BUTTON) > + buttons[count++] = make_button(desc, IDCANCEL, MAKEINTRESOURCEW(IDS_BUTTON_CANCEL)); > + if (flags & TDCBF_CLOSE_BUTTON) > + buttons[count++] = make_button(desc, IDCLOSE, MAKEINTRESOURCEW(IDS_BUTTON_CLOSE)); > + > + /* There must be at least one button */ > + if (count == 0) > + buttons[count++] = make_button(desc, IDOK, MAKEINTRESOURCEW(IDS_BUTTON_OK)); > + > + /* Position buttons */ > + location_x = alignment; > + for (i = 0; i < count; i++) > + { > + /* When beginning new row, align the first */ > + if (location_x + buttons[i].width + DIALOG_SPACING_BUTTON_W > desc->dialog_width) > + { > + if (first_row) > + { > + int diff = desc->dialog_width - location_x; > + > + first_row = FALSE; > + for (int j = 0; j < i; j++) /* Align first row to the right */ > + buttons[j].x += diff; > + alignment = buttons[0].x; /* left-align all coming rows to the first row */ > + } > + location_x = alignment; > + desc->dialog_height += DIALOG_BUTTON_HEIGHT + DIALOG_SPACING_BUTTON_H; > + } > + > + buttons[i].x = location_x; > + buttons[i].y = desc->dialog_height; > + > + location_x += buttons[i].width + DIALOG_SPACING_BUTTON_W; > + } > + if (first_row) /* Always align first row to the right */ > + { > + int diff = desc->dialog_width - (buttons[count - 1].x + buttons[count - 1].width + DIALOG_SPACING_BUTTON_W); > + for (int i = 0; i < count; i++) > + buttons[i].x += diff; > + } I don't like this. Why do you need temporary array, and iterating twice seems excessive. > + > + /* Now that we got them all positioned, create all buttons */ > + for (i = 0; i < count; i++) > + { > + size += taskdialog_add_control(desc, buttons[i].id, WC_BUTTONW, BS_PUSHBUTTON, COMCTL32_hModule, > + buttons[i].text, buttons[i].x, buttons[i].y, buttons[i].width, DIALOG_BUTTON_HEIGHT); > + } This won't work for custom buttons. > + > + /* Add height for last row and spacing */ > + desc->dialog_height += DIALOG_BUTTON_HEIGHT + DIALOG_SPACING; > + > + HeapFree(GetProcessHeap(), 0, buttons); > return size; > } > > @@ -295,7 +393,7 @@ static DLGTEMPLATE *create_taskdialog_template(const TASKDIALOGCONFIG *taskconfi > desc.dialog_width = min(desc.dialog_width, screen_width); > > size += taskdialog_add_main_instruction(&desc); > - size += taskdialog_add_common_buttons(&desc); > + size += taskdialog_add_buttons(&desc); I'd rather have it separated if possible. I'll do some testing myself, before commenting further. > > template = Alloc(size); > if (!template) >

2 8

Build server and Staging patch submission
by Michael Müller April 24, 2017

April 24, 2017

Hi all, I would like to announce some changes regarding the build server and the patch submission for Wine Staging. As already mentioned in the last email about the packaging changes, one of the main ideas was to implement an easy to use web interface, so that wine developers can create their own builds. As you can most probably imagine, such a system is quite complex (updating VMs, building packages, signing packages, updating repos, synchronizing files, ...) but I think we have now reached a state in which we can put the system into a public beta. There are some tasks left (like adding support for FreeBSD), but the main system works and was already used to create the 2.5, 2.6 and 2.0.1 WineHQ packages. I have created a wiki article which describes how to get access to the builder and basically everything else you need to know. You can find it at https://wiki.winehq.org/Build_Server. There is another change we would like to announce. Since Wine Staging was merged into WineHQ, there was no longer an official way to suggest patches. This lead to a non-transparent system of private emails, requests in bug reports etc. This is not something you would expect from an open source project, so we took the opportunity and also included a patch tracker into the website. Submissions and comments are now public for everyone and you can also subscribe to changes. For more information about the system and how to submit patches, take look at https://wiki.winehq.org/Wine-Staging_Patches. We have tested the patch tracker in a closed beta phase, so you can also find some older submissions there. Both systems are available at https://dev.wine-staging.com/. They use the same login system, so you don't need to registers two accounts. Alternatively, you can just use Github for the login, then just have to verify your email address. Everything else should be described in the corresponding Wiki articles. So, try it out and drop me an email if you find any issues! Regards, Michael

1 0

Re: a-m-w-c-processthreads-l1-1-0: Forward OpenThreadToken() to advapi32.
by Sebastian Lackner April 23, 2017

April 23, 2017

On 24.04.2017 00:30, Olivier F. R. Dierick wrote: > Copy and paste of the stdcall from the 1.1.1 spec file. > > Application Psyprax needs this [1]. > > [1] https://forum.winehq.org/viewtopic.php?f=9&t=27888 > > Signed-off-by: Olivier F. R. Dierick <o.dierick(a)piezo-forte.be> > --- > .../api-ms-win-core-processthreads-l1-1-0.spec > | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > It would be better to update tools/make_specfiles, so that advapi32 stuff is synchronized automatically for this dll. Best regards, Sebastian

1 0

Re: ntdll: Mark LDR data as initialized.
by Marvin April 23, 2017

April 23, 2017

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=30362 Your paranoid android. === w7u (32 bit loader) === loader.c:2199: Test failed: expected WAIT_OBJECT_0, got 0x102 loader.c:2209: Test failed: expected thread exit code 196, got 259 loader.c:1713: Test failed: expected thread exit code 196, got 195

1 0

Re: [v3 PATCH 1/2] wined3d: Add dirty region tracking.
by Henri Verbeet April 23, 2017

April 23, 2017

On 20 April 2017 at 16:01, Masanori Kakura <kakurasan(a)gmail.com> wrote: > v2 - Use array > - Regions are stored in struct wined3d_texture > - Reuse old region if possible > v3 - Update Wine d3d8 test > - Get rid of "Ignoring dirty_region" FIXME in wined3d_texture_add_dirty_region() > > Signed-off-by: Masanori Kakura <kakurasan(a)gmail.com> > --- > dlls/d3d8/tests/visual.c | 10 ++--- > dlls/d3d9/tests/visual.c | 10 ++--- > dlls/wined3d/cs.c | 51 +++++++++++++++++++++- > dlls/wined3d/device.c | 6 +++ > dlls/wined3d/texture.c | 97 ++++++++++++++++++++++++++++++++++++++++-- > dlls/wined3d/wined3d_private.h | 9 +++- > 6 files changed, 167 insertions(+), 16 deletions(-) > Do you have any thoughts on the interactions with the resource location management?

2 2

Re: [PATCH 5/5] ddraw/tests: Introduce a helper function to create windows.
by Marvin April 23, 2017

April 23, 2017

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=30354 Your paranoid android. === w864 (32 bit ddraw7) === ddraw7.c:3724: Test failed: Lit quad without normals has color 0x00ff0000, expected 0x00000000. ddraw7.c:3728: Test failed: Lit quad with normals has color 0x00ff0000, expected 0x00000000. ddraw7.c:3752: Test failed: Lit quad with light has color 0x00ff00ff. ddraw7.c:3752: Test failed: Lit quad with singular world matrix has color 0x00ff00ff. ddraw7.c:3752: Test failed: Lit quad with transformation matrix has color 0x00ff00ff. ddraw7.c:3752: Test failed: Lit quad with non-affine matrix has color 0x00ff0000.

1 0

Slow emulation of StretchBlt
by andrea April 22, 2017

April 22, 2017

Hi, I am looking at the performance of AppleWin (a 32 bit app) in Wine and have come across this function StretchBlt which seems to be much slower in Wine that in Windows 10. The call looks like this StretchBlt( hFrameDC, xdest, ydest, wdest, hdest, g_hDeviceDC, xSrc, ySrc, FRAMEBUFFER_BORDERLESS_W, FRAMEBUFFER_BORDERLESS_H, SRCCOPY); With typical values xdest 0 int ydest 0 int wdest 1120 int hdest 768 int xSrc 22 int ySrc 17 int #define FRAMEBUFFER_BORDERLESS_W 560 #define FRAMEBUFFER_BORDERLESS_H 384 In Wine they take 20x the time in Windows 10 (running inside a VirtualBox). The consequence is CPU utilisation 20x higher in Wine than Windows. I am running wine 2.5 in a 64 bit Fedora 25 machine. Does this ring any bell? Andrea

5 6

Debian Download Page
by Stefan Leichter April 22, 2017

April 22, 2017

Hi, when following the instruction at https://wiki.winehq.org/Debian the command "sudo apt-get update" gives me the error: -------------------------------------------------------------------- W: GPG error: https://dl.winehq.org jessie InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 818A435C5FCBF54A -------------------------------------------------------------------- Does anyone else see the same error? Minor item: Any reason that "wine-stable" is not listed on the download page? Regards Stefan

2 2

Linux desktop integration: Fill StartupWMClass in .desktop menu entries
by Eike Hein April 22, 2017

April 22, 2017

Hi, I maintain the taskbar in Plasma 5 and would like to poke folks about a desktop integration pain point we (and our friends at other DEs) face with apps run via wine. Here's the relevant ticket: https://bugs.winehq.org/show_bug.cgi?id=32699 Let me give some broader context on this. Modern DEs have a swath of UX going on that relies on being able to identify the application owning a particular window in the windowing system. "Identify the application" in practice means "map the window to a .desktop file installed in the menu file system". We want to do this because: a) Many a taskbar/dock-like UI tries to form a smooth life cycle across "launcher button", "startup notification" and "window button(s)". For each of those we need to work out the app they are associated with to be able to match them up. If it fails, ugly things like co-existing task buttons or missing startup throbbers and similar happen. b) UI these days tends to prefer to load assets such as app icons via the values defined in .desktop files instead of using the window icon, to ensure consistent results (apps may load icon assets differently from the shell, leading to asset quality issues, inconsistent theming, the icon changing between the lifecycle states mentioned in 'a', etc.) Plus the Wayland windowing system we're all migrating towards currently doesn'ㅅ have window icons. c) DEs do things like track app starts/quits for things like "Most used apps" launcher lists. To match a window to a .desktop file we rely on window and process metadata. On X11, there's the WM_CLASS window property: https://tronche.com/gui/x/icccm/sec-4.html#WM_CLASS Ideally, the second part of a X11 window's WM_CLASS matches the name of a .desktop file installed on the system. Failing that, it matches the value of StartupWMClass in that .desktop file (this is the key part I'll get back to soon). Here's some more documentation by our friends at Gnome on this standard scheme: https://wiki.gnome.org/Projects/GnomeShell/ApplicationBased (Look for WM_CLASS / StartupWMClass.) In practice, we pull many other tricks to cope with broken meta data. We try to match things by names, we grab process command lines from /proc and check them against Exec keys in .desktop files in complicated ways, ... it's a multi-page heuristic, and other DEs (Unity via bamf, Gnome Shell via iirc ShellWindowTracker) have equivalent code. Something Gnome does that we don't do (because I think it's brittle) is to store the name of the .desktop file an app was launched with as a private window hint on the next window that comes up (which then doesn't work when the app was launched from CLI instead of shell UI). None of these things currently allow us to handle wine very well. As of now (perhaps thanks to the patch mentioned in the linked ticket), wine sets WM_CLASS to the executable name of the app, e.g. "Notepad.exe". But the .desktop files it generates don't match this by name, and they don't contain a StartupWMClass key. The process command line is the Windows-like path to the .exe, which we also can't easily match up to the complicated Exec values in wine's .desktop files. (Aside: On Wayland, there's a more modern version of this: Windows have an appid property, which should be set to a .desktop file name. I don't know what the status of Wayland support in wine is -- currently the wine clients we encounter run over Xwayland, so we're back to WM_CLASS. We hope wine will set this property correctly when the time comes, or we're happy to work with you guys to find a way to support wine.) Long story short ... could this be picked up? The goal must be to have either .desktop file names or their StartupWMClass values match whatever is set as WM_CLASS. Currently, the easiest fix would be to set StartupWMClass to the executable name set as WM_CLASS. I assume the .desktop file generator has access to this, as the .lnk file referred to by Exec should presumably contain the name of the executable to be launched. Cheers, Eike

2 2

Re: [PATCH 3/4] d3d9/tests: Accept alpha channel differences in shadow_test.
by Marvin April 21, 2017

April 21, 2017

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=30337 Your paranoid android. === w8 (32 bit visual) === visual.c:8096: Test failed: Got unexpected color 0x00ff00ff for no color attribute test.

2 1

Re: [PATCH 1/4] ddraw/tests: Don't use DDBLT_COLORFILL in test_flip (v2).
by Marvin April 21, 2017

April 21, 2017

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=30335 Your paranoid android. === w8 (32 bit ddraw1) === ddraw1.c:6945: Test failed: Got unexpected color 0x00000000. ddraw1.c:6949: Test failed: Got unexpected color 0x00000000. ddraw1.c:7012: Test failed: Got unexpected color 0x00000000. ddraw1.c:7014: Test failed: Got unexpected color 0x00000000.

2 1

Re: [Tools v2] winetest/dissect: Check the build id and get its details in one Git call.
by Alexandre Julliard April 21, 2017

April 21, 2017

Francois Gouget <fgouget(a)codeweavers.com> writes: > get_build_info() only returns an empty subject if the commit does not > exist. It may still exist but not be a proper commit SHA-1. I'm not sure it's a good idea to relax that test. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 7/8] d3dx9: Support parameters sharing in effect.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/d3dx9_private.h | 11 ++- > dlls/d3dx9_36/effect.c | 197 +++++++++++++++++++++++++++++++++++++++++- > dlls/d3dx9_36/tests/effect.c | 17 ++-- > 3 files changed, 211 insertions(+), 14 deletions(-) > > diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h > index da4564d..d6d4280 100644 > --- a/dlls/d3dx9_36/d3dx9_private.h > +++ b/dlls/d3dx9_36/d3dx9_private.h > @@ -215,15 +215,22 @@ struct d3dx_parameter > struct d3dx_parameter *annotations; > struct d3dx_parameter *members; > > - struct d3dx_parameter *referenced_param; > struct d3dx_param_eval *param_eval; > > struct d3dx_parameter *top_level_param; > + union > + { > + struct d3dx_parameter *referenced_param; > + struct d3dx_parameter *shared_param; > + }; > + LONG shared_refcount; > }; I think you mentioned this before but it shouldn't be necessary to store the whole parameter structure in the pool. I guess it could be improved after the fact but it shouldn't be too complicated to do it right away. Probably it could just be shared_param_list[] from patch 8/8 and shared_refcount (which at that point won't need to be in struct d3dx_parameter anymore), in addition to data. You could access the other parameter fields through shared_param_list[0]. > +static void d3dx_pool_sync_shared_parameter(struct ID3DXEffectPoolImpl *pool, struct d3dx_parameter *param) > +{ > + unsigned int i, free_entry_index; > + LONG new_refcount; > + > + if (!(param->flags & PARAMETER_FLAG_SHARED) || !pool > + || is_param_type_sampler(param->type)) > + return; > + > + free_entry_index = pool->parameter_count; > + for (i = 0; i < pool->parameter_count; ++i) > + { > + if (!pool->parameters[i]->shared_refcount) > + free_entry_index = i; > + else if (is_same_parameter(param, pool->parameters[i])) > + break; > + } > + if (i == pool->parameter_count) > + { > + i = free_entry_index; > + if (i >= pool->table_size) > + { > + struct d3dx_parameter ** new_alloc; No whitespace after "**", please. > + else > + { > + new_size = pool->table_size * 2; > + new_alloc = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, pool->parameters, > + sizeof(*pool->parameters) * new_size); > + if (!new_alloc) > + { > + ERR("Out of memory.\n"); > + return; > + } > + } > + pool->parameters = new_alloc; > + pool->table_size = new_size; > + } > + pool->parameters[i] = HeapAlloc(GetProcessHeap(), > + HEAP_ZERO_MEMORY, sizeof(*pool->parameters[i])); > + copy_parameter_structure(param, pool->parameters[i], NULL, FALSE); > + param_set_data_pointer(pool->parameters[i], (unsigned char *)param->data, FALSE, FALSE); > + if (i == pool->parameter_count) > + pool->parameter_count++; > + } I don't think this works. HeapReAlloc might move the memory block and that would break the preexisting effects using the pool (and also the previous parameters of the new effect). HEAP_REALLOC_IN_PLACE_ONLY would avoid that but it also makes it easier for the reallocation to fail. I see two ways to fix this: - refresh all the parameters of the effects that are in the pool when you have to reallocate (shared_param_list[] might help here) - allocate the memory in chunks, keeping an array (or list) of chunks which you allocate (but never resize) as needed

2 1

Re: [Tools v2] winetest: Get rid of $gitdir.
by Alexandre Julliard April 21, 2017

April 21, 2017

Francois Gouget <fgouget(a)codeweavers.com> writes: > WineHQ appears to use /home/winehq/opt/source/git/wine.git as the Wine > checkout directory. After this patch the scripts will use > $workdir/wine.git so this should most likely symlink to the above > location. I'm not sure that's an improvement. > However any non-official site will actually need write access to it from > the scripts to ensure that wine.git is up to date before processing > reports. Otherwise the reports risk being rejected due to the not yet > known commit id. This could still work with a placement in tools/winehq: > one would just have to symlink that to a writable location but it does > imply placing it in $workdir may make more sense. I don't think it makes sense to put it in the website directory, it's not meant to be accessed from the web. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 3/8] d3dx9: Reference top level parameter instead of dirty flag.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > -static BOOL param_set_dirty_flag_ptr(void *dirty_flag_ptr, struct d3dx_parameter *param) > +static BOOL param_set_top_level_param(void *top_level_param, struct d3dx_parameter *param) > { > - param->dirty_flag_ptr = (DWORD *)dirty_flag_ptr; > + param->top_level_param = (struct d3dx_parameter *)top_level_param; > return FALSE; > } The cast is unnecessary (yeah, it was already unnecessary, I missed that earlier). It might make sense to store both? Avoiding the extra indirection in is_param_dirty() might help a tiny bit. It's fine to tackle this later on.

2 2

Re: [PATCH 8/8] d3dx9: Use local dirty flags for shared parameters.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: As I somewhat hinted at in the previous patch, this one should probably be merged with 7/8. It did help to keep them separated for the first review pass though :) > diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h > index d6d4280..f9cbf7c 100644 > --- a/dlls/d3dx9_36/d3dx9_private.h > +++ b/dlls/d3dx9_36/d3dx9_private.h > @@ -222,15 +222,14 @@ struct d3dx_parameter > { > struct d3dx_parameter *referenced_param; > struct d3dx_parameter *shared_param; > + struct d3dx_parameter **shared_param_list; This name is a bit confusing given that it is an array of (pointers to) matching parameters from different effects and shared_param is a pointer to the shared pool parameter. It becomes less of an issue if the new field moves to the separate shared parameter structure I mentioned. Maybe just call it "parameters"? > @@ -3193,7 +3196,16 @@ static void d3dx_pool_sync_shared_parameter(struct ID3DXEffectPoolImpl *pool, st > param_set_data_pointer(param, (unsigned char *)pool->parameters[i]->data, FALSE, TRUE); > } > new_refcount = InterlockedIncrement(&pool->parameters[i]->shared_refcount); > + if (new_refcount == 1) > + pool->parameters[i]->shared_param_list = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > + sizeof(*pool->parameters[i]->shared_param_list)); > + else > + pool->parameters[i]->shared_param_list = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, > + pool->parameters[i]->shared_param_list, > + sizeof(*pool->parameters[i]->shared_param_list) * new_refcount); It's probably better to make an initial allocation for > 1 elements and then expand by doubling the size, as usual with this kind of stuff.

1 0

Re: [PATCH 6/8] d3dx9: Factor out free_parameter_data() function.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +static void free_parameter(struct d3dx_parameter *param, BOOL element, BOOL child) > +{ > + unsigned int i; > + > + TRACE("Free parameter %p, name %s, type %s, child %s\n", param, param->name, > + debug_d3dxparameter_type(param->type), child ? "yes" : "no"); Again, given that it's a chance for some cleanup, please add the period at the end of the message. Also I'd change the "child" part, either just using %#x or child ? "child" : "" (or something else, the point being that the current "child yes" vs "child no" looks pretty awkward). Maybe you can also trace the "element" parameter. > + if (param->param_eval) > + { > + d3dx_free_param_eval(param->param_eval); > + param->param_eval = NULL; Is setting the field to NULL necessary? Same for the others below.

1 0

Re: [PATCH 5/8] d3dx9: Reference effect pool implementation in d3dx9_base_effect structure.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +struct ID3DXEffectPoolImpl > +{ > + ID3DXEffectPool ID3DXEffectPool_iface; > + LONG ref; > +}; Please rename it to "d3dx_effect_pool" or similar, while you're at it. Same for the interface methods (d3dx_effect_pool_QueryInterface(), ...) We want to get rid of the MS-like names for our implementation and this seems like a good chance to clean this one up. > - TRACE("base %p, data %p, data_size %lu, effect %p\n", base, data, data_size, effect); > + TRACE("base %p, data %p, data_size %lu, effect %p, pool %p\n", base, data, data_size, effect, pool); Since you're touching it, please add a period at the end of the message.

1 0

Re: [PATCH 2/8] d3dx9/tests: Add more tests for IsParameterUsed() function.
by Matteo Bruni April 21, 2017

April 21, 2017

2017-04-20 13:26 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > + todo_wine > + ok(effect3->lpVtbl->IsParameterUsed(effect3, "vs_arr2", "tech0"), > + "Unexpected IsParameterUsed result FALSE.\n"); > + todo_wine > + ok(!effect3->lpVtbl->IsParameterUsed(effect3, "vs_arr2[0]", "tech0"), > + "Unexpected IsParameterUsed result FALSE.\n"); Typo. I'd just leave out the TRUE / FALSE from these messages, one has to look up the code anyway if there is a test failure.

1 0

Re: [2/2] [v2] secur32: Add support for SECPKG_ATTR_KEY_INFO.
by Jacek Caban April 21, 2017

April 21, 2017

Hi Akihiro, The patch looks mostly good now, thanks. I have a few minor comments. On 20.04.2017 13:58, Akihiro Sagawa wrote: > +struct schan_alg_id_name { > + ALG_ID alg_id; > + const char* name; > + const WCHAR *nameW; > +}; Using an array here like const WCHAR nameW[8]; would make later initialization nicer by not requiring multiple string variables. > + > +static int comp_alg_id(const void *a, const void *b) > +{ > + const struct schan_alg_id_name *lhs = a; > + const struct schan_alg_id_name *rhs = b; > + return (int)lhs->alg_id - (int)rhs->alg_id; > +} > + > +static void* get_alg_name(ALG_ID id, BOOL wide) > +{ > + static const WCHAR ecdsaW[] = {'E','C','D','S','A',0}; > + static const WCHAR rsaW[] = {'R','S','A',0}; > + static const WCHAR desW[] = {'D','E','S',0}; > + static const WCHAR rc2W[] = {'R','C','2',0}; > + static const WCHAR triple_desW[] = {'3','D','E','S',0}; > + static const WCHAR aesW[] = {'A','E','S',0}; > + static const WCHAR rc4W[] = {'R','C','4',0}; > + static const struct schan_alg_id_name alg_name_map[] = { > + { CALG_ECDSA, "ECDSA", ecdsaW }, > + { CALG_RSA_SIGN, "RSA", rsaW }, > + { CALG_DES, "DES", desW }, > + { CALG_RC2, "RC2", rc2W }, > + { CALG_3DES, "3DES", triple_desW }, > + { CALG_AES_128, "AES", aesW }, > + { CALG_AES_192, "AES", aesW }, > + { CALG_AES_256, "AES", aesW }, > + { CALG_RC4, "RC4", rc4W }, > + }; > + const struct schan_alg_id_name *res; > + struct schan_alg_id_name key; > + > + key.alg_id = id; > + res = bsearch(&key, alg_name_map, > + sizeof(alg_name_map)/sizeof(alg_name_map[0]), sizeof(alg_name_map[0]), > + comp_alg_id); You could use id directly as a key. You don't need it to be of schan_alg_id_name* type, the key may be just ALG_ID. Thanks, Jacek

1 0

Re: [PATCH] tests: Don't evaluate the todo condition when not running on Wine.
by Sebastian Lackner April 21, 2017

April 21, 2017

On 21.04.2017 15:01, Huw Davies wrote: > Signed-off-by: Huw Davies <huw(a)codeweavers.com> > --- > This cropped up because I wanted to write a todo condition involving > IsProcessDPIAware(), since this doesn't exist in XP, its address is > loaded dynamically. I could have written the condition as > pIsProcessDPIAware && pIsProcessDPIAware(), but since I know the > function is always present in Wine, that's unnecessary. Perhaps > more to the point, I didn't expect the current behaviour. > > include/wine/test.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/wine/test.h b/include/wine/test.h > index af602c0fac..fa3f90c932 100644 > --- a/include/wine/test.h > +++ b/include/wine/test.h > @@ -123,7 +123,7 @@ extern void __winetest_cdecl winetest_trace( const char *msg, ... ) WINETEST_PRI > winetest_loop_todo(); \ > winetest_end_todo()) > #define todo_wine todo_if(!strcmp(winetest_platform, "wine")) > -#define todo_wine_if(is_todo) todo_if((is_todo) && !strcmp(winetest_platform, "wine")) > +#define todo_wine_if(is_todo) todo_if(!strcmp(winetest_platform, "wine") && (is_todo)) > > > #ifdef NONAMELESSUNION > I don't mind if its changed, but please note that it was intentional to check "is_todo" first. The idea was that side effects (like assigning variables, changing last error, ...) are applied on both Wine and Windows. Just think about something like: FirstCall(); todo_wine_if(SecondCall()) ok(GetLastError() == ..., "\n"); With your patch applied, it will test two entirely different things on Windows (error from FirstCall) and Wine (error from SecondCall), which would also be confusing. Best regards, Sebastian

3 3

Re: [PATCH] msvcrt: Handle synchronous flag for x64 C++ exceptions
by Piotr Caban April 21, 2017

April 21, 2017

Hi, On 04/20/17 16:42, Daniel Lehman wrote: > + if (descr->magic >= CXX_FRAME_MAGIC_VC8 && > + (descr->flags & FUNC_DESCR_SYNCHRONOUS) && > + (rec->ExceptionCode != CXX_EXCEPTION && rec->ExceptionCode != STATUS_UNWIND_CONSOLIDATE)) > + return ExceptionContinueSearch; /* handle only c++ exceptions */ I think the condition for STATUS_UNWIND_CONSOLIDATE should also check number of parameters and ExceptionInformation[0]==(ULONG_PTR)call_catch_block like it's done during unwinding. Also the code breaks unwinding for longjump when FUNC_DESCR_SYNCHRONOUS flag is set. Thanks, Piotr

1 0

Re: [PATCH 08/12] d3d10core/tests: Allow passing any interface to get_refcount().
by Henri Verbeet April 21, 2017

April 21, 2017

On 21 April 2017 at 10:59, Józef Kucia <jkucia(a)codeweavers.com> wrote: > -static ULONG get_refcount(IUnknown *iface) > +#define get_refcount(iface) get_refcount_((IUnknown *)iface) > +static ULONG get_refcount_(IUnknown *iface) You don't really need that macro.

1 0

Linux desktop integration: Fill StartupWMClass in .desktop menu entries
by Eike Hein April 21, 2017

April 21, 2017

Hi, I maintain the taskbar in Plasma 5 and would like to poke folks about a desktop integration pain point we (and our friends at other DEs) face with apps run via wine. Here's the relevant ticket: https://bugs.winehq.org/show_bug.cgi?id=32699 Let me give some broader context on this. Modern DEs have a swath of UX going on that relies on being able to identify the application owning a particular window in the windowing system. "Identify the application" in practice means "map the window to a .desktop file installed in the menu file system". We want to do this because: a) Many a taskbar/dock-like UI tries to form a smooth life cycle across "launcher button", "startup notification" and "window button(s)". For each of those we need to work out the app they are associated with to be able to match them up. If it fails, ugly things like co-existing task buttons or missing startup throbbers and similar happen. b) UI these days tends to prefer to load assets such as app icons via the values defined in .desktop files instead of using the window icon, to ensure consistent results (apps may load icon assets differently from the shell, leading to asset quality issues, inconsistent theming, the icon changing between the lifecycle states mentioned in 'a', etc.) Plus the Wayland windowing system we're all migrating towards currently doesn'ㅅ have window icons. c) DEs do things like track app starts/quits for things like "Most used apps" launcher lists. To match a window to a .desktop file we rely on window and process metadata. On X11, there's the WM_CLASS window property: https://tronche.com/gui/x/icccm/sec-4.html#WM_CLASS Ideally, the second part of a X11 window's WM_CLASS matches the name of a .desktop file installed on the system. Failing that, it matches the value of StartupWMClass in that .desktop file (this is the key part I'll get back to soon). Here's some more documentation by our friends at Gnome on this standard scheme: https://wiki.gnome.org/Projects/GnomeShell/ApplicationBased (Look for WM_CLASS / StartupWMClass.) In practice, we pull many other tricks to cope with broken meta data. We try to match things by names, we grab process command lines from /proc and check them against Exec keys in .desktop files in complicated ways, ... it's a multi-page heuristic, and other DEs (Unity via bamf, Gnome Shell via iirc ShellWindowTracker) have equivalent code. Something Gnome does that we don't do (because I think it's brittle) is to store the name of the .desktop file an app was launched with as a private window hint on the next window that comes up (which then doesn't work when the app was launched from CLI instead of shell UI). None of these things currently allow us to handle wine very well. As of now (perhaps thanks to the patch mentioned in the linked ticket), wine sets WM_CLASS to the executable name of the app, e.g. "Notepad.exe". But the .desktop files it generates don't match this by name, and they don't contain a StartupWMClass key. The process command line is the Windows-like path to the .exe, which we also can't easily match up to the complicated Exec values in wine's .desktop files. (Aside: On Wayland, there's a more modern version of this: Windows have an appid property, which should be set to a .desktop file name. I don't know what the status of Wayland support in wine is -- currently the wine clients we encounter run over Xwayland, so we're back to WM_CLASS. We hope wine will set this property correctly when the time comes, or we're happy to work with you guys to find a way to support wine.) Long story short ... could this be picked up? The goal must be to have either .desktop file names or their StartupWMClass values match whatever is set as WM_CLASS. Currently, the easiest fix would be to set StartupWMClass to the executable name set as WM_CLASS. I assume the .desktop file generator has access to this, as the .lnk file referred to by Exec should presumably contain the name of the executable to be launched. Cheers, Eike

1 0

Re: [Tools] winetest: Get rid of $gitdir.
by Francois Gouget April 21, 2017

April 21, 2017

Actually this patch depends on the next one 'winetest/dissect: Check the build id and get its details in one Git call.' I'll resubmit them in a cleaner way. -- Francois Gouget <fgouget(a)codeweavers.com>

1 0

Re: [PATCH 1/4] regedit: Validate REG_SZ import data before processing it any further (v2)
by Alexandre Julliard April 21, 2017

April 21, 2017

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > programs/regedit/regproc.c | 6 ++++++ > programs/regedit/tests/regedit.c | 14 +++++++------- > 2 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/programs/regedit/regproc.c b/programs/regedit/regproc.c > index 96c4564..ccd9ebe 100644 > --- a/programs/regedit/regproc.c > +++ b/programs/regedit/regproc.c > @@ -293,6 +293,12 @@ static int REGPROC_unescape_string(WCHAR* str) > str[val_idx] = str[str_idx]; > break; > } > + } else if (str[str_idx] == '"') { > + WCHAR *p = str + str_idx + 1; > + while (*p == ' ' || *p == '\t') p++; > + if (*p && *p != ';') return 0; > + str[val_idx++] = str[str_idx]; > + break; That would work, but it seems to me that the comment check would be better in the caller, so that you can use the same code for validating value names. -- Alexandre Julliard julliard(a)winehq.org

2 2

Re: [PATCH] user32: add GetAutoRotationState/GetDisplayAutoRotationPreferences stubs
by Huw Davies April 21, 2017

April 21, 2017

On Thu, Apr 20, 2017 at 05:33:15PM -0500, Austin English wrote: > diff --git a/dlls/user32/misc.c b/dlls/user32/misc.c > index fa0502e..3581235 100644 > --- a/dlls/user32/misc.c > +++ b/dlls/user32/misc.c > @@ -910,3 +910,24 @@ LRESULT WINAPI ImeWndProcW( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam ) > > return DefWindowProcW(hwnd, msg, wParam, lParam); > } > + > +/********************************************************************** > + * GetAutoRotationState [USER32.@] > + */ > +BOOL WINAPI GetAutoRotationState( AR_STATE *state ) > +{ > + *state = AR_NOT_SUPPORTED; > + FIXME("(%p): stub\n", state); > + return TRUE; > +} > + I think these would be better in sysparams.c. I realise that there are already functions of this nature in misc.c - they could probably be moved across too. User32 does indeed have some odd 'misc' like functions, but these aren't them. Also, we'd generally put the FIXME() at the start of the function, not after the assignment. Apart from that, this looks good to me. Huw.

1 0

shell32: Solve the problem about 'OK' button is always gray when call SHBrowseForFolder with flags including both BIF_BROWSEFORCOMPUTER and BIF_RETURNONLYFSDIRS
by 许文龙 April 20, 2017

April 20, 2017

shell32: Solve the problem about 'OK' button is always gray when call SHBrowseForFolder with flags including both BIF_BROWSEFORCOMPUTER and BIF_RETURNONLYFSDIRS

2 1

Re: [05/10] webservices: Add support for outgoing TCP connections.
by Alexandre Julliard April 20, 2017

April 20, 2017

Hans Leidekker <hans(a)codeweavers.com> writes: > Signed-off-by: Hans Leidekker <hans(a)codeweavers.com> > --- > dlls/webservices/channel.c | 286 ++++++++++++++++++++++++++------- > dlls/webservices/listener.c | 16 +- > dlls/webservices/webservices_private.h | 5 + > 3 files changed, 240 insertions(+), 67 deletions(-) This breaks the Android build: x86_64-linux-android-gcc -m64 -c -o proxy.o ../../../wine/dlls/webservices/proxy.c -I. -I../../../wine/dlls/webservices \ -I../../include -I../../../wine/include -D__WINESRC__ -D_REENTRANT -fPIC -Wall -pipe \ -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers \ -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wpointer-arith \ -Wlogical-op -gdwarf-2 -gstrict-dwarf -g -O2 -fno-diagnostics-show-caret In file included from ../../../wine/dlls/webservices/webservices_private.h:19:0, from ../../../wine/dlls/webservices/proxy.c:29: ../../../wine/include/ws2tcpip.h:57:16: error: conflicting types for 'socklen_t' ../../../wine/include/ws2tcpip.h:29:18: note: in definition of macro 'WS' In file included from /home/julliard/wine/build/toolchain-android-x86_64/sysroot/usr/include/stdio.h:50:0, from ../../../wine/dlls/webservices/proxy.c:19: /home/julliard/wine/build/toolchain-android-x86_64/sysroot/usr/include/sys/types.h:127:21: note: previous declaration of 'socklen_t' was here -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [v3 PATCH 2/2] wined3d: Update only dirty regions in wined3d_device_update_texture().
by Marvin April 20, 2017

April 20, 2017

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=30284 Your paranoid android. === w864 (64 bit visual) === visual.c:7989: Test failed: Got unexpected color 0x00007580 for quad 2 (different colors). visual.c:8096: Test failed: Got unexpected color 0x00000080 for no color attribute test. === w1064 (64 bit visual) === visual.c:8096: Test failed: Got unexpected color 0x00ff00ff for no color attribute test.

1 0

Re: [v3 PATCH 1/2] wined3d: Add dirty region tracking.
by Marvin April 20, 2017

April 20, 2017

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=30282 Your paranoid android. === w864 (64 bit visual) === visual.c:8096: Test failed: Got unexpected color 0x00ff0000 for no color attribute test.

1 0

Re: [PATCH] mfplat: Implement MFCreateAttributes
by Nikolay Sivov April 20, 2017

April 20, 2017

On 04/20/2017 10:44 AM, Alistair Leslie-Hughes wrote: > +{ > + mfattributes *This = impl_from_IMFAttributes(iface); > + > + if(IsEqualGUID(riid, &IID_IUnknown) || > + IsEqualGUID(riid, &IID_IMFAttributes)) > + { > + TRACE("(IID_IUnknown %p)\n", object); > + *object = This; > + } > + else > + { > + FIXME("(%s, %p)\n", debugstr_guid(riid), object); > + *object = NULL; > + return E_NOINTERFACE; > + } > + > + IUnknown_AddRef((IUnknown*)*object); > + return S_OK; > +} Please add a regular TRACE for this method, and return iface or interface pointer from This, instead of This itself. Trace you have now is inaccurate. > +HRESULT WINAPI MFCreateAttributes(IMFAttributes **attributes, UINT32 size) > +{ > + mfattributes *object; > + > + TRACE("%p, %d\n", attributes, size); > + > + object = HeapAlloc( GetProcessHeap(), 0, sizeof(*attributes) ); > + if(!object) > + return E_OUTOFMEMORY; This should use sizeof(*object).

2 1

Re: [v2 PATCH] mfplat: Implement MFCreateAttributes
by Dmitry Timoshkov April 20, 2017

April 20, 2017

Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > +typedef struct _mfattributes > +{ > + IMFAttributes IMFAttributes_iface; > + LONG ref; > +} mfattributes; Please try to exchange these two lines in the structure above and see what happens. -- Dmitry.

1 0

Review | endproc.c: Implement ProcessPage_OnEndProcessTree
by Akarsha Sehwag April 19, 2017

April 19, 2017

Hi Could someone take out a little time to review the code once? Bug I intend to solve: https://bugs.winehq.org/show_bug.cgi?id=39640 I am new to wine. So, I would appreciate suggestions regarding my coding style if that's not-so-perfect according to wine standards. Or any other problems with the code I have written. I apologize for submitting an untested code earlier. I hope now it works fine. /*-------------------------------------------CODE--------------------------------------------------*/ typedef struct process_list { DWORD *pid; /*dynamic array to store the process IDs*/ SIZE_T count; /*index to maintain the last entry of the array;*/ SIZE_T size; /*the current size of the pid array*/ } process_list; static void init_process_list(process_list *list) { list->size = 4; /*initialise size with 4. Will increase if necessary.*/ list->pid = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, list->size * sizeof(list->pid)); list->count = 0; } static void increase_list_size(process_list *list) { list->size *= 2; list->pid = HeapReAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, list->pid, list->size * sizeof(list->pid)); *//DOUBT: *Would sizeof(list->pid) be better or sizeof(int)? //Since in the former case, since list->pid is actually a pointer to DWORD, so //sizeof(list->pid) would return 8 bytes and not 4 bytes. } static void process_list_append(process_list *list, DWORD id) { if(list->count == list->size) increase_list_size(list); list->pid[list->count] = id; list->count += 1; } static void free_process_list(process_list *list) { HeapFree(GetProcessHeap(), 0, list->pid); } static void enum_process_children(HANDLE snapshot, process_list *list, DWORD pid) { PROCESSENTRY32 entry; SIZE_T start, end, i ; start = list->count; entry.dwSize = sizeof(entry); if(!Process32First(snapshot, &entry)) return; do { if(entry.th32ParentProcessID == pid) process_list_append(list, entry.th32ProcessID); } while (Process32Next(snapshot, &entry)); end = list->count; for(i = start; i < end; ++i) { enum_process_children(snapshot, list, list->pid[i]); } } void ProcessPage_OnEndProcessTree(void) { {...} process_list list; SIZE_T i; HANDLE snapshot; {...} init_process_list(&list); if(list.pid == NULL) return; process_list_append(&list, dwProcessId); snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); if(!snapshot) return; enum_process_children(snapshot, &list, dwProcessId); CloseHandle(snapshot); for(i = 0; i < list.count; ++i) { hProcess = OpenProcess(PROCESS_TERMINATE, FALSE, list.pid[i]); if (!hProcess) { GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP); break; } if (!TerminateProcess(hProcess, 0)) { GetLastErrorText(wstrErrorText, sizeof(wstrErrorText)/sizeof(WCHAR)); MessageBoxW(hMainWnd, wstrErrorText,wszUnable2Terminate, MB_OK|MB_ICONSTOP); } CloseHandle(hProcess); } free_process_list(&list); } /*---------------------------------------END OF CODE------------------------------------------*/ Thanks and regards -- Akarsha Sehwag

1 0