August 2016 - Wine-Devel - list.winehq.org

Re: Implementing Fiber-Local Storage Callbacks: sharing data between DLLs

by John Sheu

On Wed, Aug 31, 2016 at 12:21 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: > On Wed, Aug 31, 2016 at 02:12:23AM -0700, John Sheu wrote: >> TL;WR: I'd like to share a struct definition between ntdll and >> kernel32, passed through the PEB. Is this kosher according to Wine >> style/design, and if so, which header should I stick it in? >> > > I don't know anything about the particular issue you're working on, so > I can just speak generally. > > We try to avoid creating internal, Wine-only APIs. Sometimes it's > necessary, and when we do that, the header goes in <include/wine/>. > One example is "gdi_driver.h" which defines the interface between e.g. > user32 and the graphics drivers. > > Are the structures you need declared publicly in the win32 API, even > as opaque pointers? If so, they may belong in <include/winternl.h>. > > A specific proposal about what changes you'd like might bring more > detailed responses. Thanks Andrew! A little more elaboration, on what I already wrote: ntdll and kernel32 will have to jointly manage ownership of the storage array that FLS data is stored in, because it can be deleted either 1. When a thread is shutdown, in the LdrShutdownThread path. 2. When a fiber is deleted, in when DeleteFiber is called. In addition, (3) if the thread is aborted (through NtTerminateProcess), the storage array isn't deleted on thread abort, since (behavior confirmed by conformance tests) the FLS data is used later when FlsFree is called. So we have cases where in (1) ntdll deletes the FLS data array, (2) kernel32 deletes the FLS data array, and (3) ntdll passes ownership of the data array to kernel32. Thus both dlls will have to have to jointly manage ownership of the array. -- The question is how we manage this. One option is to introduce additional function call entry points to either ntdll or kernel32, called by the other, to manages FLS lifetime. I see though that the list of entry points is pretty tightly defined in the .spec files for each DLL, and the convention may be (correct me if I'm wrong) not to introduce "weird" private Wine-only APIs to the list. The other option is to use some internal structs, pointed to by the PEB and TEB structures. That's the approach I'd like to go with. My proposal would be: 1. Define a common struct to hold the FLS data array, that would be: struct FlsSlots { LIST_ENTRY fls_link; void fls_slots[SLOT_COUNT]; }; 2. TEB.FlsSlots right now is a void** member; make this a struct FlsSlots* member instead. 3. Link the FlsSlots struct into PEB.FlsListHead. This would require that struct FlsSlots be defined somewhere -- either in wine/include, as mentioned, or in <include/winternl.h>. Since it isn't declared publicly in the win32 API, even as an opaque pointer -- I'm leaning toward the former. Maybe in "wine/include/fls.h" ? Thanks, -John Sheu

Aug. 31, 2016

Re: [PATCH 2/2] winhttp: Also pass hostname to jsproxy

by Nikolay Sivov

On 31.08.2016 22:21, Andrew Eikum wrote: > > +static inline char *strdupWA_sized( const WCHAR *src, DWORD size ) > +{ > + char *dst = NULL; > + if (src) > + { > + int len = WideCharToMultiByte( CP_ACP, 0, src, size, NULL, 0, NULL, NULL ) + 1; > + if ((dst = heap_alloc( len ))) > + { > + WideCharToMultiByte( CP_ACP, 0, src, len, dst, size, NULL, NULL ); > + dst[len - 1] = 0; > + } > + } > + return dst; > +} > + This is usually called strndup* in Wine dlls. Also 'len' and 'size' seem to be misplaced, and I'm not sure it will do the right thing if for negative size, unless it's not meant to ever be negative.

Aug. 31, 2016

Re: Implementing Fiber-Local Storage Callbacks: sharing data between DLLs

by Andrew Eikum

On Wed, Aug 31, 2016 at 02:12:23AM -0700, John Sheu wrote: > TL;WR: I'd like to share a struct definition between ntdll and > kernel32, passed through the PEB. Is this kosher according to Wine > style/design, and if so, which header should I stick it in? > I don't know anything about the particular issue you're working on, so I can just speak generally. We try to avoid creating internal, Wine-only APIs. Sometimes it's necessary, and when we do that, the header goes in <include/wine/>. One example is "gdi_driver.h" which defines the interface between e.g. user32 and the graphics drivers. Are the structures you need declared publicly in the win32 API, even as opaque pointers? If so, they may belong in <include/winternl.h>. A specific proposal about what changes you'd like might bring more detailed responses. Hope that was at least a little helpful, Andrew

Aug. 31, 2016

Re: [PATCH 1/4] xinput1_3: Better trace all functions

by Sebastian Lackner

On 31.08.2016 20:30, Bruno Jesus wrote: > On Wed, Aug 31, 2016 at 3:23 PM, Sebastian Lackner > <sebastian(a)fds-team.de> wrote: >> On 31.08.2016 20:17, Bruno Jesus wrote: >>> - FIXME("(%d %p %p) Stub!\n", dwUserIndex, pDSoundRenderGuid, pDSoundCaptureGuid); >>> + FIXME("(index %u, render guid %p, capture guid %p) Stub!\n", index, render_guid, capture_guid); >> >> debugstr_guid() would be more appropriate for tracing GUIDs. > > They are output parameters ;-) > Oops, you are right. ;)

Aug. 31, 2016

