October 2016 - 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

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: include: Avoid overflow (warning) in RGBA_MAKE.
by Dmitry Timoshkov Feb. 26, 2017

Feb. 26, 2017

Gerald Pfeifer <gerald(a)pfeifer.com> wrote: > --- a/include/d3dtypes.h > +++ b/include/d3dtypes.h > @@ -53,7 +53,7 @@ typedef LONG D3DFIXED; > #define RGBA_GETRED(rgb) (((rgb) >> 16) & 0xff) > #define RGBA_GETGREEN(rgb) (((rgb) >> 8) & 0xff) > #define RGBA_GETBLUE(rgb) ((rgb) & 0xff) > -#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((a) << 24) | ((r) << 16) | ((g) << 8) | (b))) > +#define RGBA_MAKE(r, g, b, a) ((D3DCOLOR) (((unsigned)(a) << 24) | ((r) << 16) | ((g) << 8) | (b))) This is a public header, probably you need to fix the callers of this macro instead. -- Dmitry.

3 4

Re: kernel32: Avoid two potential buffer overflows of cStr in create_hardware_branch.
by Sebastian Lackner Feb. 12, 2017

Feb. 12, 2017

On 08.10.2016 22:31, Gerald Pfeifer wrote: > + char cStr[sizeof(dent->d_name)+sizeof(procname_ide_media)], http://man7.org/linux/man-pages/man3/readdir.3.html says: """The standard also notes that the use of sizeof(d_name) is incorrect; use strlen(d_name) instead. (On some systems, this field is defined as char d_name[1]!)""" This means your solution will not work on all systems.

3 4

Re: [v2] preloader: Allow a debugger to find ld.so's rendezvous-struct
by Alexandre Julliard Dec. 11, 2016

Dec. 11, 2016

