June 2011 - Wine-Devel - list.winehq.org

Re: shdocvw: Add ITaskbarList2 Interface

by Henri Verbeet

2011/6/18 André Hentschel <nerv(a)dawncrow.de>: > -static HRESULT STDMETHODCALLTYPE taskbar_list_QueryInterface(ITaskbarList *iface, REFIID riid, void **object) > +static HRESULT STDMETHODCALLTYPE taskbar_list_QueryInterface(ITaskbarList2 *iface, REFIID riid, LPVOID *ppv) > { > - TRACE("iface %p, riid %s, object %p\n", iface, debugstr_guid(riid), object); > + struct taskbar_list *This = impl_from_ITaskbarList2(iface); > + > + *ppv = NULL; > + > + if(IsEqualGUID(&IID_IUnknown, riid)) { > + TRACE("(%p)->(IID_IUnknown %p)\n", This, ppv); > + *ppv = &This->ITaskbarList2_iface; > + }else if(IsEqualGUID(&IID_ITaskbarList, riid)) { > + TRACE("(%p)->(IID_ITaskbarList %p)\n", This, ppv); > + *ppv = &This->ITaskbarList2_iface; > + }else if(IsEqualGUID(&IID_ITaskbarList2, riid)) { > + TRACE("(%p)->(IID_ITaskbarList2 %p)\n", This, ppv); > + *ppv = &This->ITaskbarList2_iface; > + } > > - if (IsEqualGUID(riid, &IID_ITaskbarList) > - || IsEqualGUID(riid, &IID_IUnknown)) > - { > - IUnknown_AddRef(iface); > - *object = iface; > + if(*ppv) { > + IUnknown_AddRef((IUnknown*)*ppv); > return S_OK; > } > > - WARN("%s not implemented, returning E_NOINTERFACE\n", debugstr_guid(riid)); > - > - *object = NULL; > + WARN("(%p)->(%s %p) interface not supported\n", This, debugstr_guid(riid), ppv); > return E_NOINTERFACE; > } > I'm not sure that's really an improvement.

June 20, 2011

Re: Disablin xinput2 for now?

by Alexandre Julliard

Vitaliy Margolen <wine-devel(a)kievinfo.com> writes: > On 06/20/2011 02:55 AM, Marcus Meissner wrote: >> As this might be a X.org issue, should we packagers turn off >> xinput2 support for the time being? > > If that will fix all dinput regressions - absolutely. Or add a > registry option to disable xinput2. It will of course fix some regressions, and introduce new ones for things that have been fixed by the changes. Don't tell me that dinput was working perfectly before. And there is a registry option (GrabPointer). -- Alexandre Julliard julliard(a)winehq.org

June 20, 2011

Re: [PATCH 2/3] server: Added server-side raw input function implementations (try 4)

by Vitaliy Margolen

On 06/20/2011 02:55 AM, Vincas Miliūnas wrote: > (see the message in the first part of the patch) > +struct raw_input > +{ > + struct list entry; > + unsigned short usage_page; > + unsigned short usage; > + BOOL processed; > + BOOL retrieved; > + RAWINPUT raw; > + > + /* common fields */ > + unsigned int type; > + unsigned int message; > + unsigned int flags; > + unsigned int extra_info; > + > + /* mouse fields */ > + unsigned int point_x; > + unsigned int point_y; > + unsigned int mouse_data; > + unsigned int cursor_x; > + unsigned int cursor_y; > + > + /* keyboard fields */ > + unsigned int vk_code; > + unsigned int scan_code; > +}; Mouse and keyboard fields should be a union to save space. > +#define NUM_RAW_DEVICES (sizeof( raw_devices ) / sizeof( struct raw_device )) Please use "sizeof(array)/sizeof(array[0])" construct instead. > + UINT i, size_in_bytes; In server please use standard C types ("unsigned int" in this case). > + /* Currently fake handles are provided, however they are only used for the GetRawInputDeviceInfo function, > + thus it should not create any undesirable side effects */ > + result[i].hDevice = (HANDLE)raw_devices[i].handle; This doesn't sound right. What this handle can be used for? > + if (!(raw = malloc( sizeof( *raw ) ))) In server please use mem_alloc() instead. > +/* Unregister a raw input device */ > +DECL_HANDLER(unregister_raw_input_device) I don't see handing of case where device is not found. Should you return error instead? > + LIST_FOR_EACH( registration_entry, &current->process->raw_registered ) > + { > + struct raw_registration *registration = LIST_ENTRY( registration_entry, struct raw_registration, entry ); You can use this instead: LIST_FOR_EACH_ENTRY( registration, &current->process->raw_registered, struct raw_registration, entry ) > + /* Prevent unprocessed raw input entries from being queued indefinitely */ > + while (current->process->raw_inputs_len >= MAX_RAW_INPUT_QUEUE_LENGTH) > + { > + struct raw_input *head = LIST_ENTRY( list_head( &current->process->raw_inputs ), struct raw_input, entry ); > + list_remove( &head->entry ); > + current->process->raw_inputs_len -= 1; > + free( head ); > + } Looks like you need a circular buffer (an array of fixed size with head and tail pointers) instead of a list here. Vitaliy.

June 20, 2011

Re: [PATCH 1/3] user32/tests: Added client-side raw input function tests (try 4)

by Vitaliy Margolen

On 06/20/2011 02:55 AM, Vincas Miliūnas wrote: > Fixed function name typo, compiler warnings, code style issues in a > fragment of new code and incorrect usage of wcslen in the tests instead > of lstrlenW after previous changes. My apologies :). > > +#define DEADBEEF 0xdeadbeef Please don't do this. Just explicitly use 0xdeadbeef everywhere. > + todo_wine ok(ret == 0 && GetLastError() == DEADBEEF && count >= 1, "Given (NULL, &count, sizeof), " > + "GetRawInputDeviceList should return the number of raw input devices\n"); If call succeeded you don't need to check last error. > + device_list = malloc(count * sizeof(RAWINPUTDEVICELIST)); Don't use malloc/free in Wine. Use HeapAlloc / HeapFree. > + ret = pGetRawInputDeviceList(device_list, &count2, sizeof(RAWINPUTDEVICELIST)); > + todo_wine ok(ret == (UINT)-1 && GetLastError() == ERROR_INSUFFICIENT_BUFFER && count2 == count, > + "Given (device_list, &count2 = 0, sizeof), GetRawInputDeviceList should return an error\n"); If something fails it's nice to know the error code - you need to print it. For that you need to more GetLastError outside of ok() function call. See other tests as example. > +static BOOL get_raw_input_data_wnd_proc_was_called = FALSE; Static variables automatically set to 0 at the start. You don't need to do it explicitly. Vitaliy.

