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

UIRibbon

by Fabian Maurer

Hi Wine-Devel, I'm currently working on a patchset to get UIRibbon to work, or at least to not crash the program. I noticed that the ribbon xml files need to be compiled with a program called UICC.exe from the windows SDK, the resulting resources have some proprietary binary format. If we want to load and display a ribbon, we'll have to understand it. Does anyone know about information on that, or is the only way to reverse engineer it? Regards, Fabian Maurer

July 28, 2017

Re: [2/6] wsdapi: Add code to create SOAP envelope from header and body elements.

by Vincent Povirk

+ wideStringLength = lstrlenW(wideString); + sizeNeeded = WideCharToMultiByte(CP_UTF8, 0, wideString, wideStringLength, NULL, 0, NULL, NULL); + + if (sizeNeeded < 0) + { + return NULL; + } + + newString = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeNeeded + 1); + WideCharToMultiByte(CP_UTF8, 0, wideString, wideStringLength, newString, sizeNeeded, NULL, NULL); This would be much simpler if you passed in -1 as the wide string length. Using HEAP_ZERO_MEMORY to null-terminate the string is a bit weird. + else if (curNode->Type == TextType) + { + textNode = WSDAllocateLinkedMemory(parent, sizeof(WSDXML_TEXT)); + if (textNode == NULL) goto cleanup; + + textNode->Node.Next = NULL; + textNode->Node.Parent = parent; + textNode->Node.Type = TextType; + textNode->Text = duplicate_string(textNode, ((WSDXML_TEXT *)curNode)->Text); + if (textNode->Text == NULL) goto cleanup; + } I'm not sure, but I think you need to add textNode as a child of newElement? + /* SequenceID attribute */ + if (header->AppSequence->SequenceId != NULL) + { + if (!add_string_attribute(xmlContext, appSequenceElement, discoveryNsUri, sequenceIdString, header->AppSequence->SequenceId)) goto cleanup; + } Do we need the namespace for SequenceID?

July 28, 2017

Re: [PATCH 1/3] uiribbon: Add DLL

by Fabian Maurer

Hello Alistair, Thanks for the quick feedback. > 1. Add new dll (DLLMain and private functions) Should I add all private functions like DllGetClassObject as stub, or already implemented? Because the class factory logic is is a bunch of code, that's why I separated it from the initial commit. > 2. Add idl to include (Including all the interfaces required) Just for clarification, I should add all the needed interfaces (including IUIApplication) at once in the idl, but only add the IUnknown methods into IUIFramework_Vtbl? I'll fix the other issues, and also a few style inconsistencies I noticed. I'm also working on tests, but I figured it makes sense to have a stubbed implementation first. Regards, Fabian Maurer

July 28, 2017

Re: [PATCH 1/3] uiribbon: Add DLL

by Alistair Leslie-Hughes

Hi Fabian, This is about the series of uiribbon. >+BOOL WINAPI DllMain(HINSTANCE hInstDLL, DWORD fdwReason, LPVOID lpv) Since nothing is implemented in this dll, please return check for DLL_WINE_PREATTACH. (example dlls/authz/authz.c). In this patch you could also add the private functions. patch 2. > + uiribbon.idl \ ... >interface IUIFramework : IUnknown >+{ >+ >+} >+ If your going to add a idl file, please fill in methods available for each interface. Please also avoid the forward declare for IUIApplication include it in the idl as well. >+ >+HRESULT UIRibbonFrameworkImpl_Create(IUnknown* pUnkOuter, LPVOID* ppObj)l; Move this function to after +static const IUIFrameworkVtbl IUIFramework_Vtbl = then you wont need the forward declare. I would have the following patches. 1. Add new dll (DLLMain and private functions) 2. Add idl to include (Including all the interfaces required) 3. Add the stubbed interface. 4. Tests? (maybe not possible at this stage). Best Regards Alistair Leslie-Hughes

July 28, 2017

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

by Borislav Petkov

On Thu, Jul 27, 2017 at 07:04:52PM -0700, Ricardo Neri wrote: > However using the union could be less readable than having two almost > identical functions. So having some small duplication for the sake of clarity and readability is much better, if you ask me. And it's not like you're duplicating a lot of code - it is only a handful of functions. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

July 28, 2017

Re: [PATCH] kernel32: Add support for detecting volume information with libudev.

by Alexandre Julliard

Zebediah Figura <z.figura12(a)gmail.com> writes: > On 07/25/2017 05:33 AM, Alexandre Julliard wrote: >> Zebediah Figura <z.figura12(a)gmail.com> writes: >> >>> Using GetVolumeInformation may fail due to lack of permissions; e.g. >>> when the drive was mounted as root. This patch uses libudev, if it >>> is available, to retrieve the volume label, serial number, and type. >>> >>> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> >>> --- >>> dlls/kernel32/Makefile.in | 3 +- >>> dlls/kernel32/volume.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 90 insertions(+), 1 deletion(-) >> >> libudev should be loaded dynamically, and that sort of thing should >> ideally be done in mountmgr. >> > > Should I move any existing code from GetVolumeInformation() into > mountmgr.sys? What are the responsibilities of each module in this > respect? Yes, kernel32 is not supposed to contain any of that code. It should only contain the NtQueryVolumeInformationFile part, and ntdll in turn should do an ioctl or similar call to mountmgr to get the information. -- Alexandre Julliard julliard(a)winehq.org