Re: [PATCH 1/4] xinput1_3: Better trace all functions

by Bruno Jesus

On Wed, Aug 31, 2016 at 3:23 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > On 31.08.2016 20:17, Bruno Jesus wrote: >> - FIXME("(%d %p %p) Stub!\n", dwUserIndex, pDSoundRenderGuid, pDSoundCaptureGuid); >> + FIXME("(index %u, render guid %p, capture guid %p) Stub!\n", index, render_guid, capture_guid); > > debugstr_guid() would be more appropriate for tracing GUIDs. They are output parameters ;-)

Aug. 31, 2016

Re: [PATCH 1/4] xinput1_3: Better trace all functions

by Sebastian Lackner

On 31.08.2016 20:17, Bruno Jesus wrote: > - FIXME("(%d %p %p) Stub!\n", dwUserIndex, pDSoundRenderGuid, pDSoundCaptureGuid); > + FIXME("(index %u, render guid %p, capture guid %p) Stub!\n", index, render_guid, capture_guid); debugstr_guid() would be more appropriate for tracing GUIDs.

Aug. 31, 2016

Re: How to find out the source code function from compiled running address

by Bruno Jesus

On Tue, Aug 30, 2016 at 5:40 PM, Michael Müller <michael(a)fds-team.de> wrote: > Am 30.08.2016 um 22:09 schrieb Bernhard Übelacker: >> $ wine winedbg --gdb d3d8_test.exe.so >> ... >> 0025:0026: loads DLL C:\windows\system32\wined3d.dll @0x7ec20000 (0<0>) >> ... >> Wine-gdb> info symbol 0x7ec2EF19 >> init_output_registers + 1172 in section .text of /.../dlls/wined3d/wined3d.dll.so >> > > This does not work in case wined3d got relocated. You can either use > addr2line or winedbg directly on the target process. For addr2line you > need to know the base address of wined3d in the crashed process and then > use something like: > > addr2line -e wined3d.dll.so -j .text 0xdeadbeef > > where 0xdeadbeef is the address of the crash minus the base address of > the ELF section as shown in the crash dump. I think using winedbg is > easier. Either directly start the process using winedbg or in case the > process detects debuggers, you can also SHIFT + right click on the crash > dialog to start winedbg. Then you can just use bt to display the > backtrace including the source code lines (although the output should be > identical to the crash dialog). For arbitrary addresses, you can just > disassemble some instructions, it should also display the filename and > line number using x/10i 0xdeadbeef. You can also use registers this way > (for example x/10i $eip). > > If the application sets up its own crash handler, you can either attach > to the process when it displays an error by starting winedbg as > standalone program and then use "info process" and "attach 0x...". This > way you won't get a correct backtrace though, so you might want to edit > dlls/ntdll/signal_i386.c instead. I attached a hack to prevent 32 bit > applications from catching access violations. In case they call > SetUnhandledExceptionFilter, you also need to make this a noop as it > replaces the handler to start the debugger. Thanks to you both, I'll try both methods and find out what works best. Best regards, Bruno

Aug. 31, 2016

Re: [PATCH 1/1] gdiplus: Fix for the "graphics object has no HDC" issue in GdipDrawPath

by Vincent Povirk

>> If there's an HDC, we use a mix of GDI rendering and GDI+ rendering. >> If GDI can support a particular drawing operation, we use that. >> Otherwise, we render it internally and send it to the HDC using >> GdiAlphaBlend. >> >> At least, that's the general policy. There may be exceptions. > > I see, thanks for clearing that up. > > Regarding the crash, in "SOFTWARE_GdipDrawThinPath" the line > "GdipDeletePath(path);" seems faulty. > It seems odd to delete "path", maybe it should be "flat_path" instead? Yes, you're right.

Aug. 31, 2016

Re: [PATCH 2/4] ntoskrnl.exe: Use static initializer for device_drivers.

by Sebastian Lackner

On 31.08.2016 14:10, Jacek Caban wrote: > On 30.08.2016 22:00, Sebastian Lackner wrote: >> On 30.08.2016 21:30, Jacek Caban wrote: >>> Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> >>> --- >>> dlls/ntoskrnl.exe/ntoskrnl.c | 40 ++++++++-------------------------------- >>> 1 file changed, 8 insertions(+), 32 deletions(-) >>> >> Wouldn't it be better to use a macro similar to LIST_INIT() for that (to allow >> later changes of the internal structure)? > > I considered that. In case of list, I believe that motivation for the > macro was not about hiding internal structure, but due to non-obvious > initialization (it sets two pointer to point to the same struct). I > believe that this logic does not apply to wine_rb_tree. However, I'm > fine with having the macro. > > Thanks, > Jacek > So far both solutions are fine, and currently I also do not see any need to have non-null struct initializations. I'll leave the final decision up to Alexandre. Best regards, Sebastian

Aug. 31, 2016

Re: [2/8] webservices: Implement WsAddCustomHeader.

by Sebastian Lackner

On 31.08.2016 14:35, Hans Leidekker wrote: > + if (WsXmlStringEquals( desc->elementLocalName, &msg->header[i]->name, NULL ) && > + WsXmlStringEquals( desc->elementNs, &msg->header[i]->ns, NULL ) == S_OK) This looks suspicious because it does not match the follow up patch. are you missing a == S_OK maybe?

Aug. 31, 2016