June 20, 2011

Re: [PATCH 1/4] dlls/ntdll: Add handle count to NtQuerySystemInformation

by Vitaliy Margolen

On 06/20/2011 05:28 AM, Austin Lund wrote: > Thanks for the review. Does this patch address these concerns correctly? No quite there yet: > + SERVER_START_REQ( create_snapshot ) You not checking if this call succeeded or not. > + while (sizeof(ULONG) + sizeof(SYSTEM_HANDLE_ENTRY)*shi->Count >= len) You don't need a loop to calculate new size. > + shi = RtlReAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, shi, len); > + if (shi == NULL) > + return STATUS_NO_MEMORY; You leaking old shi here. Also why do you need to zero allocated memory if you assigning all of it valid values? Vitaliy.

June 20, 2011

Re: Disablin xinput2 for now?

by Vitaliy Margolen

On 06/20/2011 02:55 AM, Marcus Meissner wrote: > As this might be a X.org issue, should we packagers turn off > xinput2 support for the time being? If that will fix all dinput regressions - absolutely. Or add a registry option to disable xinput2. Vitaliy.

June 20, 2011

Re: [PATCH 2/2] d3dx9/line: Implemented tests for ID3DXLine's Begin and End methods.

by Matteo Bruni

2011/6/18 Charles Welton <charleswfb(a)gmail.com>: > --- > dlls/d3dx9_36/tests/line.c | 23 +++++++++++++++++++++++ > 1 files changed, 23 insertions(+), 0 deletions(-) > > + srand(time(NULL)); > + random = ((FLOAT) rand())/RAND_MAX; /* Random value between 0.0 and 1.0. We're using this later. */ I didn't check why your patch failed with the testbot. Nevertheless, you should avoid to use random values: the tests should be repeatable. Just pick some arbitrary values and use them. Also, for better clarity, you may want to use a different variable for the modified identity matrix (because, at that point, it isn't identity anymore). Maybe you could check the world matrix between ID3DXLine_Begin and ID3DXLine_End too.

June 20, 2011

Re: GSoC: dinput8 Action Mapping

by Lucas Zawacki

> Because callback doesn't have any string parameters it would be an easy > thing to do. I've made an EnumDevicesBySemantics version using A -> W calls, but it turned out to be trickier than I thought because the callback function is passed the device interfaces and then the A callback gets W interfaces. I'll send the patches with corrections later today but I really think the implementation for EnumDevicesBySemantics should remain separate right now.

June 20, 2011

Re: [PATCH 2/4] server: Add function to count only valid handles in a process

by Austin Lund