July 28, 2017

Re: [PATCH] kernel32: Add support for detecting volume information with libudev.

by Zebediah Figura

On 07/25/2017 05:33 AM, Alexandre Julliard wrote: > Zebediah Figura <z.figura12(a)gmail.com> writes: > >> Using GetVolumeInformation may fail due to lack of permissions; e.g. >> when the drive was mounted as root. This patch uses libudev, if it >> is available, to retrieve the volume label, serial number, and type. >> >> Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> >> --- >> dlls/kernel32/Makefile.in | 3 +- >> dlls/kernel32/volume.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 90 insertions(+), 1 deletion(-) > > libudev should be loaded dynamically, and that sort of thing should > ideally be done in mountmgr. > Should I move any existing code from GetVolumeInformation() into mountmgr.sys? What are the responsibilities of each module in this respect?

July 28, 2017

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

by Zebediah Figura

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

July 28, 2017

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

by Ricardo Neri

On Thu, 2017-07-27 at 15:26 +0200, Borislav Petkov wrote: > On Tue, Jul 25, 2017 at 04:48:13PM -0700, Ricardo Neri wrote: > > I meant to say the 4 most significant bytes. In this case, the > > 64-address 0xffffffffffff1234 would lie in the kernel memory while > > 0xffff1234 would correctly be in the user space memory. > > That explanation is better. > > > Yes, perhaps the check above is not needed. I included that check as > > part of my argument validation. In a 64-bit kernel, this function could > > be called with val with non-zero most significant bytes. > > So say that in the comment so that it is obvious *why*. > > > I have looked into this closely and as far as I can see, the 4 least > > significant bytes will wrap around when using 64-bit signed numbers as > > they would when using 32-bit signed numbers. For instance, for two > > positive numbers we have: > > > > 7fff:ffff + 7000:0000 = efff:ffff. > > > > The addition above overflows. > > Yes, MSB changes. > > > When sign-extended to 64-bit numbers we would have: > > > > 0000:0000:7fff:ffff + 0000:0000:7000:0000 = 0000:0000:efff:ffff. > > > > The addition above does not overflow. However, the 4 least significant > > bytes overflow as we expect. > > No they don't - you are simply using 64-bit regs: > > 0x00005555555546b8 <+8>: movq $0x7fffffff,-0x8(%rbp) > 0x00005555555546c0 <+16>: movq $0x70000000,-0x10(%rbp) > 0x00005555555546c8 <+24>: mov -0x8(%rbp),%rdx > 0x00005555555546cc <+28>: mov -0x10(%rbp),%rax > => 0x00005555555546d0 <+32>: add %rdx,%rax > > rax 0xefffffff 4026531839 > rbx 0x0 0 > rcx 0x0 0 > rdx 0x7fffffff 2147483647 > > ... > > eflags 0x206 [ PF IF ] > > (OF flag is not set). True, I don't have the OF set. However the 4 least significant bytes wrapped around; which is what I needed. > > > We can clamp the 4 most significant bytes. > > > > For a two's complement negative numbers we can have: > > > > ffff:ffff + 8000:0000 = 7fff:ffff with a carry flag. > > > > The addition above overflows. > > Yes. > > > When sign-extending to 64-bit numbers we would have: > > > > ffff:ffff:ffff:ffff + ffff:ffff:8000:0000 = ffff:ffff:7fff:ffff with a > > carry flag. > > > > The addition above does not overflow. However, the 4 least significant > > bytes overflew and wrapped around as they would when using 32-bit signed > > numbers. > > Right. Ok. > > And come to think of it now, I'm wondering, whether it would be > better/easier/simpler/more straight-forward, to do the 32-bit operations > with 32-bit types and separate 32-bit functions and have the hardware do > that for you. > > This way you can save yourself all that ugly and possibly error-prone > casting back and forth and have the code much more readable too. That sounds fair. I had to explain a lot this code and probably is not worth it. I can definitely use 32-bit variable types for the 32-bit case and drop all these castings. The 32-bit and 64-bit functions would look identical except for the variables used to compute the effective address. Perhaps I could use a union: union eff_addr { #if CONFIG_X86_64 long addr64; #endif int addr32; }; And use one or the other based on the address size given by the CS.L CS.D bits of the segment descriptor or address size overrides. However using the union could be less readable than having two almost identical functions. Thanks and BR, Ricardo

July 28, 2017

Re: [4/6] wsdapi: Implement "Hello" message creation support.

by Vincent Povirk

- obj = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*obj)); + heap = GetProcessHeap(); + obj = HeapAlloc(heap, HEAP_ZERO_MEMORY, sizeof(*obj)); I really hope we don't have to start optimizing for fewer GetProcessHeap calls. + /* Calculate space needed, including colon and potential trailing space */ + memoryNeeded = sizeof(WCHAR) * (lstrlenW(cur->Element->LocalName) + lstrlenW(cur->Element->Space->PreferredPrefix) + 2); This doesn't account for the terminating NULL wsprintfW will add. + discoveredNamespaces = WSDAllocateLinkedMemory(NULL, sizeof(struct list)); + if (!discoveredNamespaces) goto cleanup; Why not allocate this on the stack?

July 27, 2017