Keno Fischer <keno(a)juliacomputing.com> writes: > @@ -123,6 +123,18 @@ static struct wine_preload_info preload_info[] = > { 0, 0 } /* end of list */ > }; > > +/* A fake .dynamic section through which we can inform the debugger of the > + address of the chain-loaded dynamic linker's rendezvous struct */ > +__asm__( > +".section .dynamic,\"aw\"\n" > +".global _DYNAMIC\n" > +"_DYNAMIC: .quad 21\n" // DT_DEBUG > +".quad 0\n" > +".quad 0\n" > +".quad 0\n" > +".previous\n" > +); This is causing a warning here: gcc -m32 -c -o preloader.o preloader.c -I. -I../include -D__WINESRC__ -fno-builtin -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 -fno-omit-frame-pointer -Werror -g -O2 -fno-diagnostics-show-caret {standard input}: Assembler messages: {standard input}:5: Warning: setting incorrect section attributes for .dynamic -- Alexandre Julliard julliard(a)winehq.org

2 6

Do we delete spammers accounts from bugzilla?
by Bruno Jesus Nov. 28, 2016

Nov. 28, 2016

Today we had one more piece of spam, the bot registered the account a few moments before creating a bug. In this case should we use bugzilla's admin features to delete the user? Best wishes, Bruno

3 3

Re: [PATCH 5/7] ddraw: Discard the dst vertex buffer if possible.
by Henri Verbeet Nov. 13, 2016

Nov. 13, 2016

On 30 October 2016 at 15:02, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > Once we have buffer->buffer copies working we can load the source data > into a buffer in SetExecuteData and never have to lock the destination > buffer. In general I think wined3d_device_copy_sub_resource_region() is supposed to work for buffers. Is there something in particular that's broken?

2 1

Re: [PATCH] ws2_32: Fix the default behavior of IPV6_V6ONLY
by Marvin Nov. 1, 2016

Nov. 1, 2016

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=26345 Your paranoid android. === w2008s64 (64 bit sock) === sock: unhandled exception c0000005 at 000007FEFF126F94

3 2

d3dx9: d3dx9_base_effect_get_pass_desc
by Luis C. Busquets Pérez Oct. 31, 2016

Oct. 31, 2016

Dear developers, I have tried to run Civ4BeyondtheSword.exe with the internal d3dx9_33.dll I can run the programme but when I start a game, it hangs. I saw that a lot of FIXME's with the message: fixme:d3dx:d3dx9_base_effect_get_pass_desc Pixel shader and vertex shader are not supported, yet. Therefore, I tried to finishing implementing the function. Find below a patch that works. With this patch, the programme does not hang. Fell free to study, redo it or rewrite it but please I look forward to somebody who corrects it and manages to integrate that function in the wine tree so that I do not need to compile the wine. On the other hand, the settler image seems to something in the middle. Furthermore, the leaders also seem to have something in the middle. Actually, it seems that the image that should be behind the leader in the bottom is in the middle of them and bend vertically when they bend. I assume this is not because of the patch but I guess that this information could serve to locate and correct another bug. --- dlls/d3dx9_36/effect.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c index 42f5aea..c69b73c 100644 --- a/dlls/d3dx9_36/effect.c +++ b/dlls/d3dx9_36/effect.c @@ -150,6 +150,7 @@ struct d3dx9_base_effect UINT parameter_count; UINT technique_count; UINT object_count; + BOOL clonable; struct d3dx_parameter *parameters; struct d3dx_technique *techniques; @@ -999,6 +1000,11 @@ static HRESULT d3dx9_base_effect_get_pass_desc(struct d3dx9_base_effect *base, D3DXHANDLE pass, D3DXPASS_DESC *desc) { struct d3dx_pass *p = get_valid_pass(base, pass); + struct d3dx_state *s; + struct d3dx_parameter *param; + IDirect3DVertexShader9 *vshader; + IDirect3DPixelShader9 *pshader; + UINT i, size; if (!desc || !p) { @@ -1008,11 +1014,45 @@ static HRESULT d3dx9_base_effect_get_pass_desc(struct d3dx9_base_effect *base, desc->Name = p->name; desc->Annotations = p->annotation_count; - - FIXME("Pixel shader and vertex shader are not supported, yet.\n"); desc->pVertexShaderFunction = NULL; desc->pPixelShaderFunction = NULL; + if (base->clonable) + { + for (i=0; i < p->state_count; i++) + { + s = p->states + i; + if (state_table[s->operation].class == SC_VERTEXSHADER) + { + param = &(s->parameter); + vshader = *(struct IDirect3DVertexShader9 **)param- >data; + if (vshader) + { + IDirect3DVertexShader9_GetFunction(vshader, NULL, &size); + if (desc->pVertexShaderFunction) + free((void *)desc->pVertexShaderFunction); + desc->pVertexShaderFunction = malloc(size); + IDirect3DVertexShader9_GetFunction(vshader, (void *)(desc->pVertexShaderFunction), &size); + } + } + if (state_table[s->operation].class == SC_PIXELSHADER) + { + param = &(s->parameter); + pshader = *(struct IDirect3DPixelShader9 **)param- >data; + if (pshader) + { + IDirect3DPixelShader9_GetFunction(pshader, NULL, &size); + if (desc->pPixelShaderFunction) + free((void *)desc->pPixelShaderFunction); + desc->pPixelShaderFunction = malloc(size); + IDirect3DPixelShader9_GetFunction(pshader, (void *)(desc->pPixelShaderFunction), &size); + } + } + } + } + + TRACE("Pass '%s': %i annots. Vertexshader %p and Pixelshader %p\n", desc->Name, desc->Annotations, desc->pVertexShaderFunction, desc->pPixelShaderFunction); + return D3D_OK; } @@ -5955,6 +5995,8 @@ static HRESULT d3dx9_base_effect_init(struct d3dx9_base_effect *base, base->effect = effect; + base->clonable = TRUE; + read_dword(&ptr, &tag); TRACE("Tag: %x\n", tag); -- 2.7.4

4 4

Re: [PATCH 7/7] ddraw: Fix remaining style issues in executebuffer.c
by Henri Verbeet Oct. 31, 2016

Oct. 31, 2016

On 30 October 2016 at 15:02, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > Feel free to ignore this. With this the file follows the wined3d style, > unless I missed something... > +static void _dump_executedata(const D3DEXECUTEDATA *lpData) Naming conventions for "_dump_executedata" and "lpData". > TRACE("dwSize : %d\n", lpData->dwSize); %u for unsigned integers, punctuation. > -static void _dump_D3DEXECUTEBUFFERDESC(const D3DEXECUTEBUFFERDESC *lpDesc) { > +static void _dump_D3DEXECUTEBUFFERDESC(const D3DEXECUTEBUFFERDESC *lpDesc) > +{ > TRACE("dwSize : %d\n", lpDesc->dwSize); Likewise. > TRACE("dwFlags : %x\n", lpDesc->dwFlags); %#x for hexadecimal values. > @@ -76,12 +78,12 @@ HRESULT d3d_execute_buffer_execute(struct d3d_execute_buffer *buffer, > for (;;) > { > D3DINSTRUCTION *current = (D3DINSTRUCTION *)instr; Should probably be const. > - BYTE size; > - WORD count; > - > - count = current->wCount; > - size = current->bSize; > - instr += sizeof(D3DINSTRUCTION); > + BYTE size; > + WORD count; > + > + count = current->wCount; > + size = current->bSize; > + instr += sizeof(D3DINSTRUCTION); "sizeof(*current)" > instr = (char*)current + ci->dwOffset; "(char *)" > + default: > + ERR("Unhandled OpCode %d !!!\n",current->bOpcode); Should probably use %#x. Punctuation. Some of these probably deserve a patch on their own, and there may very well be more issues.

1 0

Re: [PATCH 6/7] ddraw: Implement executebuffer point and line draws.
by Henri Verbeet Oct. 31, 2016

Oct. 31, 2016

On 30 October 2016 at 15:02, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > case D3DOP_TRIANGLE: > { > WORD *indices; > - unsigned int index_pos = buffer->index_pos; > + unsigned int index_pos = buffer->index_pos, num_indices; Since you're doing style cleanups in the next patch, "index_count".

1 0

Re: [PATCH] wined3d: Invalidate INDEXBUFFER device state when contents of bound index buffer is changed.
by Henri Verbeet Oct. 31, 2016

Oct. 31, 2016

On 28 October 2016 at 12:20, Józef Kucia <jkucia(a)codeweavers.com> wrote: > + if (buffer->resource.bind_count) > + { > + if (buffer->buffer_type_hint == GL_ELEMENT_ARRAY_BUFFER) > + device_invalidate_state(buffer->resource.device, STATE_INDEXBUFFER); > + } You can merge those if-conditions. What happens exactly that makes this required?

2 3

Re: [PATCH 2/7] ddraw: Use wined3d_device_process_vertices for execute buffers.
by Henri Verbeet Oct. 31, 2016

Oct. 31, 2016

On 30 October 2016 at 15:02, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote: > @@ -318,65 +277,36 @@ HRESULT d3d_execute_buffer_execute(struct d3d_execute_buffer *buffer, > + hr = wined3d_resource_map(wined3d_buffer_get_resource(buffer->src_vertex_buffer), 0, > + &map_desc, &box, WINED3D_MAP_DISCARD); > if (FAILED(hr)) > return hr; > - dst = map_desc.data; > + memcpy(map_desc.data, src, ci->dwCount * sizeof(*src)); > + wined3d_resource_unmap(wined3d_buffer_get_resource(buffer->dst_vertex_buffer), 0); That looks wrong.

1 0

RE: [PATCH] xinput1_3: Prefer builtin DLL
by Filip Frackiewicz Oct. 30, 2016

Oct. 30, 2016

Hi Bruno, I hope I am not intruding or stating something already known,,, >- case DLL_WINE_PREATTACH: >- return FALSE; /* prefer native version */ case DLL_PROCESS_ATTACH: DisableThreadLibraryCalls(inst); break; This code looks different than the patch https://bugs.winehq.org/attachment.cgi?id=55851 attached to https://bugs.winehq.org/show_bug.cgi?id=41428 The patch attached to the bug did fix the issue in another game for me. Just thought I'd mention this just in case :) Cheers, Filip

2 2

Re: [PATCH 6/7] ddraw: Implement executebuffer point and line draws.
by Stefan Dösinger Oct. 30, 2016

Oct. 30, 2016

Am 2016-10-30 um 07:02 schrieb Stefan Dösinger: > + case D3DOP_LINE: > + primitive_size = 2; > + wined3d_device_set_primitive_type(device->wined3d_device, WINED3D_PT_LINELIST); I should probably mention the purpose of this patch: It fixes the minimap and bike performance display in Moto Racer 2.

1 0

Imort system certificates in crypt32@Wine
by Donat Enikeev Oct. 28, 2016

Oct. 28, 2016

2 6

Re: [PATCH] d2d1: Use transformed geometry to translate glyph run outline
by Henri Verbeet Oct. 28, 2016

Oct. 28, 2016

On 28 October 2016 at 13:23, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/d2d1/render_target.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > This is probably fine, but why would you want to do that?

2 1

Re: [v2 PATCH] ucrtbase: Added _sopen_dispatch/_wsopen_dispatch
by Huw Davies Oct. 28, 2016

Oct. 28, 2016

On Fri, Oct 28, 2016 at 01:43:19PM +0300, Nikolay Sivov wrote: > diff --git a/dlls/msvcrt/file.c b/dlls/msvcrt/file.c > index 432d5c7..37e5ece 100644 > --- a/dlls/msvcrt/file.c > +++ b/dlls/msvcrt/file.c > @@ -2144,9 +2144,10 @@ static int check_bom(HANDLE h, int oflags, BOOL seek) > } > > /********************************************************************* > - * _wsopen_s (MSVCRT.@) > + * _wsopen_dispatch (UCRTBASE.@) > */ > -int CDECL MSVCRT__wsopen_s( int *fd, const MSVCRT_wchar_t* path, int oflags, int shflags, int pmode ) > +int CDECL MSVCRT__wsopen_dispatch( const MSVCRT_wchar_t* path, int oflags, int shflags, int pmode, > + int *fd, int secure ) > { > DWORD access = 0, creation = 0, attrib; > SECURITY_ATTRIBUTES sa; You probably want to add 'secure' to the TRACE() just below here and change the order of the parameters in that TRACE() to match the new function. Huw.

3 3

Re: [v3 PATCH] ucrtbase: Added _sopen_dispatch/_wsopen_dispatch
by Huw Davies Oct. 28, 2016

Oct. 28, 2016

Signed-off-by: Huw Davies <huw(a)codeweavers.com>

1 0

Re: [PATCH] winebus.sys: Remove dead code
by Sebastian Lackner Oct. 28, 2016

Oct. 28, 2016

It is dead code, but I don't think putting it in a comment makes it better. In general, commented out code should be avoided because it is not checked by the compiler. In addition, we might want to implement driver unloading in the near future, and will have to add the code back then (including the "if" because failures to create the monitor are not critical). 2016-10-28 3:00 GMT+02:00 Aric Stewart <aric(a)codeweavers.com>: > > Coverity 1373717 > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus_udev.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > >

2 1

Cmd.exe test cases
by Carlos Rafael Ramirez Oct. 28, 2016

Oct. 28, 2016

Hello, I want to add some test cases to cmd program. But I don't understand the structure of them. Where to put the command and where to put the assertion or expected output? I am coding a patch and I want to make it like TDD, first the test fail and after the fix the test passes. Thank you

2 1

Re: [3/3] hidclass.sys: Only return new reports when reading data from ringbuffer.
by Aric Stewart Oct. 27, 2016

Oct. 27, 2016

On 10/27/16 9:45 AM, Sebastian Lackner wrote: > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > > Depending on the buffer size, we might get a huge amount of old reports otherwise. > > @ Aric: I believe there are some more things wrong with the ringbuffer implementation. > We shouldn't really need a ->start pointer, this is exactly what the ring->pointers[] > are used for. Also, the current implementation never uses all slots. > Reviews on the ringbuffer code is very welcome! Thanks! It has been a while since I did that implementation but if I recall the purpose for start and end is about tracking the writing positions, It was more important before this patch, but after this patch you are correct that start is no longer needed. 2 -aric > dlls/hidclass.sys/buffer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/hidclass.sys/buffer.c b/dlls/hidclass.sys/buffer.c > index 14e83e4..0b29f97 100644 > --- a/dlls/hidclass.sys/buffer.c > +++ b/dlls/hidclass.sys/buffer.c > @@ -170,7 +170,7 @@ UINT RingBuffer_AddPointer(struct ReportRingBuffer *ring) > for( ;count < ring->pointer_alloc; count++) > ring->pointers[count] = POINTER_UNUSED; > } > - ring->pointers[idx] = ring->start; > + ring->pointers[idx] = ring->end; > LeaveCriticalSection(&ring->lock); > return idx; > } >

1 0

Re: [PATCH] ucrtbase: Added _sopen_dispatch/_wsopen_dispatch
by Huw Davies Oct. 27, 2016

Oct. 27, 2016

On 27 Oct 2016, at 17:17, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > diff --git a/dlls/msvcrt/file.c b/dlls/msvcrt/file.c > index 432d5c7..9097598 100644 > --- a/dlls/msvcrt/file.c > +++ b/dlls/msvcrt/file.c > @@ -2330,6 +2330,18 @@ int CDECL MSVCRT__wsopen( const MSVCRT_wchar_t *path, int oflags, int shflags, . > } > > /********************************************************************* > + * _wsopen_dispatch (UCRTBASE.@) > + */ > +int CDECL MSVCRT__wsopen_dispatch( const MSVCRT_wchar_t *path, int oflags, int shflags, > + int pmode, int *filehandle, int secure) > +{ > + if (secure) > + FIXME("secure argument ignored.\n"); > + > + return MSVCRT__wsopen_s( filehandle, path, oflags, shflags, pmode ); > +} Wouldn’t it make more sense to have _wsopen() and _wsopen_s() call _wsopen_dispatch() (and likewise for the char* version)? Huw.

1 0

Re: ntdll: Add ApiSetQueryApiSetPresence stub.
by Nikolay Sivov Oct. 27, 2016

Oct. 27, 2016

Do you have a program that needs this?

1 0

Re: ntdll: Forward EtwEventRegister() from EventRegister().
by Nikolay Sivov Oct. 27, 2016

Oct. 27, 2016

> --- a/dlls/ntdll/ntdll.spec > +++ b/dlls/ntdll/ntdll.spec > @@ -41,6 +41,7 @@ > # @ stub DbgUiStopDebugging > @ stub DbgUiWaitStateChange > @ stdcall DbgUserBreakPoint() > +@ stdcall EtwEventRegister(ptr ptr ptr ptr) advapi32.EventRegister This won't do it. Advapi32 should forward to ntdll, not the other way around.

1 0

Re: [PATCH 1/2] edit: Test that EM_REPLACESEL works after EM_GETHANDLE.
by Sebastian Lackner Oct. 26, 2016

Oct. 26, 2016

On 27.10.2016 00:41, Roman Pišl wrote: > Signed-off-by: Roman Pišl <rpisl(a)seznam.cz> > --- > dlls/user32/tests/edit.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) I haven't reviewed the patch yet, but please note that the subject should start with the component (here user32), not with the filename. It was also wrong in http://source.winehq.org/git/wine.git/patch/ddfc5275af0081cba6e834bf9d9e35d…, but seems like Alexandre didn't notice it. ;) Best regards, Sebastian

1 0

d3dx9: d3dx9_base_effect_get_pass_desc
by Luis C. Busquets Pérez Oct. 26, 2016

Oct. 26, 2016

Dear developers, I have tried to run Civ4BeyondtheSword.exe with the internal d3dx9_33.dll I can run the programme but when I start a game, it hangs. I saw that a lot of FIXME's with the message: fixme:d3dx:d3dx9_base_effect_get_pass_desc Pixel shader and vertex shader are not supported, yet. Therefore, I tried to finishing implementing the function. Find attached a patch that works. With this patch, the programme does not hang. Fell free to study, redo it or rewrite it but please I look forward to somebody who corrects it and manages to integrate that function in the wine tree so that I do not need to compile the wine. On the other hand, find attached an image, where you can see that the settler image seems to something in the middle. Furthermore, the leaders also seem to have something in the middle. Actually, it seems that the image that should be behind the leader in the bottom is in the middle of them and bend vertically when they bend. I assume this is not because of the patch but I guess that this information could serve to locate and correct another bug.

1 0

Re: [PATCH] hid: implement HidD_SetOutputReport
by Sebastian Lackner Oct. 26, 2016

Oct. 26, 2016

On 24.10.2016 16:06, Aric Stewart wrote: > Also fixes an issue with IOCTL_HID_SET_FEATURE where the wrong buffers where being sent. > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/hid/hid.spec | 2 +- > dlls/hid/hidd.c | 6 ++++++ > dlls/hidclass.sys/device.c | 11 +++++++---- > include/ddk/hidsdi.h | 1 + > 4 files changed, 15 insertions(+), 5 deletions(-) > While you are just at it, shouldn't the first char in SystemBuffer be skipped? "packet.reportBuffer = irp->AssociatedIrp.SystemBuffer;" Also, it looks like the first assignment of "irp->IoStatus.Information = 0;" is not really required, its also assigned later. Regards, Sebastian

1 0

Re: [6/6] webservices: Implement WsCall.
by Marvin Oct. 26, 2016

Oct. 26, 2016

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

1 0

Re: [PATCH 1/4] d2d1: Implement FillContainsPoint for rectangles
by Nikolay Sivov Oct. 26, 2016

Oct. 26, 2016

On Fri, Oct 21, 2016 at 7:28 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: > > + *contains = probe.x > geometry->u.rectangle.rect.left - tolerance && > + probe.x < geometry->u.rectangle.rect.right + tolerance && > + probe.y > geometry->u.rectangle.rect.top - tolerance && > + probe.y < geometry->u.rectangle.rect.bottom + tolerance; > + > Does it really work like that? I'd expect tolerance to be irrelevant for paths constructed purely from lines. So basically in general case path is approximated with lines (or enclosed area with triangles) with given tolerance, and then point is tested against this flattened path. While in rectangular case it's already flat to begin with.

2 1

Product Registration Bug
by Jonathan Allen Oct. 25, 2016

Oct. 25, 2016

Dear List, Do we have any movement on bug 29354? I'm trying to move to an all-Linux environment and am blocked on two (old, venerable) XP (but not MS) applications that go to sleep in the ... fixme:msi:msi_validate_product_id partial stud: template ... ... code and can't move beyond that point. Completely stuck - help! Jonathan

1 0

Re: [PATCH 01/10 v2] server: Introduced iosb struct for server-side IO_STATUS_BLOCK representation and use it in irp_call.
by Sebastian Lackner Oct. 25, 2016

Oct. 25, 2016

On 19.10.2016 19:05, Jacek Caban wrote: > v2: Don't store async queues in a list. > > Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> > --- > server/async.c | 34 +++++++++++++++++++++++++++++++++ > server/device.c | 58 > +++++++++++++++++++++++++-------------------------------- > server/file.h | 14 ++++++++++++++ > 3 files changed, 73 insertions(+), 33 deletions(-) > I'm not sure if Alexandre has already agreed to this approach, but from my point of view it would make sense to do some measurements. How fast is the proposed named pipe approach compared to alternatives (unpatched Wine, or the Linux-only implementation included in Wine Staging)? In general I like the idea, but I fear that adding significant optimizations afterwards will be very difficult. Regards, Sebastian

3 6

Re: Polish translation
by Nikolay Sivov Oct. 25, 2016

Oct. 25, 2016

On 24.10.2016 21:55, m4sk1n wrote: > Here is translated .po file from my GlotPress. Hope it will be used. > Hi. Thank you for your contribution. Translations should be submitted as regular patches, generated using git format-patch or similar. Also make sure to use your real name when submitting.

1 0

Testing: gcc -foutline-msabi-xlogues patch
by Daniel Santos Oct. 24, 2016

Oct. 24, 2016

I've been working on a gcc patch targeting 64-bit Wine to reduce the impact of the implicit register clobbers when a Microsoft's x64 function calls a System V ABI function and I appear to finally have a working implementation. However, I cannot get recent Wine tests to complete with an unpatched compiler (I have 30 failures, 4 which are crashes and I even get some dmesgs from drivers.) While the same Wine tests pass both with and without the new optimization, I could use some external testing to make sure there's no flaws that I've missed (especially a full test with no failures). The patch set is not actually complete as my unit tests didn't catch many of the flaws (now fixed) that I caught when building real Wine, so I'm re-working them. I have my current version up on github: https://github.com/daniel-santos/gcc/tree/gcc-5_4_0-outline-msabi-xlogues. Background For those unfamiliar, the differences between the two ABIs requires an ms_abi function to save and restore 12 registers: RSI, RDI and XMM6-15. The size of pushing/popping RSI and RDI is insignificant, but saving and restoring 10 SSE registers takes between 94 and 174 bytes each function (dependent upon the SP offset), thereby bloating ".text" size quite a bit. Details This patch uses out-of-line stubs for register for saves/restores to reduce .text size. The *theory* (not yet proven) is that the reduction of instruction cache misses will offset the extra few instructions required to facilitate this. But while we're going this far, the optimization also saves/restores an additional 6 non-volatile ms_abi registers if they are clobbered in the function: RBX, RBP and R12-15. This extra step actually saves very little ".text" code, but the stub implements them using MOVs which the CPU is better able to parallelize. While MOV instructions are larger (4 bytes vs 1 or 2), they only appear in the stubs, theoretically gaining the performance benefits of MOVs without paying for the cost of the extra size. This optimization works both with and without forced stack realignment (https://bugs.winehq.org/show_bug.cgi?id=27680) although more can be done to produce better code in the realigned case. In the forced realignment case, this optimization reduces .text size Wine 64 by 19% and in the normal (aligned stack) case by 22%. As an example, here is a test function (with stubs) built with -foutline-msabi-xlogues (but no realignment): __attribute__ ((noinline)) long sysv_fn0 () { return 42; } __attribute__ ((ms_abi, noinline)) long msabifn0_6 () { __asm__ __volatile__ ("" ::: "rbx", "rbp", "r12", "r13", "r14", "r15"); return sysv_fn0 (); } 0000000000401000 <msabifn0_6>: 401000: 48 8d 44 24 88 lea -0x78(%rsp),%rax 401005: 48 81 ec f8 00 00 00 sub $0xf8,%rsp 40100c: e8 09 e8 01 00 callq 41f81a <__msabi_save_18> 401011: 31 c0 xor %eax,%eax 401013: e8 48 fc ff ff callq 400c60 <sysv_fn0> 401018: 48 8d b4 24 80 00 00 lea 0x80(%rsp),%rsi 40101f: 00 401020: 4c 8d 56 78 lea 0x78(%rsi),%r10 401024: e9 41 e8 01 00 jmpq 41f86a <__msabi_restore_ret_18> 000000000041f81a <__msabi_save_18>: 41f81a: 4c 89 78 90 mov %r15,-0x70(%rax) 000000000041f81e <__msabi_save_17>: 41f81e: 4c 89 70 98 mov %r14,-0x68(%rax) 000000000041f822 <__msabi_save_16>: 41f822: 4c 89 68 a0 mov %r13,-0x60(%rax) 000000000041f826 <__msabi_save_15>: 41f826: 4c 89 60 a8 mov %r12,-0x58(%rax) 000000000041f82a <__msabi_save_14>: 41f82a: 48 89 68 b0 mov %rbp,-0x50(%rax) 000000000041f82e <__msabi_save_13>: 41f82e: 48 89 58 b8 mov %rbx,-0x48(%rax) 000000000041f832 <__msabi_save_12>: 41f832: 48 89 78 c0 mov %rdi,-0x40(%rax) 41f836: 48 89 70 c8 mov %rsi,-0x38(%rax) 41f83a: 44 0f 29 78 d0 movaps %xmm15,-0x30(%rax) 41f83f: 44 0f 29 70 e0 movaps %xmm14,-0x20(%rax) 41f844: 44 0f 29 68 f0 movaps %xmm13,-0x10(%rax) 41f849: 44 0f 29 20 movaps %xmm12,(%rax) 41f84d: 44 0f 29 58 10 movaps %xmm11,0x10(%rax) 41f852: 44 0f 29 50 20 movaps %xmm10,0x20(%rax) 41f857: 44 0f 29 48 30 movaps %xmm9,0x30(%rax) 41f85c: 44 0f 29 40 40 movaps %xmm8,0x40(%rax) 41f861: 0f 29 78 50 movaps %xmm7,0x50(%rax) 41f865: 0f 29 70 60 movaps %xmm6,0x60(%rax) 41f869: c3 retq 000000000041f86a <__msabi_restore_ret_18>: 41f86a: 4c 8b 7e 90 mov -0x70(%rsi),%r15 000000000041f86e <__msabi_restore_ret_17>: 41f86e: 4c 8b 76 98 mov -0x68(%rsi),%r14 000000000041f872 <__msabi_restore_ret_16>: 41f872: 4c 8b 6e a0 mov -0x60(%rsi),%r13 000000000041f876 <__msabi_restore_ret_15>: 41f876: 4c 8b 66 a8 mov -0x58(%rsi),%r12 000000000041f87a <__msabi_restore_ret_14>: 41f87a: 48 8b 6e b0 mov -0x50(%rsi),%rbp 000000000041f87e <__msabi_restore_ret_13>: 41f87e: 48 8b 5e b8 mov -0x48(%rsi),%rbx 000000000041f882 <__msabi_restore_ret_12>: 41f882: 48 8b 7e c0 mov -0x40(%rsi),%rdi 41f886: 44 0f 28 7e d0 movaps -0x30(%rsi),%xmm15 41f88b: 44 0f 28 76 e0 movaps -0x20(%rsi),%xmm14 41f890: 44 0f 28 6e f0 movaps -0x10(%rsi),%xmm13 41f895: 44 0f 28 26 movaps (%rsi),%xmm12 41f899: 44 0f 28 5e 10 movaps 0x10(%rsi),%xmm11 41f89e: 44 0f 28 56 20 movaps 0x20(%rsi),%xmm10 41f8a3: 44 0f 28 4e 30 movaps 0x30(%rsi),%xmm9 41f8a8: 44 0f 28 46 40 movaps 0x40(%rsi),%xmm8 41f8ad: 0f 28 7e 50 movaps 0x50(%rsi),%xmm7 41f8b1: 0f 28 76 60 movaps 0x60(%rsi),%xmm6 41f8b5: 48 8b 76 c8 mov -0x38(%rsi),%rsi 41f8b9: 4c 89 d4 mov %r10,%rsp 41f8bc: c3 retq Here is a similar function with forced realignment. This can be improved with an additional "have frame pointer" set of stubs. __attribute__ ((ms_abi, noinline, force_align_arg_pointer)) long msabifn0_5 () { __asm__ __volatile__ ( "" ::: "rbx", "r12", "r13", "r14", "r15" ); return sysv_fn0 (); } 0000000000400fc0 <msabifn0_5>: 400fc0: 55 push %rbp 400fc1: 48 8d 44 24 90 lea -0x70(%rsp),%rax 400fc6: 48 89 e5 mov %rsp,%rbp 400fc9: 48 83 e0 f0 and $0xfffffffffffffff0,%rax 400fcd: 48 8d 60 b0 lea -0x50(%rax),%rsp 400fd1: e8 78 e8 01 00 callq 41f84e <__msabi_save_13> 400fd6: 41 57 push %r15 400fd8: 41 56 push %r14 400fda: 41 55 push %r13 400fdc: 41 54 push %r12 400fde: 31 c0 xor %eax,%eax 400fe0: e8 7b fc ff ff callq 400c60 <sysv_fn0> 400fe5: 48 8d a5 20 ff ff ff lea -0xe0(%rbp),%rsp 400fec: 41 5c pop %r12 400fee: 41 5d pop %r13 400ff0: 41 5e pop %r14 400ff2: 41 5f pop %r15 400ff4: 48 8d 74 24 50 lea 0x50(%rsp),%rsi 400ff9: e8 a0 e8 01 00 callq 41f89e <__msabi_restore_13> 400ffe: 48 81 c4 c0 00 00 00 add $0xc0,%rsp 401005: 5d pop %rbp 401006: c3 retq Daniel

1 2

Re: [PATCH 09/10] kernel32/tests: Added more named pipe tests.
by Marvin Oct. 24, 2016

Oct. 24, 2016

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=26285 Your paranoid android. === w864 (64 bit pipe) === pipe.c:2575: Test failed: FlushFileBuffers failed: 87 pipe.c:2575: Test failed: FlushFileBuffers failed: 87

1 0

Stack realignment configure option?
by Daniel Santos Oct. 22, 2016

Oct. 22, 2016

I've read up a bit on the issues with realignment on Wine 64 and all of the programs that crash because they luckily get away with violating the ABI on Windows. While not crashing is a pretty good thing, forcing stack realignment also affects optimizations in some less obvious ways, inhibiting sibling calls, ret from function bodies, adding a frame pointer, etc. I don't actually have any bright ideas for this atm, but can we at least add a configure option to disable this for those who don't happen to have such programs crashing? Probably only Gentoo users and geeks will care for it.

1 1

TRACE in wined3d_check_device_format()
by wylda＠volny.cz Oct. 22, 2016

Oct. 22, 2016

Hi, isn't that middle \n in TRACE some kind of leftover? TRACE("wined3d %p, adapter_idx %u, device_type %s, adapter_format %s, usage %s, %s,\n" "resource_type %s, check_format %s.\n", wined3d, adapter_idx, debug_d3ddevicetype(device_type), debug_d3dformat(adapter_format_id), debug_d3dusage(usage), debug_d3dusagequery(usage), debug_d3dresourcetype(resource_type), debug_d3dformat(check_format_id)); Regards, W.

2 1

How to make winedbg listen to same port every time?
by A B Oct. 21, 2016

Oct. 21, 2016

If i run : winedbg --gdb --no-start test.exe it say : target remote localhost:55304 But if I run it second time , it say a different port , for example target remote localhost:111111 is there a command line argument for winedbg so it to listen to same port every time , and to do not change the port ,so I can specity to witch port it to listen , or I have to recompile winedbg ? Thank you!

2 2

Re: [PATCH 6/9] wined3d: Replace wined3d_buffer_load_sysmem() calls with wined3d_buffer_load_location().
by Henri Verbeet Oct. 21, 2016

Oct. 21, 2016

On 20 October 2016 at 12:50, Józef Kucia <jkucia(a)codeweavers.com> wrote: > - /* Download the buffer, but don't permanently enable double buffering */ > - if (!(buffer->flags & WINED3D_BUFFER_DOUBLEBUFFER)) > - { > - wined3d_buffer_load_sysmem(buffer, context); > - buffer->flags &= ~WINED3D_BUFFER_DOUBLEBUFFER; > - } > + /* Download the buffer, but don't permanently enable double buffering. */ > + wined3d_buffer_load_location(buffer, context, WINED3D_LOCATION_SYSMEM); > + buffer->flags &= ~WINED3D_BUFFER_DOUBLEBUFFER; Wouldn't this also clear WINED3D_BUFFER_DOUBLEBUFFER if it was previously set? Longer term, it's probably inappropriate for wined3d_buffer_load_location() to set WINED3D_BUFFER_DOUBLEBUFFER to begin with.

1 0

Re: [PATCH 5/9] wined3d: Implement wined3d_buffer_prepare_location() for WINED3D_LOCATION_BUFFER.
by Henri Verbeet Oct. 21, 2016

Oct. 21, 2016

On 20 October 2016 at 12:50, Józef Kucia <jkucia(a)codeweavers.com> wrote: > @@ -612,7 +619,7 @@ void buffer_get_memory(struct wined3d_buffer *buffer, struct wined3d_context *co > { > if ((buffer->flags & WINED3D_BUFFER_USE_BO) && !buffer->resource.map_count) > { > - buffer_create_buffer_object(buffer, context); > + wined3d_buffer_prepare_location(buffer, context, WINED3D_LOCATION_BUFFER); > if (buffer->buffer_object) Note that that should be equivalent to "if (wined3d_buffer_prepare_location(buffer, context, WINED3D_LOCATION_BUFFER))".

1 0

Re: [PATCH v3] winebus.sys: Handle device reports for hidraw devices
by Sebastian Lackner Oct. 21, 2016

Oct. 21, 2016

On 17.10.2016 17:14, Aric Stewart wrote: > v2: Suggestions from Sebastian Lackner > v3: Further suggestions from Sebastian Lackner > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus.h | 3 + > dlls/winebus.sys/bus_udev.c | 73 ++++++++++++++++++++++++ > dlls/winebus.sys/main.c | 133 +++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 208 insertions(+), 1 deletion(-) > > Now that all the handling for last_report is in main.c, I would suggest to split it out into a separate patch for easier reviewing. The first patch could contain IOCTL_HID_READ_REPORT and corresponding platform code only. Caching of the last record can then be implemented in the second patch. > > v3-0003-winebus.sys-Handle-device-reports-for-hidraw-device.txt > > > diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h > index 7ce54bf..339449d 100644 > --- a/dlls/winebus.sys/bus.h > +++ b/dlls/winebus.sys/bus.h > @@ -25,6 +25,8 @@ typedef struct > int (*compare_platform_device)(DEVICE_OBJECT *device, void *platform_dev); > NTSTATUS (*get_reportdescriptor)(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length); > NTSTATUS (*get_string)(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length); > + NTSTATUS (*begin_report_processing)(DEVICE_OBJECT *device); > + void (*end_report_processing)(DEVICE_OBJECT *device); > } platform_vtbl; > > void *get_platform_private(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > @@ -37,3 +39,4 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) DECLSPEC_HIDDEN; > void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; > +void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD length) DECLSPEC_HIDDEN; > diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c > index 243aabb..517a7d3 100644 > --- a/dlls/winebus.sys/bus_udev.c > +++ b/dlls/winebus.sys/bus_udev.c > @@ -61,6 +61,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > > #ifdef HAVE_UDEV > > +WINE_DECLARE_DEBUG_CHANNEL(hid_report); > + > static struct udev *udev_context = NULL; > static DRIVER_OBJECT *udev_driver_obj = NULL; > > @@ -73,6 +75,11 @@ struct platform_private > { > struct udev_device *udev_device; > int device_fd; > + > + BYTE *report_buffer; > + int buffer_length; > + HANDLE report_thread; > + int control_pipe[2]; > }; > > static inline struct platform_private *impl_from_DEVICE_OBJECT(DEVICE_OBJECT *device) > @@ -222,11 +229,77 @@ static NTSTATUS hidraw_get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buf > return STATUS_SUCCESS; > } > > +static DWORD CALLBACK device_report_thread(void *args) > +{ > + DEVICE_OBJECT *device = (DEVICE_OBJECT*)args; > + struct platform_private *private = impl_from_DEVICE_OBJECT(device); > + struct pollfd plfds[2]; > + > + plfds[0].fd = private->device_fd; > + plfds[0].events = POLLIN; > + plfds[0].revents = 0; > + plfds[1].fd = private->control_pipe[0]; > + plfds[1].events = POLLIN; > + plfds[1].revents = 0; > + > + while (1) > + { > + int size; > + if (poll(plfds, 2, -1) <= 0) continue; > + if (plfds[1].revents || !private->report_buffer || private->buffer_length == 0) > + break; > + size = read(plfds[0].fd, private->report_buffer, private->buffer_length); > + if (size == -1) > + TRACE_(hid_report)("Read failed. Likely an unplugged device\n"); > + else if (size == 0) > + TRACE_(hid_report)("Failed to read report\n"); > + else > + process_hid_report(device, private->report_buffer, size); > + } > + return 0; > +} > + > +static NTSTATUS begin_report_processing(DEVICE_OBJECT *device) > +{ > + struct platform_private *private = impl_from_DEVICE_OBJECT(device); > + private->buffer_length = 1024; > + private->report_buffer = HeapAlloc(GetProcessHeap(), 0, private->buffer_length); My understanding is that report_buffer is only used from the device_report_thread. Do we really have to store it in the platform private data then? For such a small amount of data, you could probably even allocate it directly on the stack in device_report_thread. > + > + pipe(private->control_pipe); Please also check for success here. > + private->report_thread = CreateThread(NULL, 0, device_report_thread, device, 0, NULL); > + if (!private->report_thread) > + { > + ERR("Unable to create device report thread\n"); > + close(private->control_pipe[0]); > + close(private->control_pipe[1]); > + return STATUS_UNSUCCESSFUL; > + } > + else > + return STATUS_SUCCESS; > +} > + > +static void end_report_processing(DEVICE_OBJECT *device) > +{ > + struct platform_private *private = impl_from_DEVICE_OBJECT(device); > + if (private->report_thread) > + { > + write(private->control_pipe[1], "q", 1); > + WaitForSingleObject(private->report_thread, INFINITE); > + close(private->control_pipe[0]); > + close(private->control_pipe[1]); report_thread should also be closed when no longer needed. > + } > + private->buffer_length = 0; > + HeapFree(GetProcessHeap(), 0, private->report_buffer); > + private->report_buffer = NULL; > +} > + > static const platform_vtbl hidraw_vtbl = > { > compare_platform_device, > hidraw_get_reportdescriptor, > hidraw_get_string, > + begin_report_processing, > + end_report_processing, > }; > > static void try_add_device(struct udev_device *dev) > diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c > index 86adbf5..0bd2c76 100644 > --- a/dlls/winebus.sys/main.c > +++ b/dlls/winebus.sys/main.c > @@ -40,6 +40,7 @@ > #include "bus.h" > > WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > +WINE_DECLARE_DEBUG_CHANNEL(hid_report); > > struct pnp_device > { > @@ -58,6 +59,14 @@ struct device_extension > const WCHAR *busid; /* Expected to be a static constant */ > > const platform_vtbl *vtbl; > + > + BOOL processing_started; > + BYTE *last_report; > + DWORD last_report_size; > + DWORD buffer_size; > + LIST_ENTRY irp_queue; > + CRITICAL_SECTION report_cs; > + > BYTE platform_private[1]; > }; > > @@ -214,6 +223,10 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > ext->busid = busidW; > ext->vtbl = vtbl; > It might be better to explicitly initialize all fields here. I don't know if you can rely on the fact that Wines current implementation nulls the driver extension. > + InitializeListHead(&ext->irp_queue); > + InitializeCriticalSection(&ext->report_cs); > + ext->report_cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": report_cs"); > + > /* add to list of pnp devices */ > pnp_dev->device = device; > list_add_tail(&pnp_devset, &pnp_dev->entry); > @@ -271,6 +284,8 @@ void bus_remove_hid_device(DEVICE_OBJECT *device) > { > struct device_extension *ext = (struct device_extension *)device->DeviceExtension; > struct pnp_device *pnp_device = ext->pnp_device; > + LIST_ENTRY *entry; > + IRP *irp; > > TRACE("(%p)\n", device); > > @@ -279,7 +294,28 @@ void bus_remove_hid_device(DEVICE_OBJECT *device) > LeaveCriticalSection(&device_list_cs); > > IoInvalidateDeviceRelations(device, RemovalRelations); > + > + if (ext->processing_started) > + ext->vtbl->end_report_processing(device); At this point the device_fd was already closed (too early). Do we really need the callback, or could we also do the platform specific cleanup in try_remove_device()? > + > + /* Cancel pending IRPs */ > + EnterCriticalSection(&ext->report_cs); > + entry = RemoveHeadList(&ext->irp_queue); > + while(entry != &ext->irp_queue) You could write this loop a bit easier like: while ((entry = RemoveHeadList(&ext->irp_queue)) != &ext->irp_queue) { ... } The same also applies to the code below. > + { > + irp = CONTAINING_RECORD(entry, IRP, Tail.Overlay.ListEntry); > + irp->IoStatus.u.Status = STATUS_CANCELLED; > + irp->IoStatus.Information = 0; > + IoCompleteRequest(irp, IO_NO_INCREMENT); > + entry = RemoveHeadList(&ext->irp_queue); > + } > + LeaveCriticalSection(&ext->report_cs); > + > + ext->report_cs.DebugInfo->Spare[0] = 0; > + DeleteCriticalSection(&ext->report_cs); > + > HeapFree(GetProcessHeap(), 0, ext->serial); > + HeapFree(GetProcessHeap(), 0, ext->last_report); > IoDeleteDevice(device); > > /* pnp_device must be released after the device is gone */ > @@ -432,6 +468,58 @@ NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) > irp->IoStatus.Information = (strlenW((WCHAR *)irp->UserBuffer) + 1) * sizeof(WCHAR); > break; > } > + case IOCTL_HID_GET_INPUT_REPORT: > + { > + HID_XFER_PACKET *packet = (HID_XFER_PACKET*)(irp->UserBuffer); > + TRACE_(hid_report)("IOCTL_HID_GET_INPUT_REPORT\n"); > + EnterCriticalSection(&ext->report_cs); > + if (!ext->processing_started) > + { > + irp->IoStatus.u.Status = status = ext->vtbl->begin_report_processing(device); > + if (status == STATUS_SUCCESS) > + ext->processing_started = TRUE; > + else > + { > + LeaveCriticalSection(&ext->report_cs); > + break; > + } > + } > + > + if (packet->reportBufferLen < ext->last_report_size) > + { > + irp->IoStatus.u.Status = status = STATUS_BUFFER_TOO_SMALL; > + LeaveCriticalSection(&ext->report_cs); > + break; > + } > + > + if (ext->last_report_size) > + memcpy(packet->reportBuffer, ext->last_report, ext->last_report_size); > + packet->reportBufferLen = ext->last_report_size; > + irp->IoStatus.Information = ext->last_report_size; > + irp->IoStatus.u.Status = status = STATUS_SUCCESS; > + LeaveCriticalSection(&ext->report_cs); > + break; > + } > + case IOCTL_HID_READ_REPORT: > + { > + TRACE_(hid_report)("IOCTL_HID_READ_REPORT\n"); > + EnterCriticalSection(&ext->report_cs); > + if (!ext->processing_started) > + { > + if (ext->vtbl->begin_report_processing(device) == STATUS_SUCCESS) Did you want to assign the result to "status" here? Also, it is a bit unfortunate that we have tracking of report processing in both the common code and also in the platform specific code. If you modify begin_report_processing to return STATUS_SUCCESS if it already has been started, you could simplify this code to something like: --- snip --- status = ext->vtbl->begin_report_processing(device); if (status != STATUS_SUCCESS) { ... } --- snip --- It also ensures that the status in both components never gets out of sync. Not sure if it is a problem, but due to the fact that the device has been opened a long time before starting to read from it, couldn't there be a large amount of reports in the kernel buffers? Do you think it makes sense to skip such outdated reports? > + ext->processing_started = TRUE; > + else > + { > + irp->IoStatus.u.Status = status; > + LeaveCriticalSection(&ext->report_cs); > + break; > + } > + } > + InsertTailList(&ext->irp_queue, &irp->Tail.Overlay.ListEntry); > + status = STATUS_PENDING; > + LeaveCriticalSection(&ext->report_cs); > + break; > + } > default: > { > ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode; > @@ -441,11 +529,54 @@ NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) > } > } > > - IoCompleteRequest(irp, IO_NO_INCREMENT); > + if (status != STATUS_PENDING) > + IoCompleteRequest(irp, IO_NO_INCREMENT); > > return status; > } > > +void process_hid_report(DEVICE_OBJECT *device, BYTE *report, DWORD length) > +{ > + struct device_extension *ext = (struct device_extension*)device->DeviceExtension; > + IRP *irp; > + LIST_ENTRY *entry; > + > + EnterCriticalSection(&ext->report_cs); > + if (length > ext->buffer_size) > + { > + HeapFree(GetProcessHeap(), 0, ext->last_report); > + ext->last_report = HeapAlloc(GetProcessHeap(), 0, length); It would be better to handle failures here. > + ext->buffer_size = length; > + } > + > + memcpy(ext->last_report, report, length); > + ext->last_report_size = length; > + > + entry = RemoveHeadList(&ext->irp_queue); > + while(entry != &ext->irp_queue) > + { > + IO_STACK_LOCATION *irpsp; > + TRACE_(hid_report)("Processing Request\n"); > + irp = CONTAINING_RECORD(entry, IRP, Tail.Overlay.ListEntry); > + irpsp = IoGetCurrentIrpStackLocation(irp); > + > + if (irpsp->Parameters.DeviceIoControl.OutputBufferLength < length) > + { > + irp->IoStatus.Information = 0; > + irp->IoStatus.u.Status = STATUS_BUFFER_TOO_SMALL; > + } > + else > + { > + memcpy(irp->UserBuffer, report, length); > + irp->IoStatus.Information = length; > + irp->IoStatus.u.Status = STATUS_SUCCESS; > + } > + IoCompleteRequest(irp, IO_NO_INCREMENT); > + entry = RemoveHeadList(&ext->irp_queue); > + } > + LeaveCriticalSection(&ext->report_cs); > +} > + > NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) > { > static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0}; > > >

2 8

Re: [PATCH] hidclass.sys: Use IoSetCompletionRoutine
by Aric Stewart Oct. 20, 2016

Oct. 20, 2016

On 10/18/16 5:40 PM, Sebastian Lackner wrote: > On 18.10.2016 17:07, Aric Stewart wrote: >> Signed-off-by: Aric Stewart <aric(a)codeweavers.com> >> --- >> dlls/hidclass.sys/device.c | 20 +++++++------------- >> dlls/hidclass.sys/main.c | 13 +++++-------- >> dlls/hidclass.sys/pnp.c | 10 +++++----- >> 3 files changed, 17 insertions(+), 26 deletions(-) > > Thanks! While you are just at it, could you please also check the two > occurances of "if (irp->IoStatus.u.Status == STATUS_PENDING)"? They > look wrong to me, the correct way would be to check the return value > of IoCallDriver. Also, it looks like call_minidriver is leaking "buffer". Actually that was intentional at the time. There was code in ntoskrnl that was freeing that buffer already so I was getting a double free issue. Now you have been doing a lot of work there so maybe that has changed. I have not gone back and revisited it. -aric > > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> >

1 0

Re: [PATCH] ws2_32: Allow user to enable IP dual stack (v6).
by Marvin Oct. 20, 2016

Oct. 20, 2016

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=26243 Your paranoid android. === w2008s64 (64 bit sock) === sock: unhandled exception c0000005 at 000007FEFF1220C0

1 0

Re: [PATCH] d2d1: Initial implementation of bitmap render target
by Henri Verbeet Oct. 19, 2016

Oct. 19, 2016

On 14 October 2016 at 18:12, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > + if (!pixel_size && size) > + { > + texture_desc.Width = ceilf((size->width * parent_target->desc.dpiX) / 96.0f); > + texture_desc.Height = ceilf((size->height * parent_target->desc.dpiY) / 96.0f); > + dxgi_rt_desc.dpiX = (texture_desc.Width * 96.0f) / size->width; > + dxgi_rt_desc.dpiY = (texture_desc.Height * 96.0f) / size->height; > + } > + else if (pixel_size && !size) > + { > + texture_desc.Width = pixel_size->width; > + texture_desc.Height = pixel_size->height; > + dxgi_rt_desc.dpiX = parent_target->desc.dpiX; > + dxgi_rt_desc.dpiY = parent_target->desc.dpiY; > + } > + else if (pixel_size && size) > + { > + texture_desc.Width = pixel_size->width; > + texture_desc.Height = pixel_size->height; > + dxgi_rt_desc.dpiX = (pixel_size->width * 96.0f) / size->width; > + dxgi_rt_desc.dpiY = (pixel_size->height * 96.0f) / size->height; > + } > + else > + { > + texture_desc.Width = parent_target->pixel_size.width; > + texture_desc.Height = parent_target->pixel_size.height; > + dxgi_rt_desc.dpiX = parent_target->desc.dpiX; > + dxgi_rt_desc.dpiY = parent_target->desc.dpiY; > + } It may just be taste, but I think this is a bit harder to follow than it needs to be. What do you think about the following? if (pixel_size) { texture_desc.Width = pixel_size->width; texture_desc.Height = pixel_size->height; } else if (size) { texture_desc.Width = ceilf((size->width * parent_target->desc.dpiX) / 96.0f); texture_desc.Height = ceilf((size->height * parent_target->desc.dpiY) / 96.0f); } else { texture_desc.Width = parent_target->pixel_size.width; texture_desc.Height = parent_target->pixel_size.height; } if (size) { dxgi_rt_desc.dpiX = (texture_desc.Width * 96.0f) / size->width; dxgi_rt_desc.dpiY = (texture_desc.Height * 96.0f) / size->height; } else { dxgi_rt_desc.dpiX = parent_target->desc.dpiX; dxgi_rt_desc.dpiY = parent_target->desc.dpiY; }

2 1

Re: [PATCH 6/6] wined3d: Rename buffer_invalidate_bo_range() to buffer_mark_dirty_area().
by Henri Verbeet Oct. 19, 2016

Oct. 19, 2016

On 16 October 2016 at 21:46, Józef Kucia <jkucia(a)codeweavers.com> wrote: > -static void buffer_invalidate_bo_range(struct wined3d_buffer *buffer, unsigned int offset, unsigned int size) > +static void buffer_mark_dirty_area(struct wined3d_buffer *buffer, unsigned int offset, unsigned int size) I assume this is for consistency with buffer_clear_dirty_areas() and the like, but in the larger context we're using validate/invalidate for terminology.

2 2

Re: [PATCH 3/6] wined3d: Rename buffer_internal_preload() to wined3d_buffer_load().
by Henri Verbeet Oct. 19, 2016

Oct. 19, 2016

On 16 October 2016 at 21:46, Józef Kucia <jkucia(a)codeweavers.com> wrote: > -void buffer_internal_preload(struct wined3d_buffer *buffer, struct wined3d_context *context, > +void wined3d_buffer_load(struct wined3d_buffer *buffer, struct wined3d_context *context, > const struct wined3d_state *state) This makes the naming consistent with wined3d_texture_load(), so that's good. Still, I don't think that one necessarily covers what the function does very well either, so if anyone has good suggestions...

1 0

Re: [PATCH 2/6] wined3d: Clean up buffer_internal_preload().
by Henri Verbeet Oct. 19, 2016

Oct. 19, 2016

On 16 October 2016 at 21:46, Józef Kucia <jkucia(a)codeweavers.com> wrote: > -static inline void fixup_transformed_pos(float *p) > +static inline unsigned int fixup_transformed_pos(float *p) > { > /* rhw conversion like in position_float4(). */ > if (p[3] != 1.0f && p[3] != 0.0f) > @@ -452,6 +454,8 @@ static inline void fixup_transformed_pos(float *p) > p[2] *= w; > p[3] = w; > } > + > + return 4 * sizeof(*p); > } Since you're cleaning things up, note that this should probably use struct wined3d_vec4. > - for (i = start / buffer->stride; i < min((end / buffer->stride) + 1, vertices); ++i) > + for (i = start / buffer->stride; i < min((end / buffer->stride) + 1, vertex_count); ++i) > { > - for (j = 0; j < buffer->stride; ++j) > + for (j = 0; j < buffer->stride;) > { > switch (buffer->conversion_map[j]) > { In general this conversion code shouldn't be used a lot, but I could imagine it being better to loop over the vertices in the fixup functions instead of in the outer loop.

1 0

Re: [PATCH 1/2] taskschd: Added ITriggerCollection::Create implementation.
by Dmitry Timoshkov Oct. 18, 2016

Oct. 18, 2016

Jacek Caban <jacek(a)codeweavers.com> wrote: > + if(IsEqualGUID(&IID_IUnknown, riid) || IsEqualGUID(&IID_IDispatch, riid) || IsEqualGUID(&IID_ITrigger, riid) > + || IsEqualGUID(&IID_IDailyTrigger, riid)) { > + *ppv = &This->IDailyTrigger_iface; > + }else { Please try to follow the style of the existing code for placing braces, add space between if and (, } and else, and split the above line of comparisons. > @@ -144,8 +373,29 @@ static HRESULT WINAPI TriggerCollection_get__NewEnum(ITriggerCollection *iface, > static HRESULT WINAPI TriggerCollection_Create(ITriggerCollection *iface, TASK_TRIGGER_TYPE2 type, ITrigger **trigger) > { > trigger_collection *This = impl_from_ITriggerCollection(iface); > - FIXME("(%p)->(%d %p)\n", This, type, trigger); > - return E_NOTIMPL; > + > + TRACE("(%p)->(%d %p)\n", This, type, trigger); > + > + switch(type) { > + case TASK_TRIGGER_DAILY: { > + DailyTrigger *daily_trigger; > + > + daily_trigger = heap_alloc(sizeof(*daily_trigger)); > + if(!daily_trigger) > + return E_OUTOFMEMORY; > + > + daily_trigger->IDailyTrigger_iface.lpVtbl = &DailyTrigger_vtbl; > + daily_trigger->ref = 1; > + > + *trigger = (ITrigger*)&daily_trigger->IDailyTrigger_iface; > + break; > + } > + default: > + FIXME("Unimplemented type %d\n", type); > + return E_NOTIMPL; > + } Please add a global constructor for the triggger, and use it here. -- Dmitry.

1 0

Re: [PATCH v2] kernel32: Improved GetUserPreferredUILanguages stub.
by Marvin Oct. 18, 2016

Oct. 18, 2016

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=26213 Your paranoid android. === w1064 (32 bit locale) === locale.c:2650: Test failed: LCMapStringEx should fail with bad locale name locale.c:2651: Test failed: unexpected error code -559038737 locale.c:2742: Test failed: Expected lcid == 0, got 00001000, error -559038737 locale.c:4478: Test failed: IsValidLocaleName should have failed locale.c:4480: Test failed: IsValidLocaleName should have failed locale.c:4863: Test failed: For id 7, expected ret == 4, got 1, error 87 locale.c:4865: Test failed: For id 7, Expected IVC, got '' === w1064 (64 bit locale) === locale.c:2650: Test failed: LCMapStringEx should fail with bad locale name locale.c:2651: Test failed: unexpected error code -559038737 locale.c:2742: Test failed: Expected lcid == 0, got 00001000, error -559038737 locale.c:4478: Test failed: IsValidLocaleName should have failed locale.c:4480: Test failed: IsValidLocaleName should have failed locale.c:4863: Test failed: For id 7, expected ret == 4, got 1, error 87 locale.c:4865: Test failed: For id 7, Expected IVC, got ''

1 0

Re: kernel32: Improved GetUserPreferredUILanguages stub.
by Marvin Oct. 18, 2016

Oct. 18, 2016

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

1 0

Re: [PATCH v2 2/2] hid/tests: Add HID device enumeration test
by Sebastian Lackner Oct. 18, 2016

Oct. 18, 2016

On 17.10.2016 15:58, Aric Stewart wrote: > v2: Suggestions from Sebastian Lackner > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > configure | 1 + > configure.ac | 1 + > dlls/hid/tests/Makefile.in | 5 +++ > dlls/hid/tests/device.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 89 insertions(+) > create mode 100644 dlls/hid/tests/Makefile.in > create mode 100644 dlls/hid/tests/device.c > > > > v2-0002-hid-tests-Add-HID-device-enumeration-test.txt > > > diff --git a/configure b/configure > index c21bda3..b397b38 100755 > --- a/configure > +++ b/configure > @@ -17940,6 +17940,7 @@ wine_fn_config_dll gpkcsp enable_gpkcsp > wine_fn_config_dll hal enable_hal > wine_fn_config_dll hhctrl.ocx enable_hhctrl_ocx clean,implib htmlhelp > wine_fn_config_dll hid enable_hid implib > +wine_fn_config_test dlls/hid/tests hid_test > wine_fn_config_dll hidclass.sys enable_hidclass_sys implib hidclass > wine_fn_config_dll hlink enable_hlink clean,implib > wine_fn_config_test dlls/hlink/tests hlink_test > diff --git a/configure.ac b/configure.ac > index c624c9e..40060b7 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -3027,6 +3027,7 @@ WINE_CONFIG_DLL(gpkcsp) > WINE_CONFIG_DLL(hal) > WINE_CONFIG_DLL(hhctrl.ocx,,[clean,implib],[htmlhelp]) > WINE_CONFIG_DLL(hid,,[implib]) > +WINE_CONFIG_TEST(dlls/hid/tests) > WINE_CONFIG_DLL(hidclass.sys,,[implib],[hidclass]) > WINE_CONFIG_DLL(hlink,,[clean,implib]) > WINE_CONFIG_TEST(dlls/hlink/tests) > diff --git a/dlls/hid/tests/Makefile.in b/dlls/hid/tests/Makefile.in > new file mode 100644 > index 0000000..6bf3773 > --- /dev/null > +++ b/dlls/hid/tests/Makefile.in > @@ -0,0 +1,5 @@ > +TESTDLL = hid.dll > +IMPORTS = hid setupapi > + > +C_SRCS = \ > + device.c > diff --git a/dlls/hid/tests/device.c b/dlls/hid/tests/device.c > new file mode 100644 > index 0000000..47ad1a3 > --- /dev/null > +++ b/dlls/hid/tests/device.c > @@ -0,0 +1,82 @@ > +/* > + * Copyright (c) 2016 Aric Stewart > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA > + */ > + > +#include "ntstatus.h" > +#define WIN32_NO_STATUS > +#include "windows.h" > +#include "setupapi.h" > +#include "ddk/hidsdi.h" > + > +#include "wine/test.h" > + > +static void enumerate_devices(void) I didn't point this out earlier, but usually test functions start with "test_". An exception would be if you plan to use this as a helper function to run other tests for all enumerated devices. Then you could either move most of the code to START_TEST, for example: --- snip --- START_TEST(device) { ... enumeration code ... test_device_info(file); ... enumeration code ... } --- snip --- or rename it to something like "run_for_each_device(callback)" and use a callback-based approach: --- snip --- START_TEST(device) { run_for_each_device(test_device_info); } --- snip --- > +{ > + GUID hid_guid; > + WCHAR device_name[128]; > + HDEVINFO info_set; > + DWORD index = 0; > + SP_DEVICE_INTERFACE_DATA interface_data; > + DWORD detail_size = MAX_PATH * sizeof(WCHAR); > + SP_DEVICE_INTERFACE_DETAIL_DATA_W *data; > + > + HidD_GetHidGuid(&hid_guid); > + > + ZeroMemory(&interface_data, sizeof(interface_data)); > + interface_data.cbSize = sizeof(interface_data); > + > + data = HeapAlloc(GetProcessHeap(), 0, sizeof(*data) + detail_size); > + data->cbSize = sizeof(*data); > + > + info_set = SetupDiGetClassDevsW(&hid_guid, NULL, NULL, DIGCF_DEVICEINTERFACE); > + while (SetupDiEnumDeviceInterfaces(info_set, NULL, &hid_guid, index, &interface_data)) > + { > + index ++; > + > + if (SetupDiGetDeviceInterfaceDetailW(info_set, &interface_data, data, sizeof(*data) + detail_size, NULL, NULL)) > + { > + NTSTATUS status; > + BOOL rc; > + PHIDP_PREPARSED_DATA ppd; > + HIDP_CAPS Caps; > + HANDLE file = CreateFileW(data->DevicePath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, 0); > + if (file == INVALID_HANDLE_VALUE) > + { > + trace("Failed to access device %s, likely not plugged in or access is denied.\n", wine_dbgstr_w(data->DevicePath)); > + continue; > + } > + rc = HidD_GetPreparsedData(file, &ppd); > + ok(rc, "Failed to get preparsed data(0x%x)\n", GetLastError()); > + status = HidP_GetCaps(ppd, &Caps); > + ok(status == HIDP_STATUS_SUCCESS, "Failed to get Caps(0x%x)\n", status); > + rc = HidD_GetProductString(file, device_name, sizeof(device_name)); > + ok(rc, "Failed to get product string(0x%x)\n", GetLastError()); > + trace("Found device %s (%s, %02x, %02x)\n", wine_dbgstr_w(device_name), wine_dbgstr_w(data->DevicePath), Caps.UsagePage, Caps.Usage); > + rc = HidD_FreePreparsedData(ppd); > + ok(rc, "Failed to free preparsed data(0x%x)\n", GetLastError()); > + CloseHandle(file); > + } > + } > + HeapFree(GetProcessHeap(), 0, data); > + if (info_set) > + SetupDiDestroyDeviceInfoList(info_set); According to MSDN SetupDiGetClassDevs returns INVALID_HANDLE_VALUE on failure, not NULL. If this check is not necessary you can also remove it. > +} > + > +START_TEST(device) > +{ > + enumerate_devices(); > +} > > >

1 0

Re: [PATCH] ws2_32: Allow user to enable IP dual stack (v5).
by Dmitry Timoshkov Oct. 18, 2016

Oct. 18, 2016

Roman Pišl <rpisl(a)seznam.cz> wrote: > IP dual stack (v4+v6) should be disabled by default, but previous code > was setting IPV6_V6ONLY in bind() which prevented user to override it. > This patch moves setting IPV6_V6ONLY to socket creation time. > > V5 - fixes failing test on Windows > > Based on https://www.winehq.org/pipermail/wine-patches/2016-July/151919.html > > Signed-off-by: Matthieu Nottale <matthieu.nottale(a)infinit.io> > Signed-off-by: Roman Pišl <rpisl(a)seznam.cz> Probably you shouldn't add someone's else sign-off without the author's agreement, or mention that you've got an explicit agreement from the patch author otherwise. > +#ifdef IPV6_V6ONLY > + if (unixaf == AF_INET6) > + { > + int enable = 1; > + int fd = get_sock_fd(ret, 0, NULL); > + setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &enable, sizeof(enable)); > + } > +#endif It's better to verify that get_sock_fd() succeded before using its result, and call release_sock_fd() at the end. -- Dmitry.

2 2

Re: [PATCH] ws2_32: Allow user to enable IP dual stack (v4).
by Bruno Jesus Oct. 17, 2016

Oct. 17, 2016

Hi, thanks for the retry. I'm not at home and I'm replying from mobile so I can't help much, from the test results it is clear that MS broke the API starting on Vista, the.tests need to be.enhaced so that they pass on both pré vista and post xp. In the sockets tests I have made that many times due to other API breaks, look for the word behavior around tests line 1760. So you can split and make tests pass on all systems. On Monday, October 17, 2016, Roman Pišl <rpisl(a)seznam.cz> wrote: > IP dual stack (v4+v6) should be disabled by default, but previous code > was setting IPV6_V6ONLY in bind() which prevented user to override it. > This patch moves setting IPV6_V6ONLY to socket creation time. > > Based on https://www.winehq.org/pipermail/wine-patches/2016- > July/151919.html > > Signed-off-by: Matthieu Nottale <matthieu.nottale(a)infinit.io > <javascript:;>> > Signed-off-by: Roman Pišl <rpisl(a)seznam.cz <javascript:;>> > --- > dlls/ws2_32/socket.c | 23 +++++++++------------- > dlls/ws2_32/tests/sock.c | 50 ++++++++++++++++++++++++++++++ > ++++++++++++++++-- > 2 files changed, 57 insertions(+), 16 deletions(-) > > diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c > index e8c5130..d14e9d6 100644 > --- a/dlls/ws2_32/socket.c > +++ b/dlls/ws2_32/socket.c > @@ -3240,20 +3240,6 @@ int WINAPI WS_bind(SOCKET s, const struct > WS_sockaddr* name, int namelen) > } > else > { > -#ifdef IPV6_V6ONLY > - const struct sockaddr_in6 *in6 = (const struct > sockaddr_in6*) &uaddr; > - if (name->sa_family == WS_AF_INET6 && > - !memcmp(&in6->sin6_addr, &in6addr_any, sizeof(struct > in6_addr))) > - { > - int enable = 1; > - if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, > &enable, sizeof(enable)) == -1) > - { > - release_sock_fd( s, fd ); > - SetLastError(WSAEAFNOSUPPORT); > - return SOCKET_ERROR; > - } > - } > -#endif > if (name->sa_family == WS_AF_INET) > { > struct sockaddr_in *in4 = (struct sockaddr_in*) > &uaddr; > @@ -7233,6 +7219,15 @@ SOCKET WINAPI WSASocketW(int af, int type, int > protocol, > TRACE("\tcreated %04lx\n", ret ); > if (ipxptype > 0) > set_ipx_packettype(ret, ipxptype); > + > +#ifdef IPV6_V6ONLY > + if (unixaf == AF_INET6) > + { > + int enable = 1; > + int fd = get_sock_fd(ret, 0, NULL); > + setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &enable, > sizeof(enable)); > + } > +#endif > return ret; > } > > diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c > index a144bd3..a676255 100644 > --- a/dlls/ws2_32/tests/sock.c > +++ b/dlls/ws2_32/tests/sock.c > @@ -1732,7 +1732,7 @@ static void test_so_reuseaddr(void) > DWORD err; > > saddr.sin_family = AF_INET; > - saddr.sin_port = htons(9375); > + saddr.sin_port = htons(SERVERPORT+1); > saddr.sin_addr.s_addr = inet_addr("127.0.0.1"); > > s1=socket(AF_INET, SOCK_STREAM, 0); > @@ -6110,6 +6110,8 @@ static void test_ipv6only(void) > struct sockaddr_in sin4; > struct sockaddr_in6 sin6; > int ret; > + int enabled; > + int len; > > memset(&sin4, 0, sizeof(sin4)); > sin4.sin_family = AF_INET; > @@ -6142,6 +6144,50 @@ static void test_ipv6only(void) > ok(!ret, "Could not bind IPv4 address (LastError: %d; %d expected if > IPv6 binds to IPv4 as well).\n", > WSAGetLastError(), WSAEADDRINUSE); > > + closesocket(v4); > + closesocket(v6); > + v4 = INVALID_SOCKET; > + v6 = INVALID_SOCKET; > + > + /* Test again, this time disabling V6ONLY. */ > + sin4.sin_port = htons(SERVERPORT+2); > + sin6.sin6_port = htons(SERVERPORT+2); > + > + v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_TCP); > + ok(v6 != INVALID_SOCKET, "Could not create IPv6 socket (LastError: > %d; %d expected if IPv6 not available).\n", > + WSAGetLastError(), WSAEAFNOSUPPORT); > + > + enabled = 2; > + len = sizeof(enabled); > + ret = getsockopt(v6, IPPROTO_IPV6, IPV6_V6ONLY, (char*)&enabled, > &len); > + if (ret) { > + skip("Could not check IPV6_V6ONLY (option not supported?) > ((LastError: %d).\n", WSAGetLastError()); > + goto end; > + } > + ok(enabled == 1, "IPV6_V6ONLY is not enabled by default.\n"); > + > + enabled = 0; > + ret = setsockopt(v6, IPPROTO_IPV6, IPV6_V6ONLY, (char*)&enabled, len); > + ok(!ret, "Failed to disable IPV6_V6ONLY (LastError: %d).\n", > WSAGetLastError()); > + > + ret = bind(v6, (struct sockaddr*)&sin6, sizeof(sin6)); > + ok(!ret, "Could not bind IPv6 address (LastError: %d).\n", > WSAGetLastError()); > + > + enabled = 2; > + len = sizeof(enabled); > + getsockopt(v6, IPPROTO_IPV6, IPV6_V6ONLY, (char*)&enabled, &len); > + ok(enabled == 0, "IPV6_V6ONLY is enabled after bind.\n"); > + > + v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); > + ok(v4 != INVALID_SOCKET, "Could not create IPv4 socket (LastError: > %d).\n", > + WSAGetLastError()); > + > + WSASetLastError(0xdeadbeef); > + bind(v4, (struct sockaddr*)&sin4, sizeof(sin4)); > + > + ok(WSAGetLastError() == WSAEADDRINUSE, "Failed to disable IPV6_V6ONLY > (LastError: %d; %d expected if IPv6 binds to IPv4 as well).\n", > + WSAGetLastError(), WSAEADDRINUSE); > + > end: > if (v4 != INVALID_SOCKET) > closesocket(v4); > @@ -8024,7 +8070,7 @@ static void test_TransmitFile(void) > /* Setup a properly connected socket for transfers */ > memset(&bindAddress, 0, sizeof(bindAddress)); > bindAddress.sin_family = AF_INET; > - bindAddress.sin_port = htons(9375); > + bindAddress.sin_port = htons(SERVERPORT+1); > bindAddress.sin_addr.s_addr = inet_addr("127.0.0.1"); > iret = bind(server, (struct sockaddr*)&bindAddress, > sizeof(bindAddress)); > if (iret != 0) > -- > 2.7.3 > > > >

1 0

Re: [PATCH] ws2_32: Allow user to enable IP dual stack (v4).
by Marvin Oct. 17, 2016

Oct. 17, 2016

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=26191 Your paranoid android. === wvistau64 (32 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: -559038737; 10048 expected if IPv6 binds to IPv4 as well). === w2008s64 (32 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: -559038737; 10048 expected if IPv6 binds to IPv4 as well). === w7u (32 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === w7pro64 (32 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). sock.c:7810: Test failed: bytesReturned isn't supposed to be 1 sock.c:7834: Test failed: GetOverlappedResult failed, error 0 === w8 (32 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === w864 (32 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === w1064 (32 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === wvistau64 (64 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: -559038737; 10048 expected if IPv6 binds to IPv4 as well). === w2008s64 (64 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: -559038737; 10048 expected if IPv6 binds to IPv4 as well). === w7pro64 (64 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). sock.c:7810: Test failed: bytesReturned isn't supposed to be 1 sock.c:7856: Test failed: Waiting for timeout failed with 0 + errno 997 === w864 (64 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well). === w1064 (64 bit sock) === sock.c:6188: Test failed: Failed to disable IPV6_V6ONLY (LastError: 0; 10048 expected if IPv6 binds to IPv4 as well).

1 0

Re: [PATCH v2] winebus.sys: Handle device reports for hidraw devices
by Sebastian Lackner Oct. 17, 2016

Oct. 17, 2016

On 17.10.2016 15:29, Aric Stewart wrote: > v2: Suggestions from Sebastian Lackner > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus.h | 3 ++ > dlls/winebus.sys/bus_udev.c | 73 ++++++++++++++++++++++++++ > dlls/winebus.sys/main.c | 123 +++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 198 insertions(+), 1 deletion(-) > > > > v2-0001-winebus.sys-Handle-device-reports-for-hidraw-device.txt > > > diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h > index 7ce54bf..bcb1163 100644 > --- a/dlls/winebus.sys/bus.h > +++ b/dlls/winebus.sys/bus.h > @@ -25,6 +25,8 @@ typedef struct > int (*compare_platform_device)(DEVICE_OBJECT *device, void *platform_dev); > NTSTATUS (*get_reportdescriptor)(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length); > NTSTATUS (*get_string)(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length); > + NTSTATUS (*begin_report_processing)(DEVICE_OBJECT *device, BYTE **report_buffer, DWORD *buffer_length); > + void (*end_report_processing)(DEVICE_OBJECT *device, BYTE *report_buffer); > } platform_vtbl; > > void *get_platform_private(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > @@ -37,3 +39,4 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) DECLSPEC_HIDDEN; > void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; > +void process_hid_report(DEVICE_OBJECT *device, const WCHAR *busidW, BYTE *report, DWORD length) DECLSPEC_HIDDEN; > diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c > index 243aabb..37c8412 100644 > --- a/dlls/winebus.sys/bus_udev.c > +++ b/dlls/winebus.sys/bus_udev.c > @@ -61,6 +61,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > > #ifdef HAVE_UDEV > > +WINE_DECLARE_DEBUG_CHANNEL(hid_report); > + > static struct udev *udev_context = NULL; > static DRIVER_OBJECT *udev_driver_obj = NULL; > > @@ -73,6 +75,11 @@ struct platform_private > { > struct udev_device *udev_device; > int device_fd; > + > + BYTE *report_buffer; > + int buffer_length; > + HANDLE report_thread; > + int control_pipe[2]; > }; > > static inline struct platform_private *impl_from_DEVICE_OBJECT(DEVICE_OBJECT *device) > @@ -222,11 +229,77 @@ static NTSTATUS hidraw_get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buf > return STATUS_SUCCESS; > } > > +static DWORD CALLBACK device_report_thread(void *args) > +{ > + DEVICE_OBJECT *device = (DEVICE_OBJECT*)args; > + struct platform_private *private = impl_from_DEVICE_OBJECT(device); > + struct pollfd plfds[2]; > + > + plfds[0].fd = private->device_fd; > + plfds[0].events = POLLIN; > + plfds[0].revents = 0; > + plfds[1].fd = private->control_pipe[0]; > + plfds[1].events = POLLIN; > + plfds[1].revents = 0; > + > + while (1) > + { > + int size; > + if (poll(plfds, 2, -1) <= 0) continue; > + if (plfds[1].revents || !private->report_buffer || private->buffer_length == 0) > + break; > + size = read(plfds[0].fd, private->report_buffer, private->buffer_length); This could still cause problems when another thread copies the last_report while it is updated. I don't know how you are planning to implement it for other platforms, but wouldn't it be better to use a separate buffer in this thread, and let process_hid_report() do the copying? > + if (size == -1) > + TRACE_(hid_report)("Read failed. Likely an unplugged device\n"); > + else if (size == 0) > + TRACE_(hid_report)("Failed to read report\n"); > + else > + process_hid_report(device, hidraw_busidW, private->report_buffer, size); > + } > + return 0; > +} > + > +static NTSTATUS begin_report_processing(DEVICE_OBJECT *device, BYTE **buffer, DWORD *length) > +{ > + struct platform_private *private = impl_from_DEVICE_OBJECT(device); > + *length = private->buffer_length = 1024; > + *buffer = private->report_buffer = HeapAlloc(GetProcessHeap(), 0, *length); > + > + pipe(private->control_pipe); > + private->report_thread = CreateThread(NULL, 0, device_report_thread, device, 0, NULL); > + if (!private->report_thread) > + { > + ERR("Unable to create device report thread\n"); > + close(private->control_pipe[0]); > + close(private->control_pipe[1]); > + return STATUS_UNSUCCESSFUL; > + } > + else > + return STATUS_SUCCESS; > +} > + > +static void end_report_processing(DEVICE_OBJECT *device, BYTE *buffer) > +{ > + struct platform_private *private = impl_from_DEVICE_OBJECT(device); > + if (private->report_thread) > + { > + write(private->control_pipe[1], "q", 1); > + WaitForSingleObject(private->report_thread, INFINITE); > + close(private->control_pipe[0]); > + close(private->control_pipe[1]); > + } > + private->buffer_length = 0; > + private->report_buffer = NULL; > + HeapFree(GetProcessHeap(), 0, buffer); > +} > + > static const platform_vtbl hidraw_vtbl = > { > compare_platform_device, > hidraw_get_reportdescriptor, > hidraw_get_string, > + begin_report_processing, > + end_report_processing, > }; > > static void try_add_device(struct udev_device *dev) > diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c > index 86adbf5..49dcc97 100644 > --- a/dlls/winebus.sys/main.c > +++ b/dlls/winebus.sys/main.c > @@ -40,6 +40,7 @@ > #include "bus.h" > > WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > +WINE_DECLARE_DEBUG_CHANNEL(hid_report); > > struct pnp_device > { > @@ -58,6 +59,14 @@ struct device_extension > const WCHAR *busid; /* Expected to be a static constant */ > > const platform_vtbl *vtbl; > + > + BOOL processing_started; > + BYTE *last_report; > + DWORD last_report_size; > + DWORD buffer_size; > + LIST_ENTRY irp_queue; > + CRITICAL_SECTION report_cs; > + > BYTE platform_private[1]; > }; > > @@ -214,6 +223,9 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > ext->busid = busidW; > ext->vtbl = vtbl; > > + InitializeListHead(&ext->irp_queue); > + InitializeCriticalSection(&ext->report_cs); You might want to give this CS a name, like done in other parts of wine. For example: --- snip --- InitializeCriticalSection(&src->lock); src->lock.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": XA2SourceImpl.lock"); --- snip --- > + > /* add to list of pnp devices */ > pnp_dev->device = device; > list_add_tail(&pnp_devset, &pnp_dev->entry); > @@ -271,6 +283,8 @@ void bus_remove_hid_device(DEVICE_OBJECT *device) > { > struct device_extension *ext = (struct device_extension *)device->DeviceExtension; > struct pnp_device *pnp_device = ext->pnp_device; > + LIST_ENTRY *entry; > + IRP *irp; > > TRACE("(%p)\n", device); > > @@ -279,6 +293,23 @@ void bus_remove_hid_device(DEVICE_OBJECT *device) > LeaveCriticalSection(&device_list_cs); > > IoInvalidateDeviceRelations(device, RemovalRelations); > + > + if (ext->processing_started) > + ext->vtbl->end_report_processing(device, ext->last_report); > + > + /* Cancel pending IRPs */ > + EnterCriticalSection(&ext->report_cs); > + entry = RemoveHeadList(&ext->irp_queue); > + while(entry != &ext->irp_queue) > + { > + irp = CONTAINING_RECORD(entry, IRP, Tail.Overlay.ListEntry); > + irp->IoStatus.u.Status = STATUS_CANCELLED; > + irp->IoStatus.Information = 0; > + IoCompleteRequest(irp, IO_NO_INCREMENT); > + entry = RemoveHeadList(&ext->irp_queue); > + } > + LeaveCriticalSection(&ext->report_cs); > + You will have to delete the CS here. Also make sure to reset Spare[0] if it was previously set to a debug string. For example: --- snip --- src->lock.DebugInfo->Spare[0] = 0; DeleteCriticalSection(&src->lock); --- snip --- > HeapFree(GetProcessHeap(), 0, ext->serial); > IoDeleteDevice(device); > > @@ -432,6 +463,58 @@ NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) > irp->IoStatus.Information = (strlenW((WCHAR *)irp->UserBuffer) + 1) * sizeof(WCHAR); > break; > } > + case IOCTL_HID_GET_INPUT_REPORT: > + { > + HID_XFER_PACKET *packet = (HID_XFER_PACKET*)(irp->UserBuffer); > + TRACE_(hid_report)("IOCTL_HID_GET_INPUT_REPORT\n"); > + EnterCriticalSection(&ext->report_cs); > + if (!ext->processing_started) > + { > + irp->IoStatus.u.Status = status = ext->vtbl->begin_report_processing(device, &ext->last_report, &ext->buffer_size); > + if (status == STATUS_SUCCESS) > + ext->processing_started = TRUE; > + else > + { > + LeaveCriticalSection(&ext->report_cs); > + break; > + } > + } > + > + if (packet->reportBufferLen < ext->last_report_size) > + { > + irp->IoStatus.u.Status = status = STATUS_BUFFER_TOO_SMALL; > + LeaveCriticalSection(&ext->report_cs); > + break; > + } > + > + ZeroMemory(packet->reportBuffer, packet->reportBufferLen); Do we need this line? Usually callers do not expect valid data after the length provided by irp->IoStatus.Information. Is it different in this case? > + memcpy(packet->reportBuffer, ext->last_report, ext->last_report_size); > + packet->reportBufferLen = ext->last_report_size; > + irp->IoStatus.Information = ext->last_report_size; > + irp->IoStatus.u.Status = status = STATUS_SUCCESS; > + LeaveCriticalSection(&ext->report_cs); > + break; > + } > + case IOCTL_HID_READ_REPORT: > + { > + TRACE_(hid_report)("IOCTL_HID_READ_REPORT\n"); > + EnterCriticalSection(&ext->report_cs); > + if (!ext->processing_started) > + { > + if (ext->vtbl->begin_report_processing(device, &ext->last_report, &ext->buffer_size) == STATUS_SUCCESS) > + ext->processing_started = TRUE; > + else > + { > + irp->IoStatus.u.Status = status; > + LeaveCriticalSection(&ext->report_cs); > + break; > + } > + } > + InsertTailList(&ext->irp_queue, &irp->Tail.Overlay.ListEntry); > + status = STATUS_PENDING; > + LeaveCriticalSection(&ext->report_cs); > + break; > + } > default: > { > ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode; > @@ -441,11 +524,49 @@ NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) > } > } > > - IoCompleteRequest(irp, IO_NO_INCREMENT); > + if (status != STATUS_PENDING) > + IoCompleteRequest(irp, IO_NO_INCREMENT); > > return status; > } > > +void process_hid_report(DEVICE_OBJECT *device, const WCHAR *busidW, BYTE *report, DWORD length) > +{ > + struct device_extension *ext = (struct device_extension*)device->DeviceExtension; > + IRP *irp; > + LIST_ENTRY *entry; > + > + EnterCriticalSection(&ext->report_cs); > + if (ext->last_report && report != ext->last_report && ext->buffer_size <= length) Shouldn't it be "length <= ext->buffer_size" ? > + { > + memcpy(ext->last_report, report, length); > + ext->last_report_size = length; > + } > + entry = RemoveHeadList(&ext->irp_queue); > + while(entry != &ext->irp_queue) > + { > + IO_STACK_LOCATION *irpsp; > + TRACE_(hid_report)("Processing Request\n"); > + irp = CONTAINING_RECORD(entry, IRP, Tail.Overlay.ListEntry); > + irpsp = IoGetCurrentIrpStackLocation(irp); > + > + if (irpsp->Parameters.DeviceIoControl.OutputBufferLength < length) > + { > + irp->IoStatus.Information = 0; > + irp->IoStatus.u.Status = STATUS_BUFFER_TOO_SMALL; > + } > + else > + { > + memcpy(irp->UserBuffer, report, length); > + irp->IoStatus.Information = length; > + irp->IoStatus.u.Status = STATUS_SUCCESS; > + } > + IoCompleteRequest(irp, IO_NO_INCREMENT); > + entry = RemoveHeadList(&ext->irp_queue); > + } > + LeaveCriticalSection(&ext->report_cs); > +} > + > NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) > { > static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0}; > > >

1 0

Re: [PATCH] hid/tests: Add HID device enumeration test
by Sebastian Lackner Oct. 17, 2016

Oct. 17, 2016

On 14.10.2016 11:36, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > configure | 1 + > configure.ac | 1 + > dlls/hid/tests/Makefile.in | 5 +++ > dlls/hid/tests/device.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 87 insertions(+) > create mode 100644 dlls/hid/tests/Makefile.in > create mode 100644 dlls/hid/tests/device.c > > > > 0001-hid-tests-Add-HID-device-enumeration-test.txt > > > diff --git a/configure b/configure > index 99793b0..7bb6fef 100755 > --- a/configure > +++ b/configure > @@ -17936,6 +17936,7 @@ wine_fn_config_dll gpkcsp enable_gpkcsp > wine_fn_config_dll hal enable_hal > wine_fn_config_dll hhctrl.ocx enable_hhctrl_ocx clean,implib htmlhelp > wine_fn_config_dll hid enable_hid implib > +wine_fn_config_test dlls/hid/tests hid_test > wine_fn_config_dll hidclass.sys enable_hidclass_sys implib hidclass > wine_fn_config_dll hlink enable_hlink clean,implib > wine_fn_config_test dlls/hlink/tests hlink_test > diff --git a/configure.ac b/configure.ac > index ba8fd0f..9cab382 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -3025,6 +3025,7 @@ WINE_CONFIG_DLL(gpkcsp) > WINE_CONFIG_DLL(hal) > WINE_CONFIG_DLL(hhctrl.ocx,,[clean,implib],[htmlhelp]) > WINE_CONFIG_DLL(hid,,[implib]) > +WINE_CONFIG_TEST(dlls/hid/tests) > WINE_CONFIG_DLL(hidclass.sys,,[implib],[hidclass]) > WINE_CONFIG_DLL(hlink,,[clean,implib]) > WINE_CONFIG_TEST(dlls/hlink/tests) > diff --git a/dlls/hid/tests/Makefile.in b/dlls/hid/tests/Makefile.in > new file mode 100644 > index 0000000..d4a69f0 > --- /dev/null > +++ b/dlls/hid/tests/Makefile.in > @@ -0,0 +1,5 @@ > +TESTDLL = hid.dll > +IMPORTS = hid setupapi > + > +C_SRCS = \ > + device.c \ The "\" at the end seems wrong. Please note that you can also use tools/make_makefiles to update/fix this file automatically for you. > diff --git a/dlls/hid/tests/device.c b/dlls/hid/tests/device.c > new file mode 100644 > index 0000000..f3260ec > --- /dev/null > +++ b/dlls/hid/tests/device.c > @@ -0,0 +1,80 @@ > +/* > + * Copyright (c) 2016 Aric Stewart > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA > + */ > + > +#include "ntstatus.h" > +#define WIN32_NO_STATUS > +#include "windows.h" > +#include "setupapi.h" > + > +#ifndef WINE_NTSTATUS_DECLARED > +#define WINE_NTSTATUS_DECLARED > +typedef LONG NTSTATUS; > +#endif Such hacks should be avoided in tests. I believe you are trying to workaround a bug in the Wine hidsdi.h header file. Wine declares NTSTATUS after the includes: --- snip --- #include <hidusage.h> #include <ddk/hidpi.h> #ifndef WINE_NTSTATUS_DECLARED #define WINE_NTSTATUS_DECLARED typedef LONG NTSTATUS; #endif --- snip --- On Windows its declared before: --- snip --- typedef _Return_type_success_(return >= 0) LONG NTSTATUS; #include "hidusage.h" #include "hidpi.h" --- snip --- > + > +#include "ddk/hidsdi.h" > + > +#include "wine/test.h" > + > +static void enumerate_devices(void) > +{ > + GUID hid_guid; > + WCHAR device_name[128]; > + HDEVINFO info_set; > + DWORD index = 0; > + SP_DEVICE_INTERFACE_DATA interface_data; > + DWORD detail_size = MAX_PATH * sizeof(WCHAR); > + SP_DEVICE_INTERFACE_DETAIL_DATA_W *data; > + > + HidD_GetHidGuid(&hid_guid); > + > + ZeroMemory(&interface_data, sizeof(interface_data)); > + interface_data.cbSize = sizeof(interface_data); > + > + data = HeapAlloc(GetProcessHeap(), 0, sizeof(*data) + detail_size); > + data->cbSize = sizeof(*data); > + > + info_set = SetupDiGetClassDevsW(&hid_guid, NULL, NULL, DIGCF_DEVICEINTERFACE); Shouldn't this be released afterwards? > + while (SetupDiEnumDeviceInterfaces(info_set, NULL, &hid_guid, index, &interface_data)) > + { > + index ++; > + > + if (SetupDiGetDeviceInterfaceDetailW(info_set, &interface_data, data, detail_size, NULL, NULL)) Shouldn't the DeviceInterfaceDetailDataSize parameter also include the sizeof(*data) ? > + { > + PHIDP_PREPARSED_DATA ppd; > + HIDP_CAPS Caps; > + HANDLE file = CreateFileW(data->DevicePath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, 0); > + if (file == INVALID_HANDLE_VALUE) > + { > + trace("Failed to access device %s, likely not plugged in or access is denied.\n", wine_dbgstr_w(data->DevicePath)); > + continue; > + } > + ok(HidD_GetPreparsedData(file, &ppd), "Failed to preparsed data\n"); Is there a "get" missing in the debug string? > + ok(HidP_GetCaps(ppd, &Caps) == HIDP_STATUS_SUCCESS, "Failed to get Caps\n"); > + ok(HidD_GetProductString(file, device_name, sizeof(device_name)), "Failed to get product string\n"); > + trace("Found device %s (%s, %02x, %02x)\n", wine_dbgstr_w(device_name), wine_dbgstr_w(data->DevicePath), Caps.UsagePage, Caps.Usage); > + ok(HidD_FreePreparsedData(ppd), "Failed to free preparsed data\n"); Its a matter of taste, but usually its better to assign those values to a variable. This also allows to print the error code / GetLastError() in case of a failure. > + CloseHandle(file); > + } > + } > + HeapFree(GetProcessHeap(), 0, data); > +} > + > +START_TEST(device) > +{ > + enumerate_devices(); > +} > > >

1 0

Re: [PATCH] winebus.sys: Handle device reports for hidraw devices
by Sebastian Lackner Oct. 17, 2016

Oct. 17, 2016

On 17.10.2016 06:41, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus.h | 3 ++ > dlls/winebus.sys/bus_udev.c | 63 +++++++++++++++++++++++++++++ > dlls/winebus.sys/main.c | 97 ++++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 162 insertions(+), 1 deletion(-) > > > > 0001-winebus.sys-Handle-device-reports-for-hidraw-devices.txt > > > diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h > index 7ce54bf..bcb1163 100644 > --- a/dlls/winebus.sys/bus.h > +++ b/dlls/winebus.sys/bus.h > @@ -25,6 +25,8 @@ typedef struct > int (*compare_platform_device)(DEVICE_OBJECT *device, void *platform_dev); > NTSTATUS (*get_reportdescriptor)(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length); > NTSTATUS (*get_string)(DEVICE_OBJECT *device, DWORD index, WCHAR *buffer, DWORD length); > + NTSTATUS (*begin_report_processing)(DEVICE_OBJECT *device, BYTE **report_buffer, DWORD *buffer_length); > + void (*end_report_processing)(DEVICE_OBJECT *device, BYTE *report_buffer); > } platform_vtbl; > > void *get_platform_private(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > @@ -37,3 +39,4 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) DECLSPEC_HIDDEN; > void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; > +void process_hid_report(DEVICE_OBJECT *device, const WCHAR *busidW, BYTE *report, DWORD length) DECLSPEC_HIDDEN; > diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c > index 243aabb..5307224 100644 > --- a/dlls/winebus.sys/bus_udev.c > +++ b/dlls/winebus.sys/bus_udev.c > @@ -61,6 +61,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > > #ifdef HAVE_UDEV > > +WINE_DECLARE_DEBUG_CHANNEL(hid_report); > + > static struct udev *udev_context = NULL; > static DRIVER_OBJECT *udev_driver_obj = NULL; > > @@ -73,6 +75,10 @@ struct platform_private > { > struct udev_device *udev_device; > int device_fd; > + > + BYTE *report_buffer; > + int buffer_length; > + HANDLE report_thread; > }; > > static inline struct platform_private *impl_from_DEVICE_OBJECT(DEVICE_OBJECT *device) > @@ -222,11 +228,68 @@ static NTSTATUS hidraw_get_string(DEVICE_OBJECT *device, DWORD index, WCHAR *buf > return STATUS_SUCCESS; > } > > +static DWORD CALLBACK device_report_thread(void *args) > +{ > + DEVICE_OBJECT *device = (DEVICE_OBJECT*)args; > + struct platform_private *private = impl_from_DEVICE_OBJECT(device); > + struct pollfd plfds[1]; > + > + plfds[0].fd = private->device_fd; > + plfds[0].events = POLLIN; > + > + while (1) > + { > + int size; > + if (poll(plfds, 1, -1) <= 0) continue; > + if (!private->report_buffer || private->buffer_length == 0) > + break; > + size = read(plfds[0].fd, private->report_buffer, private->buffer_length); > + if (size == -1) > + TRACE_(hid_report)("Read failed. Likely an unplugged device\n"); > + else if (size == 0) > + TRACE_(hid_report)("Failed to read report\n"); > + else > + process_hid_report(device, hidraw_busidW, private->report_buffer, size); > + } > + return 0; > +} > + > +static NTSTATUS begin_report_processing(DEVICE_OBJECT *device, BYTE **buffer, DWORD *length) > +{ > + struct platform_private *private = impl_from_DEVICE_OBJECT(device); > + *length = private->buffer_length = 1024; > + *buffer = private->report_buffer = HeapAlloc(GetProcessHeap(), 0, *length); > + > + private->report_thread = CreateThread(NULL, 0, device_report_thread, device, 0, NULL); > + if (!private->report_thread) > + { > + ERR("Unable to create device report thread\n"); > + return STATUS_UNSUCCESSFUL; > + } > + else > + return STATUS_SUCCESS; > +} > + > +static void end_report_processing(DEVICE_OBJECT *device, BYTE *buffer) > +{ > + struct platform_private *private = impl_from_DEVICE_OBJECT(device); > + if (private->report_thread) > + { > + TerminateThread(private->report_thread, 0); TerminateThread is not really the best way to clean up this worker thread. Depending on when exactly it is called, it can corrupt internal data and crash the whole process. In this case, it would definitely be better to have some synchronization mechanism (for example event pipe) to allow a clean shutdown. > + WaitForSingleObject(private->report_thread, INFINITE); > + } > + private->buffer_length = 0; > + private->report_buffer = NULL; > + HeapFree(GetProcessHeap(), 0, buffer); > +} > + > static const platform_vtbl hidraw_vtbl = > { > compare_platform_device, > hidraw_get_reportdescriptor, > hidraw_get_string, > + begin_report_processing, > + end_report_processing, > }; > > static void try_add_device(struct udev_device *dev) > diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c > index 86adbf5..f12cdf7 100644 > --- a/dlls/winebus.sys/main.c > +++ b/dlls/winebus.sys/main.c > @@ -40,6 +40,7 @@ > #include "bus.h" > > WINE_DEFAULT_DEBUG_CHANNEL(plugplay); > +WINE_DECLARE_DEBUG_CHANNEL(hid_report); > > struct pnp_device > { > @@ -58,6 +59,13 @@ struct device_extension > const WCHAR *busid; /* Expected to be a static constant */ > > const platform_vtbl *vtbl; > + > + BOOL processing_started; > + BYTE *last_report; > + DWORD last_report_size; > + DWORD buffer_size; > + LIST_ENTRY irp_queue; > + > BYTE platform_private[1]; > }; > > @@ -214,6 +222,8 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > ext->busid = busidW; > ext->vtbl = vtbl; > > + InitializeListHead(&ext->irp_queue); > + > /* add to list of pnp devices */ > pnp_dev->device = device; > list_add_tail(&pnp_devset, &pnp_dev->entry); > @@ -271,6 +281,8 @@ void bus_remove_hid_device(DEVICE_OBJECT *device) > { > struct device_extension *ext = (struct device_extension *)device->DeviceExtension; > struct pnp_device *pnp_device = ext->pnp_device; > + LIST_ENTRY *entry; > + IRP *irp; > > TRACE("(%p)\n", device); > > @@ -279,6 +291,21 @@ void bus_remove_hid_device(DEVICE_OBJECT *device) > LeaveCriticalSection(&device_list_cs); > > IoInvalidateDeviceRelations(device, RemovalRelations); > + > + /* Cancel pending IRPs */ > + entry = RemoveHeadList(&ext->irp_queue); > + while(entry != &ext->irp_queue) > + { > + irp = CONTAINING_RECORD(entry, IRP, Tail.Overlay.ListEntry); > + irp->IoStatus.u.Status = STATUS_CANCELLED; > + irp->IoStatus.Information = 0; > + IoCompleteRequest(irp, IO_NO_INCREMENT); > + entry = RemoveHeadList(&ext->irp_queue); > + } > + > + if (ext->processing_started) > + ext->vtbl->end_report_processing(device, ext->last_report); > + > HeapFree(GetProcessHeap(), 0, ext->serial); > IoDeleteDevice(device); > > @@ -432,6 +459,49 @@ NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) > irp->IoStatus.Information = (strlenW((WCHAR *)irp->UserBuffer) + 1) * sizeof(WCHAR); > break; > } > + case IOCTL_HID_GET_INPUT_REPORT: > + { > + HID_XFER_PACKET *packet = (HID_XFER_PACKET*)(irp->UserBuffer); > + TRACE_(hid_report)("IOCTL_HID_GET_INPUT_REPORT\n"); > + if (!ext->processing_started) > + { > + irp->IoStatus.u.Status = status = ext->vtbl->begin_report_processing(device, &ext->last_report, &ext->buffer_size); > + if (status == STATUS_SUCCESS) > + ext->processing_started = TRUE; > + else > + break; > + } We might need synchronization here to ensure that only one worker thread is started (if there are multiple IOCTL_HID_GET_INPUT_REPORT requests at the same time). > + > + if (packet->reportBufferLen < ext->last_report_size) > + { > + irp->IoStatus.u.Status = status = STATUS_BUFFER_TOO_SMALL; > + break; > + } > + > + ZeroMemory(packet->reportBuffer, packet->reportBufferLen); > + memcpy(packet->reportBuffer, ext->last_report, ext->last_report_size); The worker thread always uses the same buffer. How do you ensure that the "last_report" you are copying here is a complete record, and not corrupted (for example partially old data)? > + packet->reportBufferLen = ext->last_report_size; > + irp->IoStatus.Information = ext->last_report_size; > + irp->IoStatus.u.Status = status = STATUS_SUCCESS; > + break; > + } > + case IOCTL_HID_READ_REPORT: > + { > + TRACE_(hid_report)("IOCTL_HID_READ_REPORT\n"); > + if (!ext->processing_started) > + { > + if (ext->vtbl->begin_report_processing(device, &ext->last_report, &ext->buffer_size) == STATUS_SUCCESS) > + ext->processing_started = TRUE; > + else > + { > + irp->IoStatus.u.Status = status; > + break; > + } > + } > + InsertTailList(&ext->irp_queue, &irp->Tail.Overlay.ListEntry); The irp_queue will require locking when it is accessed from multiple threads. > + status = STATUS_PENDING; Thats fine for now, but I'm planning to send some additional patches soon to fix STATUS_PENDING handling. Drivers on Windows have to call IoMarkIrpPending() when they return STATUS_PENDING. Also see: https://msdn.microsoft.com/en-us/library/windows/hardware/ff549422(v=vs.85)… """Otherwise, the I/O manager attempts to complete the IRP as soon as the dispatch routine returns control.""" and """If a driver queues incoming IRPs, it should call IoMarkIrpPending before it queues each IRP. Otherwise, an IRP could be dequeued, completed by another driver routine, and freed by the system before the call to IoMarkIrpPending occurs, thereby causing a crash. """ > + break; > + } > default: > { > ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode; > @@ -441,11 +511,36 @@ NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) > } > } > > - IoCompleteRequest(irp, IO_NO_INCREMENT); > + if (status != STATUS_PENDING) > + IoCompleteRequest(irp, IO_NO_INCREMENT); > > return status; > } > > +void process_hid_report(DEVICE_OBJECT *device, const WCHAR *busidW, BYTE *report, DWORD length) > +{ > + struct device_extension *ext = (struct device_extension*)device->DeviceExtension; > + IRP *irp; > + LIST_ENTRY *entry; > + > + if (ext->last_report && report != ext->last_report && ext->buffer_size <= length) > + { > + memcpy(ext->last_report, report, length); > + ext->last_report_size = length; > + } > + entry = RemoveHeadList(&ext->irp_queue); > + while(entry != &ext->irp_queue) > + { > + TRACE_(hid_report)("Processing Request\n"); > + irp = CONTAINING_RECORD(entry, IRP, Tail.Overlay.ListEntry); > + memcpy(irp->UserBuffer, report, length); Shouldn't there also be some kind of length validation? > + irp->IoStatus.Information = length; > + irp->IoStatus.u.Status = STATUS_SUCCESS; > + IoCompleteRequest(irp, IO_NO_INCREMENT); > + entry = RemoveHeadList(&ext->irp_queue); > + } > +} > + > NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) > { > static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0}; > > >

1 0

Imort system certificates in crypt32@Wine
by Donat Enikeev Oct. 16, 2016

Oct. 16, 2016

[Resending in plain/text, apologies for html] Hi Guys, Watched video recently from WineConf 2015, and results around opening community looks really promising, so I am performing 3rd attempt to be useful here :) The bug https://bugs.winehq.org/show_bug.cgi?id=30187 : Cisco IP Communicator failing to setup due to 'certmgr.exe' tool fails to install certificate to the system-wide trusted certificate store. Wine uses registry-based certificates stores by default, but treats HKLM\Root store as a special case: whenever app opens such store, Crypt32 goes through hard-coded paths in rootstore.c: static const char * const CRYPT_knownLocations[] = { "/etc/ssl/certs/ca-certificates.crt", "/etc/ssl/certs", ... }; And adds all found certificates to the special store. In the context of bug, this special-case store doesn't support adding certificates, and thus Cisco IP fails to install. So a backward compatible patch (attached) that just fixes this bug looks straightforward: make a stores collection, add there HKLM\Root certificates registry store first and then, add that system store with certificates from the environment. And return the resulting collection as a store to work with. It will allow applications do whatever they used to with certificates, keeping all the linux certificates available for verification of any kind. The problem with this approach is that current wine crypt32 doesn't actually save certificates that were added to a collection of stores (the test for this attached), while Win does. Although it could be fixed with a different patch in one function, but I would like to hear your thoughts first around following questions: 1. Does this backward-compatible patch-set make sense at all and worth proceeding? Probably you have some ongoing activities 2. Do you still think that wine should import system certificates during HKLM\Root request at all, not just shipping with those from typical windows installation? 3. Have you considered different approach of utilizing system certificates in Wine? For example, import all system certificates to the *registry* during wine-prefix initialization process, and work with them from there in a way windows does (even native crypt32 will benefit from this approach). That will allow to unify and simplify crypt32 and remove all that arguable hard-coded paths in the code, and bring more familiar environment to the windows application and, at the same time, isolation. Please share your thoughts, Donnie

1 0

mscoree: Add CorIsLatestSvc function stub.
by Andrey Gusev Oct. 16, 2016

Oct. 16, 2016

>From poking at it with python ctypes, I believe this function takes 2 >pointer arguments and returns an HRESULT. >Have you found a real app that calls this? As far as I know, it's only >called by native .NET components that shouldn't be used with builtin >mscoree. At least two apps calls this function after .NET 4.5.1 (offline) have been installed.

3 2

IP dual stack
by Roman Pišl Oct. 16, 2016

Oct. 16, 2016

Hi, I sent a small patch to fix a bug where it is not possible to disable IPV6_V6ONLY as it is always enabled in WS_bind(). I have discovered, that my patch is wrong and Matthieu Nottale already proposed right solution. But it wasn't merged. What is the current status of the patch? May I help somehow? I would like to fix that bug (it breaks our application). Roman

2 1

Re: mscoree: Add CorIsLatestSvc function stub.
by Vincent Povirk Oct. 15, 2016

Oct. 15, 2016

>From poking at it with python ctypes, I believe this function takes 2 pointer arguments and returns an HRESULT. Have you found a real app that calls this? As far as I know, it's only called by native .NET components that shouldn't be used with builtin mscoree.

1 0

build WoW64 in chroots
by wylda＠volny.cz Oct. 15, 2016

Oct. 15, 2016

Hello, some year ago i built WoW64 per Wine's wiki. But that was re-phrased now or maybe today things are not so complicated in case WoW64 built in chroots32/64. Now it is not clear to me, what are the proper WoW64 steps? : 1. inside chroot64: build wine64 (add extra switch --enable-win64) ****** change chroot ****** 2. inside chroot32: build wine32_tools 3. inside chroot32: build wine32 (uses above points 1. and 2.) 4. inside chroot32: "make install" of wine32 from above point 3 to build some last bits of code (or should be wine32_tools?) ****** change chroot ****** 5. inside chroot64: "make install" of wine32 from above point 4 (but why wine32_tools dir also needs to be available in chroot64??) 6. inside chroot64: "make install" of wine64 from above point 1 (and install dir is the same like in point 5) Or can this be simplified to: 1. inside chroot64: build wine64 (add extra switch --enable-win64) ****** change chroot ****** 2. inside chroot32: build wine32_tools 3. inside chroot32: build wine32 (uses above points 1. and 2.) 4. inside chroot32: "make install" of wine32 from above point 3 to build some last bits of code ****** change chroot ****** 5. inside chroot64: "make install" of wine64 from above point 1 (and install dir is the same like in point 4) Regards, W.

1 0

Re: libwine: Add public symbol to access list of debug options.
by Sebastian Lackner Oct. 14, 2016

Oct. 14, 2016

On 09.10.2016 18:56, Sebastian Lackner wrote: > Signed-off-by: Sebastian Lackner <sebastian(a)fds-team.de> > --- > > Fixes https://bugs.winehq.org/show_bug.cgi?id=41457. > > Currently, the "edit debug channels" feature of taskmgr is broken in > release builds, because: > > * "debug_options" is not exported as a public symbol > * taskmgr passes SYMOPT_PUBLICS_ONLY to SymSetOptions > > This combination does not make any sense (not even installing debug > packages is sufficient). This patch replaces debug_options with > __wine_dbg_options and makes it a public symbol. > > libs/wine/debug.c | 24 ++++++++++++------------ > libs/wine/wine.def | 1 + > libs/wine/wine.map | 1 + > programs/taskmgr/dbgchnl.c | 2 +- > programs/winedbg/info.c | 2 +- > 5 files changed, 16 insertions(+), 14 deletions(-) > Any opinions regarding this patch? An alternative approach would be to fix taskmgr (remove SYMOPT_PUBLICS_ONLY), and ask users to install debug packages if they want to use this feature. Would that be preferred? Regards, Sebastian

2 1

Re: [PATCH 1/5] wined3d: Add a quirk for GL implementations not supporting sRGB FBO attachments.
by Henri Verbeet Oct. 14, 2016

Oct. 14, 2016

On 14 October 2016 at 00:03, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > Signed-off-by: Matteo Bruni <mbruni(a)codeweavers.com> > --- > As it seems to be the case for i915 (at least with some Mesa versions). > > This is only a problem because the GL implementation also exposes > EXT_texture_sRGB_decode and when that extension is available we always > use the sRGB format. > > For bug 41052. > Just disabling EXT_texture_sRGB in this case is probably fine, but it's a little unfortunate that we already retrieve this information in init_format_fbo_compat_info(). Would it be hard to use that information instead?

1 0

Re: [PATCH v2 4/4] winehid: Implement handing internal ioctls
by Sebastian Lackner Oct. 13, 2016

Oct. 13, 2016

On 12.10.2016 18:19, Aric Stewart wrote: > > Mostly by passing them to the bus driver where platform specific actions > happen > > v2: Suggestion from Sebastian Lackner > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winehid.sys/main.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > This patch no longer applies because of the slightly modified version of patch 1/2. I have already prepared a rebased version, but I'm wondering a bit: Couldn't we just pass through ALL ioctls (without any filtering in winehid)? This would reduce the code complexity even further. HID related FIXMEs could also be printed in winebus. hid_internal_dispatch should probably use the "hid" debug channel anyway. What do you think? Regards, Sebastian

2 3

Re: [1/3] gdiplus/tests: Add tests for 0-pixel pens.
by Marvin Oct. 13, 2016

Oct. 13, 2016

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=26156 Your paranoid android. === wxppro (32 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === w2003std (32 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === wvistau64 (32 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === w2008s64 (32 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === w7u (32 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === w7pro64 (32 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === w8 (32 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === w864 (32 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === w1064 (32 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === wvistau64 (64 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === w2008s64 (64 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === w7pro64 (64 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === w864 (64 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0 === w1064 (64 bit graphics) === graphics.c:3982: Test failed: 1: expected 1, got 0 graphics.c:4007: Test failed: 1: expected 1, got 0

1 0

Re: [PATCH 2/2] include: Add a padding WORD to ensure that wWeight is at the correct offset.
by Marvin Oct. 13, 2016

Oct. 13, 2016

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=26151 Your paranoid android. === wxppro (32 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === w2003std (32 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === wvistau64 (32 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === w2008s64 (32 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === w7u (32 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === w7pro64 (32 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === w8 (32 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === w864 (32 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === w1064 (32 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === wvistau64 (64 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === w2008s64 (64 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === w7pro64 (64 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === w864 (64 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0 === w1064 (64 bit editor) === editor.c:1208: Test failed: got 700 editor.c:1217: Test failed: got 0 editor.c:1227: Test failed: got 700 editor.c:1236: Test failed: got 0

2 1

Re: [PATCH 3/4] include: Define IoSkipCurrentIrpStackLocation
by Sebastian Lackner Oct. 13, 2016

Oct. 13, 2016

On 12.10.2016 12:31, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > include/ddk/wdm.h | 2 ++ > 1 file changed, 2 insertions(+) > > > > 0003-include-Define-IoSkipCurrentIrpStackLocation.txt > > > diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h > index 5602f7d..5671948 100644 > --- a/include/ddk/wdm.h > +++ b/include/ddk/wdm.h > @@ -1175,9 +1175,11 @@ NTSTATUS WINAPI ObCloseHandle(IN HANDLE handle); > # ifdef NONAMELESSSTRUCT > # define IoGetCurrentIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.s.u2.CurrentStackLocation) > # define IoGetNextIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.s.u2.CurrentStackLocation - 1) > +# define IoSkipCurrentIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.s.u2.CurrentStackLocation++); ((_Irp)->CurrentLocation++) > # else > # define IoGetCurrentIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.u2.CurrentStackLocation) > # define IoGetNextIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.u2.CurrentStackLocation - 1) > +# define IoSkipCurrentIrpStackLocation(_Irp) ((_Irp)->Tail.Overlay.u2.CurrentStackLocation++); ((_Irp)->CurrentLocation++) > # endif > #else > # ifdef NONAMELESSSTRUCT > Is this definition based on a Windows header file? I have only found two versions, either with a "{ ... }" block or with a static inline function. Your definition would not work correctly in a situation like this: if (...) IoSkipCurrentIrpStackLocation(irp); Which would be expanded to: if (...) irp->Tail.Overlay.s.u2.CurrentStackLocation++; irp->CurrentLocation++; Please note that the second statement is no longer conditional. Also, what about the two other #ifdef branches directly below? Regards, Sebastian

3 5

Re: [PATCH v4 1/2] libwine: Don't call exit in the shared library.
by Sebastian Lackner Oct. 12, 2016

Oct. 12, 2016

On 11.10.2016 16:21, Jens Reyer wrote: > From: Michael Gilbert <mgilbert(a)debian.org> > Signed-off-by: Jens Reyer <jre.winesim(a)gmail.com> > --- > libs/wine/config.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libs/wine/config.c b/libs/wine/config.c > index 3b5dd68..c0b3bdd 100644 > --- a/libs/wine/config.c > +++ b/libs/wine/config.c > @@ -68,7 +68,7 @@ static void fatal_error( const char *err, ... ) > fprintf( stderr, "wine: " ); > vfprintf( stderr, err, args ); > va_end( args ); > - exit(1); > + abort(); > } > > /* die on a fatal error */ > @@ -81,7 +81,7 @@ static void fatal_perror( const char *err, ... ) > vfprintf( stderr, err, args ); > perror( " " ); > va_end( args ); > - exit(1); > + abort(); > } > > /* malloc wrapper */ > I'm not sure if this is really a good idea. In contrast to exit, abort() might trigger the creation of a core dump. Unfortunately fatal_[p]error is also used at many places as a shortcut for "fprintf(stderr,...); exit(1)". This means after this change, a lot of users might see "Aborted (core dumped)" even for harmless error messages. Patch 2 is even more misleading, and shows such a message when the user wants to see the WINEDEBUG help: $ WINEDEBUG=help ./wine winecfg [...] Available message classes: err, warn, fixme, trace Aborted (Core dumped) Before changing things in Winelib, it would first be necessary to modify all involved tools and dlls to avoid fatal_[p]error for harmless errors. Regards, Sebastian

4 4

Compile default Visual C++ 6 project with winegcc
by A B Oct. 12, 2016

Oct. 12, 2016

I'm trying to compile the default sample win32 ,Hello World project . I did winegcc -m32 -o test2 test2.cpp . It runs ok , but resources are not loaded ,there is empty windows title and missing File and Help menus. What is the correct way to make also load the resources?

3 2

Re: [PATCH 4/4] winehid: Implement handing internal ioctls
by Sebastian Lackner Oct. 12, 2016

Oct. 12, 2016

On 12.10.2016 12:31, Aric Stewart wrote: > Mostly by passing them to the bus driver where platform specific actions > happen > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winehid.sys/main.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > > > 0004-winehid-Implement-handing-internal-ioctls.txt > > > diff --git a/dlls/winehid.sys/main.c b/dlls/winehid.sys/main.c > index e8c91bb..7c17321 100644 > --- a/dlls/winehid.sys/main.c > +++ b/dlls/winehid.sys/main.c > @@ -25,6 +25,7 @@ > #include "windef.h" > #include "winbase.h" > #include "winternl.h" > +#include "winioctl.h" > #include "ddk/wdm.h" > #include "ddk/hidport.h" > #include "wine/debug.h" > @@ -33,6 +34,52 @@ WINE_DEFAULT_DEBUG_CHANNEL(hid_minidriver); > > static HID_MINIDRIVER_REGISTRATION registration; > > +static NTSTATUS WINAPI internal_ioctl(DEVICE_OBJECT *device, IRP *irp) > +{ > + IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); > + Some tracing wouldn't hurt here. > + irp->IoStatus.Information = 0; > + > + switch (irpsp->Parameters.DeviceIoControl.IoControlCode) > + { > + case IOCTL_GET_PHYSICAL_DESCRIPTOR: > + TRACE("IOCTL_GET_PHYSICAL_DESCRIPTOR\n"); > + break; > + case IOCTL_HID_ACTIVATE_DEVICE: > + TRACE("IOCTL_HID_ACTIVATE_DEVICE\n"); > + break; > + case IOCTL_HID_DEACTIVATE_DEVICE: > + TRACE("IOCTL_HID_DEACTIVATE_DEVICE\n"); > + break; > + case IOCTL_HID_GET_INDEXED_STRING: > + TRACE("IOCTL_HID_GET_INDEXED_STRING\n"); > + break; I assume there are no plans (yet) to implement those. Shouldn't they be marked as FIXME then? > + case IOCTL_HID_GET_DEVICE_ATTRIBUTES: > + case IOCTL_HID_GET_DEVICE_DESCRIPTOR: > + case IOCTL_HID_GET_REPORT_DESCRIPTOR: > + case IOCTL_HID_GET_STRING: > + case IOCTL_HID_GET_INPUT_REPORT: > + case IOCTL_HID_READ_REPORT: > + case IOCTL_HID_SET_OUTPUT_REPORT: > + case IOCTL_HID_WRITE_REPORT: > + case IOCTL_HID_GET_FEATURE: > + case IOCTL_HID_SET_FEATURE: > + { > + /* All these are handled by the lower level driver */ > + IoSkipCurrentIrpStackLocation(irp); > + return IoCallDriver(((HID_DEVICE_EXTENSION*)device->DeviceExtension)->NextDeviceObject, irp); > + } > + default: > + { > + ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode; > + FIXME("Unsupported ioctl %x (device=%x access=%x func=%x method=%x)\n" > + , code, code >> 16, (code >> 14) & 3, (code >> 2) & 0xfff, code & 3); > + } > + } > + IoCompleteRequest( irp, IO_NO_INCREMENT ); > + return irp->IoStatus.u.Status; After IoCompleteRequest the IRP struct should not be accessed anymore. > +} > + > static NTSTATUS WINAPI add_device(DRIVER_OBJECT *driver, DEVICE_OBJECT *device) > { > TRACE("\n"); > @@ -44,6 +91,7 @@ NTSTATUS WINAPI DriverEntry(DRIVER_OBJECT *driver, UNICODE_STRING *path) > { > TRACE("%s\n", debugstr_w(path->Buffer)); > > + driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = internal_ioctl; > driver->DriverExtension->AddDevice = add_device; > > registration.DriverObject = driver; > > >

1 0

Re: ntdll/tests: Added a test of using NtCancelIoEx on iosb associated with pipe listening.
by Marvin Oct. 12, 2016

Oct. 12, 2016

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=26128 Your paranoid android. === w864 (32 bit pipe) === pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: c0000000, GetLastError: 231 === w1064 (32 bit pipe) === pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: c0000000, GetLastError: 231 === w1064 (64 bit pipe) === pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: c0000000, GetLastError: 231

1 0

Re: [PATCH] winebus.sys: Implement IOCTL_HID_GET_STRING for hidraw
by Sebastian Lackner Oct. 12, 2016

Oct. 12, 2016

On 12.10.2016 12:12, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus.h | 1 + > dlls/winebus.sys/bus_udev.c | 70 +++++++++++++++++++++++++++++++++++++++++++++ > dlls/winebus.sys/main.c | 11 +++++++ > 3 files changed, 82 insertions(+) > > > > 0001-winebus.sys-Implement-IOCTL_HID_GET_STRING-for-hidraw.txt > > > diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h > index 9ab242c..9094e17 100644 > --- a/dlls/winebus.sys/bus.h > +++ b/dlls/winebus.sys/bus.h > @@ -24,6 +24,7 @@ typedef struct > { > int (*compare_platform_device)(DEVICE_OBJECT *device, void *platform_dev); > NTSTATUS (*get_reportdescriptor)(DEVICE_OBJECT *device, BYTE *buffer, DWORD length, DWORD *out_length); > + NTSTATUS (*get_string)(DEVICE_OBJECT *device, int index, WCHAR *buffer, DWORD length); > } platform_vtbl; > > void *get_platform_private(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c > index 22ea07d..4ded568 100644 > --- a/dlls/winebus.sys/bus_udev.c > +++ b/dlls/winebus.sys/bus_udev.c > @@ -51,7 +51,9 @@ > #include "winnls.h" > #include "winternl.h" > #include "ddk/wdm.h" > +#include "ddk/hidtypes.h" > #include "wine/debug.h" > +#include "wine/unicode.h" > > #include "bus.h" > > @@ -144,10 +146,78 @@ static NTSTATUS hidraw_get_reportdescriptor(DEVICE_OBJECT *device, BYTE *buffer, > #endif > } > > +static NTSTATUS hidraw_get_string(DEVICE_OBJECT *device, int index, WCHAR *buffer, DWORD length) > +{ > + struct udev_device *usbdev; > + struct platform_private *private = impl_from_DEVICE_OBJECT(device); > + > + usbdev = udev_device_get_parent_with_subsystem_devtype(private->udev_device, "usb", "usb_device"); > + if (usbdev) > + { > + WCHAR *str = NULL; > + switch (index) > + { > + case HID_STRING_ID_IPRODUCT: > + str = get_sysattr_string(usbdev, "product"); > + break; > + case HID_STRING_ID_IMANUFACTURER: > + str = get_sysattr_string(usbdev, "manufacturer"); > + break; > + case HID_STRING_ID_ISERIALNUMBER: > + str = get_sysattr_string(usbdev, "serial"); > + break; > + default: > + ERR("Unhandled string index %i\n", index); > + return STATUS_NOT_IMPLEMENTED; > + } > + if (str) > + { > + if (strlenW(str) >= length) > + { > + HeapFree(GetProcessHeap(), 0, str); > + return STATUS_BUFFER_TOO_SMALL; > + } > + strcpyW(buffer, str); > + HeapFree(GetProcessHeap(), 0, str); > + return STATUS_SUCCESS; > + } > + else if (length) > + buffer[0] = 0; > + return STATUS_SUCCESS; Shouldn't it still return STATUS_BUFFER_TOO_SMALL when length == 0? > + } > + else > + { > +#ifdef HAVE_LINUX_HIDRAW_H > + static char str[MAX_PATH] = {0}; Isn't there a risk of race-conditions that multiple HID devices attempt to use this static buffer at the same time? > + > + switch (index) > + { > + case HID_STRING_ID_IPRODUCT: > + ioctl(private->device_fd, HIDIOCGRAWNAME(MAX_PATH), &str); > + break; > + case HID_STRING_ID_IMANUFACTURER: > + break; > + case HID_STRING_ID_ISERIALNUMBER: > + break; > + default: > + ERR("Unhandled string index %i\n", index); > + return STATUS_NOT_IMPLEMENTED; > + } > +#endif > + if (str[0]) If HAVE_LINUX_HIDRAW_H is not available this will break compilation (str undeclared). > + MultiByteToWideChar(CP_ACP, 0, str, -1, buffer, length); > + else if (length) > + buffer[0] = 0; It would be better to do length checking also in this case. > + > + return STATUS_SUCCESS; > + } > +} > + > static const platform_vtbl hidraw_vtbl = > { > compare_platform_device, > hidraw_get_reportdescriptor, > + hidraw_get_string, > }; > > static void try_add_device(struct udev_device *dev) > diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c > index ad09ec1..a7b76df 100644 > --- a/dlls/winebus.sys/main.c > +++ b/dlls/winebus.sys/main.c > @@ -421,6 +421,17 @@ NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) > irp->IoStatus.Information = length; > break; > } > + case IOCTL_HID_GET_STRING: > + { > + DWORD index = (DWORD)irpsp->Parameters.DeviceIoControl.Type3InputBuffer; This will trigger a compilation warning on 64-bit. You have to cast to a ptr-size integer type first, for example: DWORD index = (ULONG_PTR)irpsp->Parameters.DeviceIoControl.Type3InputBuffer; Also, accoding to MSDN, the content of the variable is a composite value: """Parameters.DeviceIoControl.Type3InputBuffer in the I/O stack location of the IRP contains a composite value. The two most significant bytes contain the language ID of the string to be retrieved. The two least significant bytes contain one of the following three constant values...""" > + TRACE("IOCTL_HID_GET_STRING[%i]\n", index); > + irp->IoStatus.u.Status = status = ext->vtbl->get_string(device, > + index, (WCHAR*)(irp->UserBuffer), > + irpsp->Parameters.DeviceIoControl.OutputBufferLength/sizeof(WCHAR)); > + if (status == STATUS_SUCCESS) > + irp->IoStatus.Information = (strlenW((WCHAR*)(irp->UserBuffer)) + 1) * sizeof(WCHAR); > + break; > + } > default: > { > ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode; > > >

2 2

Re: [PATCH 1/2] ws2_32: Pass completion routing to NtDeviceIoControlFile in server_ioctl_sock only if caller provided comple
by Marvin Oct. 12, 2016

Oct. 12, 2016

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=26126 Your paranoid android. === wvistau64 (32 bit sock) === An error occurred while waiting for the test to complete: network read timed out (wait2/connect:AgentVersion.h:0/9)

1 0

Re: [PATCH 1/2] Tests for CertRegisterSystemStore
by Marvin Oct. 12, 2016

Oct. 12, 2016

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=26111 Your paranoid android. === w864 (64 bit store) === store.c:2664: Test failed: CertAddCertificateContextToStore failed store.c:2665: Test failed: Unexpected cert3 store: unhandled exception c0000005 at 000000000044B790

1 0

Re: [resend] riched20-add-support-for-EM_PASTESPECIAL
by Huw Davies Oct. 12, 2016

Oct. 12, 2016

On Tue, Oct 11, 2016 at 10:54:44PM -0600, Kenneth Haley wrote: > partial fix for https://bugs.winehq.org/show_bug.cgi?id=14530 > > Support CF_TEXT & CF_UNICODETEXT clipboard formats. > > Tested on Arch Linux. > > Signed-off-by: K. Haley <haleykd(a)users.sf.net> > --- > dlls/riched20/editor.c | 42 +++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 41 insertions(+), 1 deletion(-) > > diff --git a/dlls/riched20/editor.c b/dlls/riched20/editor.c > index 8b94ccf..7eeda2a 100644 > --- a/dlls/riched20/editor.c > +++ b/dlls/riched20/editor.c > @@ -2232,6 +2232,45 @@ static BOOL ME_Paste(ME_TextEditor *editor) > return TRUE; > } > > +static BOOL ME_PasteSpecial(ME_TextEditor *editor, BOOL unicode, UINT cf, REPASTESPECIAL *rps) > +{ > + DWORD dwFormat = SF_TEXT; > + EDITSTREAM es; > + ME_GlobalDestStruct gds; > + > + if (rps != NULL && rps->dwAspect != 0) { > + FIXME("REPASTESPECIAL structure handling not yet implemented\n"); > + } > + > + switch(cf) > + { > + // on XP CF_TEXT appears to paste unicode into W version of controll > + case CF_TEXT: > + case CF_UNICODETEXT: > + cf=CF_UNICODETEXT; > + if (!IsClipboardFormatAvailable(cf)) > + return FALSE; > + es.pfnCallback = ME_ReadFromHGLOBALUnicode; > + dwFormat|=SF_UNICODE; > + if (!unicode) > + FIXME("convert unicode to ANSI\n"); > + break; > + default: > + FIXME("Clipboard formats other than CF_TEXT and CF_UNICODETEXT not yet implemented!\n"); > + return FALSE; > + } > + > + if (!OpenClipboard(editor->hWnd)) > + return FALSE; > + gds.hData = GetClipboardData(cf); > + gds.nLength = 0; > + es.dwCookie = (DWORD_PTR)&gds; > + ME_StreamIn(editor, dwFormat|SFF_SELECTION, &es, FALSE); > + > + CloseClipboard(); > + return TRUE; > +} Sorry, I let this patch slip last time. Thanks for resubmitting. Unfortunately, this needs to be merged with the regular pasting code, which itself needs some work. Probably with regular paste querying for formats we want and calling an underlying 'getdata' function. Paste special would also call this function with the format specified. Additionally, all of this should use the Ole clipboard rather than the user32 api. Also note, we don't want C++ style comments in Wine. Huw.

1 0

Re: wine X button on top bar
by Ken Thomases Oct. 12, 2016

Oct. 12, 2016

I'm CC'ing the list. Please keep the discussion on the list. On Oct 10, 2016, at 12:26 PM, A B <dianaxxyyzz(a)gmail.com> wrote: > > If we run the command: wine program.exe , does wine save the hwnd of the main window for the program.exe ? And how to get it ? For example if I want to printf in wine logs ,the hwnd of the main window of program.exe , is this hwnd saved somewhere in a wine variable ? The concept of "main window" of a program is purely a human interpretation. There's no such concept in the Win32 API. Therefore, there's no specific variable in Wine which holds the "main" window's handle. You can collect a +win debug log and you'll see the window handle of all of the windows created by the program (along with a lot of activity related to them). It's up to you to determine which is the main window. -Ken

3 5

Re: [PATCH v4 0/2] Don't call exit in the shared library.
by Sebastian Lackner Oct. 11, 2016

Oct. 11, 2016

On 11.10.2016 16:21, Jens Reyer wrote: > This includes dropping the wrong ones pointed out by Sebastian > Lackner (Thanks a lot! Note however that it always compiled just > fine with these changes) and the rejected ones for wpp (I assume > they are not related to the shared library). Those changes were in an #ifdef block, I assume the code was just never compiled on Debian.

1 0

Re: ntoskrnl.exe: add KeAcquireInStackQueuedSpinLock stub
by Sebastian Lackner Oct. 11, 2016

Oct. 11, 2016

On 11.10.2016 03:17, Austin English wrote: > For https://bugs.winehq.org/show_bug.cgi?id=41472 > In my header files it is defined as FASTCALL: --- snip --- NTKERNELAPI VOID FASTCALL KeAcquireInStackQueuedSpinLock ( _Inout_ PKSPIN_LOCK SpinLock, _Out_ PKLOCK_QUEUE_HANDLE LockHandle ); --- snip --- You will have to use the DEFINE_FASTCALL*_ENTRYPOINT macros to implement it. In the spec files it should also be marked with -norelay then. Regards, Sebastian

1 0

Re: [PATCH] d2d1: Make some of drawing calls a wrappers over DrawGeometry()
by Henri Verbeet Oct. 11, 2016

Oct. 11, 2016

On 10 October 2016 at 16:29, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/d2d1/render_target.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 42 insertions(+), 3 deletions(-) > This is ok, although for future reference, I'd prefer those as "Implement d2d_d3d_render_target_DrawRectangle().", etc.

1 0

Re: ntdll: give user an error message if WINEARCH is not win32/win64
by Sebastian Lackner Oct. 11, 2016

Oct. 11, 2016

On 11.10.2016 03:46, Austin English wrote: > Fixes https://bugs.winehq.org/show_bug.cgi?id=41378 > > See also https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=838474 At the time WINEARCH is checked it is already too late. The wineprefix has already been created (with fallback to the native arch if something weird was passed).

1 0

Re: [PATCH resend] winex11: Send SC_KEYMENU on managed windows.
by Marvin Oct. 11, 2016

Oct. 11, 2016

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=26094 Your paranoid android. === w7pro64 (32 bit msg) === msg.c:11036: Test failed: msg.message = 30 instead of WM_TIMER msg.c:11040: Test failed: msg.message = 30 instead of WM_TIMER msg.c:11043: Test failed: msg.message = 30 instead of WM_TIMER === w1064 (64 bit msg) === msg.c:7444: Test failed: destroy child on thread exit: 3: the msg sequence is not complete: expected 0000 - actual 001a

1 0

Re: [PATCH 1/2] shell32: add SHRemoveLocalizedName stub
by Dmitry Timoshkov Oct. 11, 2016

Oct. 11, 2016

Austin English <austinenglish(a)gmail.com> wrote: > +HRESULT SHRemoveLocalizedName(const WCHAR *path) > +{ > + FIXME("%s stub\n", debugstr_w(path)); > + return S_OK; > +} Shouldn't it be WINAPI? -- Dmitry.

2 1

visual studio 6 create file read only
by A B Oct. 10, 2016

Oct. 10, 2016

Hello , Visual studio 6 create files read only , but photoshop is fine (r,w) . What could be the cause?

1 2

wine X button on top bar
by A B Oct. 10, 2016

Oct. 10, 2016

Hello, If "Allow windows manager to decorate window" is on. What is the function name that knows a user click this X button and send the WM_CLOSE to the app? Thank you

2 1

Re: [v2 PATCH] d2d1: Support shared bitmaps created from dxgi surface
by Henri Verbeet Oct. 10, 2016

Oct. 10, 2016

On 8 October 2016 at 19:18, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > -HRESULT d2d_bitmap_create_shared(ID2D1Factory *factory, ID3D10Device *target_device, > +HRESULT d2d_bitmap_create_shared(ID2D1RenderTarget *render_target, ID3D10Device *target_device, > REFIID iid, void *data, const D2D1_BITMAP_PROPERTIES *desc, struct d2d_bitmap **bitmap) > { > + D2D1_BITMAP_PROPERTIES d; > + ID2D1Factory *factory; > + > + ID2D1RenderTarget_GetFactory(render_target, &factory); > + ID2D1Factory_Release(factory); This is a little improper. It does save a few lines, but I'd rather just release it after we're actually done with it. > + else if (IsEqualGUID(iid, &IID_IDXGISurface) || IsEqualGUID(iid, &IID_IDXGISurface1)) "else" is redundant here, although that's minor. > + { > + ID3D10ShaderResourceView *view; > + DXGI_SURFACE_DESC surface_desc; > + IDXGISurface *surface = data; > + ID3D10Resource *resource; > + D2D1_SIZE_U pixel_size; > + HRESULT hr; > + > + if (FAILED(IDXGISurface_QueryInterface(surface, &IID_ID3D10Resource, (void **)&resource))) > + { > + WARN("Failed to get d3d resource from dxgi surface.\n"); > + return E_FAIL; > + } > + > + hr = ID3D10Device_CreateShaderResourceView(target_device, resource, NULL, &view); I think we should validate the dxgi surface's device matches the target device first, like in the IID_ID2D1Bitmap case. It's in fact possible to share dxgi resources between different devices and supposedly even different processes, but doing that would require using IDXGIResource::GetSharedHandle(), ID3D10Device::OpenSharedResource(), and D3D10_RESOURCE_MISC_SHARED, all of which are currently unimplemented in Wine. I'm not sure if d2d1 allows it. > + /* Shared DXGI surface. */ > + desc.type = D2D1_RENDER_TARGET_TYPE_DEFAULT; > + desc.pixelFormat.format = DXGI_FORMAT_B8G8R8A8_UNORM; > + desc.pixelFormat.alphaMode = D2D1_ALPHA_MODE_PREMULTIPLIED; > + desc.dpiX = 0.0f; > + desc.dpiY = 0.0f; > + desc.usage = D2D1_RENDER_TARGET_USAGE_NONE; > + desc.minLevel = D2D1_FEATURE_LEVEL_DEFAULT; > + > + hr = ID2D1Factory_CreateDxgiSurfaceRenderTarget(factory1, surface2, &desc, &rt2); > + ok(SUCCEEDED(hr), "Failed to create render target, hr %#x.\n", hr); > + > + bitmap_desc.pixelFormat.format = DXGI_FORMAT_B8G8R8A8_UNORM; > + bitmap_desc.pixelFormat.alphaMode = D2D1_ALPHA_MODE_PREMULTIPLIED; > + bitmap_desc.dpiX = 0.0f; > + bitmap_desc.dpiY = 0.0f; > + > + hr = ID2D1RenderTarget_CreateSharedBitmap(rt2, &IID_IDXGISurface, surface2, &bitmap_desc, &bitmap2); > + ok(SUCCEEDED(hr) || broken(hr == E_INVALIDARG) /* vista */, "Failed to create bitmap, hr %#x.\n", hr); It's interesting that this works. I'd expect actually drawing with that bitmap on that render target to be undefined though.

2 2

Running drivers in userspace/VM
by KOLANICH Oct. 9, 2016

Oct. 9, 2016

Hello everybody. ReactOS project is devoted to making an OS kernel binary compatible with NT kernel. In their official FAQ it is written that they cannot be replaced with GNU/Linux + Wine because of drivers which cannot be run in Wine. I'm a bit sceptical about their OS, so I wonder why cannot drivers be run in wine? Drivers are run in ring 0 or 1, right? Why it is impossible/too complicated to a) expose some privileged instructions into userland through API; b) expose IO space using IOPL; c) make a mock of NT kernel to allow the drivers use its ABI; d) patch the binaries automatically to use API instead of privileged instructions; e) if a driver doesn't works well enough in userland, create a micro-VM using VT-x and VT-d and put there NT kernel mock and the driver?

3 2

Re: [PATCH] d2d1: Support shared bitmaps created using dxgi surface
by Marvin Oct. 7, 2016

Oct. 7, 2016

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=26077 Your paranoid android. === wvistau64 (32 bit d2d1) === d2d1.c:2194: Test failed: Failed to create bitmap, hr 0x80070057. d2d1: unhandled exception c0000005 at 0040C9E8 === w2008s64 (32 bit d2d1) === d2d1.c:2194: Test failed: Failed to create bitmap, hr 0x80070057. d2d1: unhandled exception c0000005 at 0040C9E8 === wvistau64 (64 bit d2d1) === d2d1.c:2194: Test failed: Failed to create bitmap, hr 0x80070057. d2d1: unhandled exception c0000005 at 000000000040C3BC === w2008s64 (64 bit d2d1) === d2d1.c:2194: Test failed: Failed to create bitmap, hr 0x80070057. d2d1: unhandled exception c0000005 at 000000000040C3BC

1 0

Re: [PATCH v3 5/6] libwine: Don't call exit in the shared library.
by Sebastian Lackner Oct. 7, 2016

Oct. 7, 2016

On 07.10.2016 18:39, Jens Reyer wrote: > @@ -140,18 +140,18 @@ static int try_mmap_fixed (void *addr, size_t len, int prot, int flags, > mapped and we must fail. */ > for ( i = 0; i < len; i += pagesize ) > if ( mincore( (caddr_t)addr + i, pagesize, &vec ) != -1 ) > - _exit(1); > + abort(); > > /* Perform the mapping with MAP_FIXED set. This is safe > now, as none of the pages is currently in use. */ > result = mmap( addr, len, prot, flags | MAP_FIXED, fildes, off ); > if ( result == addr ) > - _exit(0); > + break; > > if ( result != (void *) -1 ) /* This should never happen ... */ > munmap( result, len ); > > - _exit(1); > + abort(); > } > > /* reap child */ The _exit() calls are happening from a child process here, so there is nothing to change. I also doubt it will compile with the "break;" which is not even inside a loop. Regards, Sebastian

1 0

Re: [PATCH v3 1/6] libport: Don't call exit in the shared library.
by Sebastian Lackner Oct. 7, 2016

Oct. 7, 2016

On 07.10.2016 18:39, Jens Reyer wrote: > From: Michael Gilbert <mgilbert(a)debian.org> > Signed-off-by: Jens Reyer <jre.winesim(a)gmail.com> > --- > libs/port/spawn.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/libs/port/spawn.c b/libs/port/spawn.c > index 97364e7..c1a95a2 100644 > --- a/libs/port/spawn.c > +++ b/libs/port/spawn.c > @@ -55,20 +55,20 @@ int _spawnvp(int mode, const char *cmdname, const char *const argv[]) > if (mode == _P_DETACH) > { > pid = fork(); > - if (pid == -1) _exit(1); > - else if (pid > 0) _exit(0); > + if (pid == -1) abort(); > + else if (pid > 0) return pid; The _exit() calls are happening from a child process here, so there is nothing to change. In fact, returning to the caller even breaks the code. Regards, Sebastian > /* else in grandchild */ > } > > signal( SIGPIPE, SIG_DFL ); > execvp(cmdname, (char **)argv); > - _exit(1); > + abort(); > } > > if (pid == -1) > return -1; > > - if (mode == _P_OVERLAY) exit(0); > + if (mode == _P_OVERLAY) abort(); > > if (mode == _P_WAIT || mode == _P_DETACH) > { >

1 0

Re: wined3d: Add missing '\n' to FIXME() messages.
by Henri Verbeet Oct. 7, 2016

Oct. 7, 2016

On 6 October 2016 at 15:31, Andrey Gusev <andrey.goosev(a)gmail.com> wrote: > + FIXME("unknown_semantics(0x%08x).\n", semantic->usage); Could you change that to something like "Unrecognised semantic usage %#x.\n"? > + if (mmask) FIXME("_unrecognized_modifier(%#x).\n", mmask); Similarly, something like "Unrecognised modifier %#x.\n" would probably be more appropriate here.

1 0

Re: [PATCH 02/16] rpcrt4: Restore original error code when ReadFile fails with ERROR_MORE_DATA.
by Alexandre Julliard Oct. 6, 2016

Oct. 6, 2016

Jacek Caban <jacek(a)codeweavers.com> writes: > @@ -448,6 +448,7 @@ static int rpcrt4_conn_np_read(RpcConnection *Connection, > char *buf = buffer; > BOOL ret = TRUE; > unsigned int bytes_left = count; > + DWORD err = GetLastError(); > > while (bytes_left) > { > @@ -460,6 +461,7 @@ static int rpcrt4_conn_np_read(RpcConnection *Connection, > bytes_left -= bytes_read; > buf += bytes_read; > } > + if (ret) SetLastError(err); I'd suggest using NtReadFile instead. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH] winebus.sys: Implement IOCTL_HID_GET_DEVICE_ATTRIBUTES for hid devices
by Sebastian Lackner Oct. 6, 2016

Oct. 6, 2016

On 06.10.2016 04:51, Aric Stewart wrote: > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus.h | 1 + > dlls/winebus.sys/bus_udev.c | 1 + > dlls/winebus.sys/main.c | 41 +++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 43 insertions(+) > > > > 0001-winebus.sys-Implement-IOCTL_HID_GET_DEVICE_ATTRIBUTES-.txt > > > diff --git a/dlls/winebus.sys/bus.h b/dlls/winebus.sys/bus.h > index 099558b..342a66b 100644 > --- a/dlls/winebus.sys/bus.h > +++ b/dlls/winebus.sys/bus.h > @@ -34,3 +34,4 @@ DEVICE_OBJECT *bus_create_hid_device(DRIVER_OBJECT *driver, const WCHAR *busidW, > const GUID *class, const platform_vtbl *vtbl, DWORD platform_data_size) DECLSPEC_HIDDEN; > DEVICE_OBJECT *bus_find_hid_device(const platform_vtbl *vtbl, void *platform_dev) DECLSPEC_HIDDEN; > void bus_remove_hid_device(DEVICE_OBJECT *device) DECLSPEC_HIDDEN; > +NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) DECLSPEC_HIDDEN; > diff --git a/dlls/winebus.sys/bus_udev.c b/dlls/winebus.sys/bus_udev.c > index 93e56f3..a01d88e 100644 > --- a/dlls/winebus.sys/bus_udev.c > +++ b/dlls/winebus.sys/bus_udev.c > @@ -299,6 +299,7 @@ NTSTATUS WINAPI udev_driver_init(DRIVER_OBJECT *driver, UNICODE_STRING *registry > > udev_driver_obj = driver; > driver->MajorFunction[IRP_MJ_PNP] = common_pnp_dispatch; > + driver->MajorFunction[IRP_MJ_INTERNAL_DEVICE_CONTROL] = hid_internal_dispatch; I guess you intentionally did not name it "common_*" because non-HID devices might need a different handler, is that correct? > > if (!(events[0] = CreateEventW(NULL, TRUE, FALSE, NULL))) > goto error; > diff --git a/dlls/winebus.sys/main.c b/dlls/winebus.sys/main.c > index 09a59d6..ef19322 100644 > --- a/dlls/winebus.sys/main.c > +++ b/dlls/winebus.sys/main.c > @@ -30,7 +30,9 @@ > #include "winternl.h" > #include "winreg.h" > #include "setupapi.h" > +#include "winioctl.h" > #include "ddk/wdm.h" > +#include "ddk/hidport.h" > #include "wine/debug.h" > #include "wine/unicode.h" > #include "wine/list.h" > @@ -346,6 +348,45 @@ NTSTATUS WINAPI common_pnp_dispatch(DEVICE_OBJECT *device, IRP *irp) > return status; > } > > +NTSTATUS WINAPI hid_internal_dispatch(DEVICE_OBJECT *device, IRP *irp) > +{ > + NTSTATUS rc = irp->IoStatus.u.Status; I would suggest to use rename the variable to "status". > + IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation(irp); > + struct device_extension *extension = (struct device_extension *)device->DeviceExtension; > + > + TRACE("(%p, %p)\n", device, irp); > + > + switch(irpsp->Parameters.DeviceIoControl.IoControlCode) There is a space missing after "switch". > + { > + case IOCTL_HID_GET_DEVICE_ATTRIBUTES: > + { > + PHID_DEVICE_ATTRIBUTES attr = (PHID_DEVICE_ATTRIBUTES)irp->UserBuffer; It would be better to use HID_DEVICE_ATTRIBUTES * instead of the version with the "P" prefix. Also, please add a check for Parameters.DeviceIoControl.OutputBufferLength to make sure the buffer is big enough. > + TRACE("IOCTL_HID_GET_DEVICE_ATTRIBUTES\n"); > + > + RtlZeroMemory(attr, sizeof(HID_DEVICE_ATTRIBUTES)); Is there any specific reason to use RtlZeroMemory instead of memset? Something like memset(attr, 0, sizeof(*attr)) would be easier. ;) > + attr->Size = sizeof(HID_DEVICE_ATTRIBUTES); > + attr->VendorID = extension->vid; > + attr->ProductID = extension->pid; > + attr->VersionNumber = extension->version; > + irp->IoStatus.u.Status = rc = STATUS_SUCCESS; > + irp->IoStatus.Information = sizeof(HID_DEVICE_ATTRIBUTES); > + break; > + } > + default: > + { > + ULONG code = irpsp->Parameters.DeviceIoControl.IoControlCode; > + FIXME("Unsupported ioctl %x (device=%x access=%x func=%x method=%x)\n", > + code, code >> 16, (code >> 14) & 3, (code >> 2) & 0xfff, code & 3); > + break; > + } > + } > + > + if (rc != STATUS_PENDING) > + IoCompleteRequest( irp, IO_NO_INCREMENT ); Do you need to handle asynchronous requests in this function? If not you can remove the if check. For consistency reasons, you should probably remove the space after "(" and before ")" in the IoCompleteRequest call. > + > + return rc; > +} > + > NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) > { > static const WCHAR udevW[] = {'\\','D','r','i','v','e','r','\\','U','D','E','V',0}; > > >

2 1

Problems with WM_DROPFILES
by John Found Oct. 6, 2016

Oct. 6, 2016

I just realized, that in WINE, my windows receive WM_DROPFILES only if they are top level windows. All children windows never accept WM_DROPFILES, regardless of the presence of WS_EX_ACCEPTFILES or calling DragAcceptFiles() for this child window. In Windows everything works OK and the children correctly receive WM_DROPFILES message. Is it a bug in WINE, or something OS/WM dependent? How to fix it? Is it a good idea, for example, to accept the message in the main window and dispatch it to the respective children windows (by using DragQueryPoint) - this approach should work both in WINE and Windows. WINE v1.9.19 XFCE4 window manager. -- http://fresh.flatassembler.net http://asm32.info John Found <johnfound(a)asm32.info>

3 7

Re: [PATCH 5/7] wined3d: Avoid destroying active contexts.
by Henri Verbeet Oct. 6, 2016

Oct. 6, 2016

On 5 October 2016 at 11:17, Józef Kucia <jkucia(a)codeweavers.com> wrote: > Ideally, a pointer to a swapchain would be completely removed from > struct wined3d_context. Yeah. One of the things that made me hesitate a bit about the first patch in this series is that it was originally an explicit design decision to avoid storing a pointer to the device in the context. At the time we needed the swapchain pointer, I think for the window or DC handle or something. Over time the reasons for having the swapchain pointer mostly went away, and it ended up as more of a way to get around not being able to access the device. Perhaps that's worse than what we were trying to avoid. Still, ideally the context wouldn't have a device pointer either. I think the legitimate accesses to the device are mostly either accessing shared resources like textures and samplers, or using the device to get at the wined3d creation flags. We can probably store the shared resources in some structure inside the device, and then store a pointer to that in the context instead, and probably just copy the flags.

2 3

Re: [PATCH] d2d1/tests: Initial tests for bitmap render target
by Henri Verbeet Oct. 6, 2016

Oct. 6, 2016

On 5 October 2016 at 17:33, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > +static inline BOOL float_eq(FLOAT left, FLOAT right) > +{ > + int x = *(int *)&left; > + int y = *(int *)&right; > + > + if (x < 0) > + x = INT_MIN - x; > + if (y < 0) > + y = INT_MIN - y; > + > + return abs(x - y) <= 8; > +} Not wrong per se, but in the D3D tests that's generally called "compare_float()" and takes an explicit "ulps" argument. Is "8" based on test results or a guess? It seems fairly large for something that shouldn't be a particularly complicated calculation. Also, "float". > + /* Both pixel size and DIP size are specified. */ > + set_size_u(&pixel_size, 128, 128); > + hr = ID2D1HwndRenderTarget_CreateCompatibleRenderTarget(hwnd_rt, &size, &pixel_size, NULL, > + D2D1_COMPATIBLE_RENDER_TARGET_OPTIONS_NONE, &rt); > + ok(SUCCEEDED(hr), "Failed to create render target, hr %#x.\n", hr); > + > + /* Doubled pixel size dimensions with the same DIP size give doubled dpi. */ > + ID2D1BitmapRenderTarget_GetDpi(rt, dpi2, dpi2 + 1); > + ok(dpi[0] == dpi2[0] / 2.0f && dpi[1] == dpi2[1] / 2.0f, "Got dpi mismatch.\n"); As an aside, the reason some of the existing tests use different DPI settings for the x and y axes is to avoid accidentally swapping them in the implementation.

1 0

Re: [PATCH v2] winebus.sys: Implement common IRP_MJ_POWER handling
by Sebastian Lackner Oct. 5, 2016

Oct. 5, 2016

On 05.10.2016 15:01, Aric Stewart wrote: > > v2: Suggestions from Sebastian Lackner > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/winebus.sys/bus.h | 1 + > dlls/winebus.sys/bus_udev.c | 1 + > dlls/winebus.sys/main.c | 21 +++++++++++++++++++++ > 3 files changed, 23 insertions(+) I have submitted a patch which initializes all MajorFunction handlers with a stub implementation [1]. This also matches Windows behavior. Please let me know if you think this patch should still be applied (I guess on Windows the bus drivers handle IRP_MJ_POWER, right?) or if we should skip it for now if it was just to fix the crash. [1] http://source.winehq.org/patches/data/126881 Regards, Sebastian

2 1

Re: [PATCH 16/16] ntdll/tests: Added a test of using NtCancelIoEx on iosb associated with pipe listening.
by Marvin Oct. 5, 2016

Oct. 5, 2016

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=26052 Your paranoid android. === w864 (32 bit pipe) === pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: c0000000, GetLastError: 231 === w1064 (32 bit pipe) === pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: c0000000, GetLastError: 231 === w864 (64 bit pipe) === pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: c0000000, GetLastError: 231 === w1064 (64 bit pipe) === pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 1, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 2, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 0, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 80000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: 40000000, GetLastError: 231 pipe.c:242: Test failed: CreateFile failed for sharing 3, access: c0000000, GetLastError: 231

1 0

wine-1.8.5-rc2 Available
by Michael Stefaniuc Oct. 5, 2016

Oct. 5, 2016

Hello guys, wine-1.8.5-rc2 is now available, including the tag: https://github.com/mstefani/wine-stable https://static-winehq.rhcloud.com/stable/ The commit "user32: Prevent resetting WS_MINIMIZE through SetWindowLong." was deferred as Józef and Alexandre were not convinced about that one. bye michael

2 1