On 20 June 2011 15:12, Vitaliy Margolen <wine-devel(a)kievinfo.com> wrote: > On 06/19/2011 08:15 PM, Austin Lund wrote: >> >> [PATCH 2/4] server: Add function to count only valid handles in a process > > This makes no sense. All handles are valid inside wineserver. When looking at bug 9484 and trying to implement this functionality, I thought that the next_process call would return the number of open handles in the process. This is currently incorrect. As it stands reply->handles is the size of the allocated handle table (which is currently a power of two times 32) which is information almost totally irrelevant to anyone wanting handle information using snapshots. My patch changes this to count the number of open handles. Later patches I have use this functionality. I've attached an amendment which just changes the name of the call.

June 20, 2011

Re: [PATCH 1/4] dlls/ntdll: Add handle count to NtQuerySystemInformation

by Austin Lund

On 20 June 2011 15:08, Vitaliy Margolen <wine-devel(a)kievinfo.com> wrote: > On 06/19/2011 08:15 PM, Austin Lund wrote: >> >> + ret = wine_server_call( req ); >> + shi->Count += reply->handles; >> + len = sizeof(SYSTEM_HANDLE_ENTRY)*shi->Count + >> sizeof(ULONG); >> + shi = RtlReAllocateHeap(GetProcessHeap(), >> HEAP_ZERO_MEMORY, shi, len); > > Please do allocations outside of server call block. And add handling of > failed realloc. Also please double the allocated size, don't reallocate > after each server call. > >> + for (i = shi->Count - reply->handles; i< shi->Count; >> i++) { >> + shi->Handle[i].OwnerPid = reply->pid; >> + } > > Please follow file's curly braces style - none for single line blocks, or on > separate line for multi-line blocks. > Thanks for the review. Does this patch address these concerns correctly?

June 20, 2011

Disablin xinput2 for now?

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, >There seem to be regressions in xinput2 support I too have experienced various mouse trouble since approx. 1.3.18 on MacOS, using XQuartz 2.5.x or 2.6.1 -- which is much more recent than any of my Ubuntu systems (Lucid). As I've currently little time for regressions, I haven't yet turned that into bug reports (and had always hoped that Wine release N+1 would eventually make things work again). Haegemonia 2 aka. Legions of Iron seems to be fixed as of 1.3.22. Previously, the mouse flickered badly when moved and it moved like crazy along one y/x direction. As of 1.3.22, Black & White 2 doesn't react to the mouse anymore. >should we packagers turn off >xinput2 support for the time being? There's still hope that the situation will stabilize as present bug reports are analyzed and the causes understood. I also hope somebody will update the Wiki pages, e.g. UsefulRegistryKeys to reflect new possibilities. I would believe there are some. The Wiki is the place where I'd expect to find a summary of options and hints for when a program does not work in area Y. I know where to look for some forms of audio trouble, but what do I know about mouse trouble? Before starting git bisect, I'd prefer to try out whether a few registry keys may help with the mouse. Which ones should I try? Regards, Jörg Höhle

June 20, 2011

Re: [1/2] d3dx9/test: Add UpdateSemantics test. (try 3)

by Stefan Dösinger

These two patches look OK to me. On Friday 17 June 2011 14:19:56 Michael Mc Donnell wrote: > Removed a superfluous test. > Added NULL check before releasing mesh. > Changed struct declaration style. > Changed the struct declaration style again. > Added GetNumBytesPerVertex test > Added GetDeclaration tests > Added equals check > Renamed larger declaration test to overlapping declaration test. > Added smaller and larger declarations. > Added D3DECL_END() marker check > Moved pointer * to new_test_context (nitpick) > Added multiple streams test

June 20, 2011

Re: [PATCH 1/3] user32/tests: Added client-side raw input function tests (try 4)

by Marvin

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 http://testbot.winehq.org/JobDetails.pl?Key=11841 Your paranoid android. === W7PROX64 (64 bit input) === No test summary line found

June 20, 2011

Disablin xinput2 for now?

by Marcus Meissner

Hi, There seem to be regressions in xinput2 support, http://bugs.winehq.org/show_bug.cgi?id=27028 As this might be a X.org issue, should we packagers turn off xinput2 support for the time being? Ciao, Marcus

June 20, 2011

Re: [3/3] msxml3: Added IStream support to IMXWriter

by Adam Martinson

On 06/19/2011 09:32 PM, Thomas Mullaly wrote: > +static inline void reset_output_buffer(mxwriter *This, LPCSTR encoding) > +{ > + xmlOutputBufferClose(This->buffer); > + > + if (encoding) > + This->buffer = xmlAllocOutputBuffer(xmlFindCharEncodingHandler(encoding)); > + else if (!This->dest) > + This->buffer = xmlAllocOutputBuffer(xmlFindCharEncodingHandler("UTF-16")); > + else { > + LPSTR enc = heap_strdupWtoA(This->encoding); > + This->buffer = xmlAllocOutputBuffer(xmlFindCharEncodingHandler(enc)); > + heap_free(enc); > + } > + > + This->dest_written = 0; > +} > Libxml2 has the various encodings enum'd out as xmlCharEncoding; better to pass the enum around and avoid the string comparison. Then you can use xmlGetCharEncodingHandler() instead of xmlFindCharEncodingHandler().

June 20, 2011

Re: [1/3] msxml3/tests: Added IStream tests for IMXWriter

by Adam Martinson

On 06/19/2011 09:31 PM, Thomas Mullaly wrote: > +#define DEFINE_EXPECT(func) \ > + static BOOL expect_ ## func = FALSE, called_ ## func = FALSE > + > +#define SET_EXPECT(func) \ > + expect_ ## func = TRUE This seems like macro-overkill. I think a struct/table here may be a bit cleaner. > +static const CHAR szUtf16BOM[] = {'\xff','\xfe'}; Normal hex here would be more readable.

June 20, 2011

Re: [PATCH 2/4] server: Add function to count only valid handles in a process

by Vitaliy Margolen

On 06/19/2011 08:15 PM, Austin Lund wrote: > [PATCH 2/4] server: Add function to count only valid handles in a process This makes no sense. All handles are valid inside wineserver. Vitaliy.

June 20, 2011

Re: [PATCH 1/4] dlls/ntdll: Add handle count to NtQuerySystemInformation

by Vitaliy Margolen

On 06/19/2011 08:15 PM, Austin Lund wrote: > + ret = wine_server_call( req ); > + shi->Count += reply->handles; > + len = sizeof(SYSTEM_HANDLE_ENTRY)*shi->Count + sizeof(ULONG); > + shi = RtlReAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, shi, len); Please do allocations outside of server call block. And add handling of failed realloc. Also please double the allocated size, don't reallocate after each server call. > + for (i = shi->Count - reply->handles; i< shi->Count; i++) { > + shi->Handle[i].OwnerPid = reply->pid; > + } Please follow file's curly braces style - none for single line blocks, or on separate line for multi-line blocks. Vitaliy.

June 20, 2011

Re: ddraw: Add tests for SetCooperativeLevels

by Marvin

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 http://testbot.winehq.org/JobDetails.pl?Key=11839 Your paranoid android. === W7PROX64 (64 bit ddrawmodes) === Timeout

June 20, 2011

CopyFileExW Support

by Arthur Dene Ice

Thank you Jerome, this is my first patch and I pretty much bunged it all up. I appreciate the help. So to recap, subscribe to list first so that I can respond to messages. check. Fix my tab/spacing issue. check. and fix my patch submission subject line so it is more in line with the standard. check.

June 19, 2011

Re: [PATCH 1/6] quartz/tests: COM cleanup for the IPin iface.

by Michael Stefaniuc

On 06/19/2011 02:27 PM, Marvin wrote: > 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 > http://testbot.winehq.org/JobDetails.pl?Key=11823 > > Your paranoid android. > > > === WNT4WSSP6 (32 bit filtergraph) === > filtergraph: unhandled exception c0000005 at 00405B3C That box has no quartz.dll. bye michael

June 19, 2011

Re: [PATCH 6/6] quartz/tests: Simplify the file handling in filtergraph.c.

by Marvin

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 http://testbot.winehq.org/JobDetails.pl?Key=11828 Your paranoid android. === WNT4WSSP6 (32 bit filtergraph) === filtergraph: unhandled exception c0000005 at 00405AEC

June 19, 2011

Re: [PATCH 5/6] quartz/tests: Store the TestFilterImpl instead of its iface in IEnumPinsImpl.

by Marvin

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 http://testbot.winehq.org/JobDetails.pl?Key=11827 Your paranoid android. === WNT4WSSP6 (32 bit filtergraph) === filtergraph: unhandled exception c0000005 at 00405B2C

June 19, 2011

Re: [PATCH 4/6] quatz/tests: Pass back the object instead of the iface in createtestfilter()

by Marvin

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 http://testbot.winehq.org/JobDetails.pl?Key=11826 Your paranoid android. === WNT4WSSP6 (32 bit filtergraph) === filtergraph: unhandled exception c0000005 at 00405B2C

June 19, 2011

Re: [PATCH 3/6] quartz/tests: Finish the COM cleanup in filtergraph.c.

by Marvin

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 http://testbot.winehq.org/JobDetails.pl?Key=11825 Your paranoid android. === WNT4WSSP6 (32 bit filtergraph) === filtergraph: unhandled exception c0000005 at 00405B3C

June 19, 2011