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

Re: windowscodecs: Fix build failure on OSX.

by Austin English

On Tue, Jun 30, 2015 at 2:02 PM, Sebastian Lackner <sebastian(a)fds-team.de> wrote: > The build failure unfortunately was introduced by my last patch > http://source.winehq.org/git/wine.git/commit/863f432a7d73b8b480a742fcbaf01d… > > Somehow I missed that this part is OSX specific and tested on Linux only - sorry for > the trouble. :/ The patch was tested by jam__ on IRC, would be nice if someone could > double check that no other warnings were introduced by my original commit. > > --- > dlls/windowscodecs/icnsformat.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/windowscodecs/icnsformat.c b/dlls/windowscodecs/icnsformat.c > index 54c7707..5d48a1a 100644 > --- a/dlls/windowscodecs/icnsformat.c > +++ b/dlls/windowscodecs/icnsformat.c > @@ -179,7 +179,7 @@ static ULONG WINAPI IcnsFrameEncode_Release(IWICBitmapFrameEncode *iface) > if (This->icns_image != NULL) > HeapFree(GetProcessHeap(), 0, This->icns_image); > > - IWICBitmapFrameEncode_Release(&This->encoder->IWICBitmapFrameEncode_iface); > + IWICBitmapEncoder_Release(&This->encoder->IWICBitmapEncoder_iface); > HeapFree(GetProcessHeap(), 0, This); > } > > -- > 2.4.4 > > With your patch applied, the build is fixed for me. There are 5 warnings, but they're unrelated to your patch (deprecated warnings): Austins-Mac-mini:windowscodecs austin$ make gcc -m32 -c -o icnsformat.o icnsformat.c -I. -I../../include -I/opt/local/include/libpng16 -D__WINESRC__ \ -D_REENTRANT -fPIC -Wall -pipe -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body \ -Wignored-qualifiers -Wstrict-prototypes -Wtype-limits -Wvla -Wwrite-strings -Wpointer-arith \ -gdwarf-2 -gstrict-dwarf -fno-omit-frame-pointer -I/usr/X11/include -I/opt/local/include \ -I/usr/local/include -g -O2 icnsformat.c:430:11: warning: 'PtrToHand' is deprecated: first deprecated in OS X 10.8 [-Wdeprecated-declarations] ret = PtrToHand(This->icns_image, &handle, This->size * This->size * 4); ^ /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/MacMemory.h:1772:1: note: 'PtrToHand' has been explicitly marked deprecated here PtrToHand( ^ icnsformat.c:439:5: warning: 'DisposeHandle' is deprecated: first deprecated in OS X 10.8 [-Wdeprecated-declarations] DisposeHandle(handle); ^ /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/MacMemory.h:1278:1: note: 'DisposeHandle' has been explicitly marked deprecated here DisposeHandle(Handle h) __OSX_AVAILABLE_BUT_DEPRECATED(__MAC_10_0, __MAC_10_8, __IPHONE_NA, __IPHONE_NA); ^ icnsformat.c:526:13: warning: 'DisposeHandle' is deprecated: first deprecated in OS X 10.8 [-Wdeprecated-declarations] DisposeHandle((Handle)This->icns_family); ^ /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/MacMemory.h:1278:1: note: 'DisposeHandle' has been explicitly marked deprecated here DisposeHandle(Handle h) __OSX_AVAILABLE_BUT_DEPRECATED(__MAC_10_0, __MAC_10_8, __IPHONE_NA, __IPHONE_NA); ^ icnsformat.c:550:43: warning: 'NewHandle' is deprecated: first deprecated in OS X 10.8 [-Wdeprecated-declarations] This->icns_family = (IconFamilyHandle)NewHandle(0); ^ /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/MacMemory.h:451:1: note: 'NewHandle' has been explicitly marked deprecated here NewHandle(Size byteCount) __OSX_AVAILABLE_BUT_DEPRECATED(__MAC_10_0, __MAC_10_8, __IPHONE_NA, __IPHONE_NA); ^ icnsformat.c:667:19: warning: 'GetHandleSize' is deprecated: first deprecated in OS X 10.8 [-Wdeprecated-declarations] buffer_size = GetHandleSize((Handle)This->icns_family); ^ /System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/MacMemory.h:1356:1: note: 'GetHandleSize' has been explicitly marked deprecated here GetHandleSize(Handle h) __OSX_AVAILABLE_BUT_DEPRECATED(__MAC_10_0, __MAC_10_8, __IPHONE_NA, __IPHONE_NA); ^ 5 warnings generated. -- -Austin

June 30, 2015

Re: kernel32: K32EnumProcessModules: Handle NULL pointer gracefully.

by Alex Henrie

> diff --git a/dlls/kernel32/module.c b/dlls/kernel32/module.c > index f495e81..5461881 100644 > --- a/dlls/kernel32/module.c > +++ b/dlls/kernel32/module.c > @@ -1226,6 +1226,12 @@ BOOL WINAPI K32EnumProcessModules(HANDLE process, HMODULE *lphModule, > if (!init_module_iterator(&iter, process)) > return FALSE; > > + if (!lphModule) > + { > + SetLastError(ERROR_NOACCESS); > + return FALSE; > + } > + > if (!needed) > { > SetLastError(ERROR_NOACCESS); Wouldn't it be easier to write "if (!lphModule || !needed)"? Adding a test or two would also be nice. -Alex

June 30, 2015

A wrapper around the GTK+ rendering engine

by Ivan Akulinchev

Hello, I just want to thank you for Wine and show you my own uxtheme.dll implementation, which uses the GTK+ rendering engine. Thus Wine applications look really native for users. Here is the link: https://github.com/akulinchev/uxthemegtk (see the screenshots at the bottom of the page) If you want, I can help you to add it to the Wine code. If not, I will develop it alone as a third-party project, but then I have a question: what is the best way to replace a Wine library? I think, I shouldn't change anything in /usr/lib/wine. I know about "native libs", but it's an ELF binary, not PE. Regards, Ivan

June 30, 2015

Re: odbccp32: Implement SQLGetPrivateProfileString (try 6 - resend)

by Nikolay Sivov

> + sect = heap_strdupAtoW(lpszSection); > + entry = heap_strdupAtoW(lpszEntry); > + string = heap_strdupAtoW(lpszDefault); > + file = heap_strdupAtoW(lpszFilename); > + retval = heap_alloc(cbRetBuffer*sizeof(WCHAR)); > + > + ret = SQLGetPrivateProfileStringW(sect, entry, string, retval, cbRetBuffer, file); > + if(ret) > + { > + WideCharToMultiByte(CP_ACP, 0, retval, -1, RetBuffer, ret+1, NULL, NULL); > + } > + > + heap_free(sect); > + heap_free(entry); > + heap_free(string); > + heap_free(file); > + > + heap_free(retval); I think it's still the same issue as last time - you're using WCHAR vs CHAR buffer lengths wrong, it might work for this function as long as it's always called with simple strings. But in general WideCharToMultiByte should not be used like that: 'cbRetBuffer' you got here is A-buffer length, yet you allocate W-buffer of this length. Later when you convert W->A you use return value from W-call as A-buffer length.

June 30, 2015

Re: d3dcompiler: Add D3DCompileFromFile, D3DCompile2 stubs (try 2)

by Matteo Bruni

2015-06-30 11:01 GMT+02:00 Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>: > Hi, > Removed #if > Corrected FIXME to be consistent with the reset of the file. > > Changelog: > d3dcompiler: Add D3DCompileFromFile, D3DCompile2 stubs > > > Best Regards > Alistair Leslie-Hughes I guess it's good enough for me. For your following patches: avoid LPxxx types, watch your whitespaces, no need to manually add '\n' in the middle of trace strings.

June 30, 2015

Re: [PATCH 0/8] advapi32: Fail on missing value or val_count parameters. (was advapi32: Fix RegEnumValueW when enumerating long values)

by Nikolay Sivov

On 30.06.2015 15:37, Bernhard Übelacker wrote: > Bug #38796: > RegEnumValue currently returns wrong minimum expected data_count when called without proper > value or val_count when the retrieved data (+overhead) exceeds the fixed buffer > variable in RegEnumValue. > > This series of patches does following: > - Add tests to cover calls to RegEnumValue without proper value and val_count parameters. > I think it's better to have tests committed first and change all other places after that.

June 30, 2015

Re: [PATCH 1/6]: msvcp110: Add tr2_sys__Copy_file implementation and test.

by Piotr Caban

On 06/29/15 19:14, YongHaoHu wrote: > > > On 15/6/29 下午10:51, Piotr Caban wrote: >> On 06/29/15 16:40, YongHaoHu wrote: >>> On 15/6/29 下午5:29, Piotr Caban wrote: >>>> On 06/27/15 19:49, YongHaoHu wrote: >>>>> + ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, >>>>> tests[i].fail_if_exists); >>>>> + ok(ret == GetLastError(), "test_tr2_sys__Copy_file(): test >>>>> %d expect: %d, got %d\n", i+1, GetLastError(), ret); >>>>> + ok(errno == 0xdeadbeef, "test_tr2_sys__Copy_file(): test %d >>>>> errno expect 0xdeadbeef, got %d\n", i+1, errno); >>>> Please check the return value of the function explicitly. Currently >>>> your implementation returns error while it should succeed and it's not >>>> visible in the tests. >>> Because the last error handling is not correct in CopyFile[1], I check >>> GetLastError() instead of values. Do you have any good idea? >> If testing exact return value of function is hard please at least test >> if function succeeds/fails when it should. > Sorry, could you explain a bit more details? Please just check if function returns 0 (ERROR_SUCCESS) or something else. This way you don't need to check exact error value (only check if it's 0 or something else). >> Currently your implementation sometimes returns error even so it >> succeeds. >> > Which one do you mean ? As you know, last_error hadling is not correct > in wine's CopyFile, was this happened in Wine platform? { "f1", "tr2_test_dir\\f1_copy", TRUE } Your implementation returns error if f1_copy file already exists. It only makes sense to use GetLastError if function have failed. Thanks, Piotr

June 30, 2015

Re: [PATCH 4/6]: msvcp110: Add tr2_sys__Stat implementation and test.

by Piotr Caban

On 06/29/15 18:46, YongHaoHu wrote: > > On 15/6/29 下午10:54, Piotr Caban wrote: >> On 06/29/15 16:42, YongHaoHu wrote: >>>> Why are you handling ERROR_BAD_DEV_TYPE error here? >>>> >>> I don't have special reason, just base on the test result on testbot. >> It looks wrong. Could you please send the test here and point on what >> testbot machine it happens? >> > Yes, it's wrong. According to your advice, I tested GetFileAttributesA > and found that its last error handling was also not correct. > (It means that many ntdll functions have problems with last_error handling.) > The code I used is : GetFileAttributesA("\\\\.\\PiPe\\tests_pipe.c"); > printf("getlast err is %d\n", GetLastError()); > The output in wine is "getlast err is 66" while "|getlast err is 0" in > testbot. > > Here is the test result: > https://testbot.winehq.org/JobDetails.pl?Key=14832&log_101=1#k101 > Plesase ignore the failed msg. In this case test should be marked with todo_wine and fail in wine. Other option is to fix GetFileAttributes function.

June 30, 2015

Re: ntdll: Randomize security cookie when available (try 4)

by Alexandre Julliard

André Hentschel <nerv(a)dawncrow.de> writes: > Where would you see the best point for srand()? > __wine_process_init()? > Should i do it with a "once" variable? > Or would you prefer something like in SystemFunction036 using /dev/urandom (suggested by Bruno)? Using srand() is not a good idea since it's process-global. Something like RtlRandom is probably better. -- Alexandre Julliard julliard(a)winehq.org

June 30, 2015

Re: [PATCH]winecfg: Double click in dlls list to edit overides. (try4)

by Nikolay Sivov

On 30.06.2015 12:18, Hao Peng wrote: > Hi, Nikolay Sivov > > Thanks for your review. > I have tested so many times and I'm pretty sure that if user double > click on the blank area of the list, > we will always receive a CBN_CLICK message. So I have to > use LB_ITEMFROMPOINT to know > whether a use really clicked on a list item, Otherwise it's wacky that > user click on the blank area and > the edit window pops out. Sounds like a Listbox bug, I can confirm that ControlSpy on wine shows LBN_DBLCLK notification when double clicking on blank area. By the way you should use LBN_DBLCLK instead of CBN_DBLCLK.

June 30, 2015

Re: [PATCH]winecfg: Double click in dlls list to edit overides. (try4)

by Hao Peng

Hi, Nikolay Sivov Thanks for your review. I have tested so many times and I'm pretty sure that if user double click on the blank area of the list, we will always receive a CBN_CLICK message. So I have to use LB_ITEMFROMPOINT to know whether a use really clicked on a list item, Otherwise it's wacky that user click on the blank area and the edit window pops out. ------------------ Original ------------------ From: "Nikolay Sivov"<bunglehead(a)gmail.com>; Date: Mon, Jun 29, 2015 04:09 AM To: "wine-devel"<wine-devel(a)winehq.org>; Cc: "Hao Peng"<penghao(a)linuxdeepin.com>; Subject: Re: [PATCH]winecfg: Double click in dlls list to edit overides. (try4) On 29.06.2015 12:35, Hao Peng wrote: > Add a feature to winecfg. User can double click in dlls list to edit > overides of selected item. > > superseded all earlier patches. > > --- > programs/winecfg/libraries.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > + case CBN_DBLCLK: > + if (LOWORD(wParam) == IDC_DLLS_LIST) > + { > + int index; > + POINT p; > + if (GetCursorPos(&p) && ScreenToClient((HWND)lParam, &p)) > + { > + index = SendDlgItemMessageW(hDlg, IDC_DLLS_LIST, > + LB_ITEMFROMPOINT, 0, MAKELPARAM(p.x, p.y)); > + if (!HIWORD(index)) > + on_edit_click(hDlg); > + } > + } > + break; Correct me if I'm wrong, but I don't think you get CBN_DBLCLK unless clicked on an item. Quick test with ControlSpy on Windows confirms that. And by the time you get CBN_DBLCLK something is always selected I think, which makes all that cursor complexity redundant.

June 30, 2015

Re: winedbg: Add be_xpoint_FAIL=-1 to enum be_xpoint_type.

by Michael Stefaniuc

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Thanks Gerald, On 06/30/2015 01:12 AM, Gerald Pfeifer wrote: > This replaces my original submission with a different approach. > > Recall this is about clang 3.4.1 complaining as follows: > > gdbproxy.c:2147:23: warning: comparison of constant -1 with > expression of type 'enum be_xpoint_type' is always false > > In my original submission I noted that "I see two ways to address > this: adding a dummy set to -1 to the enum, or casting the enum to > int before comparing against -1". > > Michael kindly provided feeback to go with the former approach. > This patch does that. (Thanks, Michael!) but can you please change the magic -1 to be_xpoint_FAIL too? > > Gerald --- programs/winedbg/be_cpu.h | 2 +- 1 file changed, 1 > insertion(+), 1 deletion(-) > > diff --git a/programs/winedbg/be_cpu.h b/programs/winedbg/be_cpu.h > index 889f638..c95ed7c 100644 --- a/programs/winedbg/be_cpu.h +++ > b/programs/winedbg/be_cpu.h @@ -20,7 +20,7 @@ > > enum be_cpu_addr {be_cpu_addr_pc, be_cpu_addr_stack, > be_cpu_addr_frame}; enum be_xpoint_type {be_xpoint_break, > be_xpoint_watch_exec, be_xpoint_watch_read, - > be_xpoint_watch_write}; + > be_xpoint_watch_write, be_xpoint_FAIL=-1}; I'd rather have be_xpoint_FAIL as first element in the enum. > struct backend_cpu { const DWORD machine; bye michael -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iD8DBQFVklO10ei8kcpE1VERAnKJAKCDf9tLHh2u4/mHPqdwjXTB/ol8MACcDQOx 1UIk4kBN8htbqwT0Wi3z5bo= =M6XR -----END PGP SIGNATURE-----

June 30, 2015

Re: [4/14]hid: Implement HidP_GetCaps

by Alexandre Julliard

Aric Stewart <aric(a)codeweavers.com> writes: > On 6/26/15 7:31 AM, Michael Stefaniuc wrote: >> Aric, >> >> On 06/26/2015 02:22 PM, Aric Stewart wrote: >>> These internal structures are shared between hid.dll and the forthcoming hidclass.sys >>> >>> Should I define them internally at this point and have a patch later that moves them to the global header? >> my crystal ball tells me that the answer will be to just have that >> internal header and copy it to hidclass.sys >> E.g. I have 8 copies of dmobject.[ch] atm > > I bet you are right here. There is a hidparse.sys that implements all > of those HidP_ functions. But I am pretty sure that you are not > suppose to be able to link a .sys file to a .dll file. Maybe not on Windows, but on Wine you could certainly call hid.dll from hidclass.sys, if that helps. -- Alexandre Julliard julliard(a)winehq.org

June 30, 2015

Re: new patch

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 https://testbot.winehq.org/JobDetails.pl?Key=14843 Your paranoid android. === build (build) === Patch failed to apply

June 30, 2015

Re: [4/14]hid: Implement HidP_GetCaps

by Aric Stewart

On 6/26/15 7:31 AM, Michael Stefaniuc wrote: > Aric, > > On 06/26/2015 02:22 PM, Aric Stewart wrote: >> These internal structures are shared between hid.dll and the forthcoming hidclass.sys >> >> Should I define them internally at this point and have a patch later that moves them to the global header? > my crystal ball tells me that the answer will be to just have that > internal header and copy it to hidclass.sys > E.g. I have 8 copies of dmobject.[ch] atm I bet you are right here. There is a hidparse.sys that implements all of those HidP_ functions. But I am pretty sure that you are not suppose to be able to link a .sys file to a .dll file. So I bet the functions are reimplemented and so the header is in both places. -aric

June 29, 2015

Re: [3/4] shell32: Release shell folders obtained via IFolderView_GetFolder.

by Matteo Bruni

2015-06-29 23:10 GMT+02:00 Thomas Faber <thomas.faber(a)reactos.org>: > As a consequence of the previous patch, Wine now leaks shell folder > references after calling GetFolder. This fixes the call sites to > release them. I think it would be better to merge this with the previous patch, to avoid the temporary regression.

June 29, 2015

Re: [4/4] shell32/tests: Remove a misleading test.

by Nikolay Sivov

Whole series looks good to me, thanks for working on this. On 30.06.2015 0:57, Thomas Faber wrote: > This removes the aforementioned test. Yes, this test was my doing. I guess I misinterpreted results, and static desktop impl hid the issue. By the way, regarding GetFolder(), msdn says that IShellItemArray is also supported, we probably can easily get away be using SHCreateShellItemArray; but that is a separate thing of course, that deserves its own patch and tests.

June 29, 2015

Re: d3dx9_36 [try 2]: Save multiplications and divisions to improve performance

by Matteo Bruni

2015-06-25 23:45 GMT+02:00 David Adam <david.adam.cnrs(a)gmail.com>: > Hi, > > I added a test with 65000 sides. All the test pass with a native d3dx9_36 > dll. You should have mentioned that the new test doesn't pass with the previous implementation, which is what makes this patch really convincing. So this patch is okay to me. Also notice that this in practice doesn't save any multiplication in the loop, at least with my version of GCC: the multiplication 2.0f * D3DX_PI was precalculated at compile time. A better patch title mentioning D3DXCreatePolygon would be nice. The tests generally have some room for improvement. As a followup it would be nice to try and improve them, maybe by replacing the comparison function with one which allows for a relative error and makes it possible to choose a potentially different error value for each test (to be able to lower those as much as possible). I would like to point you to some other place in Wine where the float comparison is done right but, on the spot, I only found relative_error + expect_color macros from d3dx9_36/tests/math.c which are really ugly. FWIW https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbe… seems to be a good read on the subject.

June 29, 2015

Re: d3dcompiler: Add D3DCompileFromFile, D3DCompile2 stubs

by Matteo Bruni

2015-06-27 8:41 GMT+02:00 Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>: > Hi, > These functions are only supported with d3dcompiler version 46 and above. > > Changelog: > d3dcompiler: Add D3DCompileFromFile, D3DCompile2 stubs > > > Best Regards > Alistair Leslie-Hughes I don't think you need to encompass the function definitions in an #if D3D_COMPILER_VERSION since they won't be exported anyway by the spec files for version < 46. Can you use a format like the one used in D3DCompile() for the tracing of function parameters in the FIXME?

June 29, 2015

Re: ntdll: Randomize security cookie when available (try 4)

by André Hentschel

Am 29.06.2015 um 14:16 schrieb Alexandre Julliard: > André Hentschel <nerv(a)dawncrow.de> writes: > >> @@ -1278,6 +1281,26 @@ static NTSTATUS map_image( HANDLE hmapping, int fd, char *base, SIZE_T total_siz >> } >> } >> >> + >> + /* randomize security cookie */ >> + >> + if (IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG < nt->OptionalHeader.NumberOfRvaAndSizes && >> + (pos = nt->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG].VirtualAddress)) >> + { >> + IMAGE_LOAD_CONFIG_DIRECTORY *loadcfg = (IMAGE_LOAD_CONFIG_DIRECTORY *)(ptr + pos); >> + ULONG_PTR *cookie = (ULONG_PTR *)loadcfg->SecurityCookie; >> + struct timeval tv; >> + >> + gettimeofday( &tv, NULL ); >> + srand( tv.tv_usec * getpid() ); >> + *cookie = rand(); > > That's not much better. It doesn't make sense to reset the seed on every > call. > Where would you see the best point for srand()? __wine_process_init()? Should i do it with a "once" variable? Or would you prefer something like in SystemFunction036 using /dev/urandom (suggested by Bruno)?

June 29, 2015

Re: [PATCH 1/6]: msvcp110: Add tr2_sys__Copy_file implementation and test.

by YongHaoHu

On 15/6/29 下午10:51, Piotr Caban wrote: > On 06/29/15 16:40, YongHaoHu wrote: >> On 15/6/29 下午5:29, Piotr Caban wrote: >>> On 06/27/15 19:49, YongHaoHu wrote: >>>> + ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, >>>> tests[i].fail_if_exists); >>>> + ok(ret == GetLastError(), "test_tr2_sys__Copy_file(): test >>>> %d expect: %d, got %d\n", i+1, GetLastError(), ret); >>>> + ok(errno == 0xdeadbeef, "test_tr2_sys__Copy_file(): test %d >>>> errno expect 0xdeadbeef, got %d\n", i+1, errno); >>> Please check the return value of the function explicitly. Currently >>> your implementation returns error while it should succeed and it's not >>> visible in the tests. >> Because the last error handling is not correct in CopyFile[1], I check >> GetLastError() instead of values. Do you have any good idea? > If testing exact return value of function is hard please at least test > if function succeeds/fails when it should. Sorry, could you explain a bit more details? > Currently your implementation sometimes returns error even so it > succeeds. > Which one do you mean ? As you know, last_error hadling is not correct in wine's CopyFile, was this happened in Wine platform? Thank you very much. : P

June 29, 2015

Re: [PATCH 4/6]: msvcp110: Add tr2_sys__Stat implementation and test.

by YongHaoHu

On 15/6/29 下午10:54, Piotr Caban wrote: > On 06/29/15 16:42, YongHaoHu wrote: >>> Why are you handling ERROR_BAD_DEV_TYPE error here? >>> >> I don't have special reason, just base on the test result on testbot. > It looks wrong. Could you please send the test here and point on what > testbot machine it happens? > Yes, it's wrong. According to your advice, I tested GetFileAttributesA and found that its last error handling was also not correct. (It means that many ntdll functions have problems with last_error handling.) The code I used is : GetFileAttributesA("\\\\.\\PiPe\\tests_pipe.c"); printf("getlast err is %d\n", GetLastError()); The output in wine is "getlast err is 66" while "|getlast err is 0" in testbot. Here is the test result: https://testbot.winehq.org/JobDetails.pl?Key=14832&log_101=1#k101 Plesase ignore the failed msg. |

June 29, 2015

Re: d3drm/tests: Add test for IDirect3DRM*::CreateDeviceFromD3D (try 9 resend).

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 https://testbot.winehq.org/JobDetails.pl?Key=14829 Your paranoid android. === build (build) === Patch failed to apply

June 29, 2015

Re: d3drm/tests: Add test for IDirect3DRM*::CreateDeviceFromSurface (try 7 resend).

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 https://testbot.winehq.org/JobDetails.pl?Key=14828 Your paranoid android. === build (build) === Patch failed to apply

June 29, 2015

Re: [PATCH 4/6]: msvcp110: Add tr2_sys__Stat implementation and test.

by Piotr Caban

On 06/29/15 16:42, YongHaoHu wrote: >> Why are you handling ERROR_BAD_DEV_TYPE error here? >> > I don't have special reason, just base on the test result on testbot. It looks wrong. Could you please send the test here and point on what testbot machine it happens? Thanks, Piotr

June 29, 2015

Re: [PATCH 1/6]: msvcp110: Add tr2_sys__Copy_file implementation and test.

by Piotr Caban

On 06/29/15 16:40, YongHaoHu wrote: > On 15/6/29 下午5:29, Piotr Caban wrote: >> On 06/27/15 19:49, YongHaoHu wrote: >>> + ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, >>> tests[i].fail_if_exists); >>> + ok(ret == GetLastError(), "test_tr2_sys__Copy_file(): test >>> %d expect: %d, got %d\n", i+1, GetLastError(), ret); >>> + ok(errno == 0xdeadbeef, "test_tr2_sys__Copy_file(): test %d >>> errno expect 0xdeadbeef, got %d\n", i+1, errno); >> Please check the return value of the function explicitly. Currently >> your implementation returns error while it should succeed and it's not >> visible in the tests. > Because the last error handling is not correct in CopyFile[1], I check > GetLastError() instead of values. Do you have any good idea? If testing exact return value of function is hard please at least test if function succeeds/fails when it should. Currently your implementation sometimes returns error even so it succeeds. Thanks, Piotr

June 29, 2015

Re: [PATCH 4/6]: msvcp110: Add tr2_sys__Stat implementation and test.

by YongHaoHu

On 15/6/29 下午5:55, Piotr Caban wrote: > On 06/27/15 19:50, YongHaoHu wrote: >> +static file_type stat_set_error(int *err_code) >> +{ >> + file_type ret; >> + *err_code = ERROR_SUCCESS; >> + >> + switch(GetLastError()) { >> + case ERROR_FILE_NOT_FOUND: >> + case ERROR_BAD_NETPATH: >> + case ERROR_INVALID_NAME: >> + case ERROR_BAD_PATHNAME: >> + case ERROR_PATH_NOT_FOUND: >> + ret = file_not_found; >> + break; >> + case ERROR_BAD_DEV_TYPE: >> + ret = regular_file; >> + break; >> + default: >> + ret = status_unknown; > Shouldn't err_code be set to GetLastError in default case? You end up > setting it to ERROR_SUCCESS in every case except if path is NULL. > Yeah, you are right. > Why are you handling ERROR_BAD_DEV_TYPE error here? > I don't have special reason, just base on the test result on testbot. >> + TRACE("(%s %p)\n", debugstr_a(path), err_code); >> + DWORD attr; > This will produce a compilation warning. > Got it. Thank you for your comment. :)

June 29, 2015

Re: [PATCH 1/6]: msvcp110: Add tr2_sys__Copy_file implementation and test.

by YongHaoHu

Hi, Piotr. On 15/6/29 下午5:29, Piotr Caban wrote: > On 06/27/15 19:49, YongHaoHu wrote: >> + ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, >> tests[i].fail_if_exists); >> + ok(ret == GetLastError(), "test_tr2_sys__Copy_file(): test >> %d expect: %d, got %d\n", i+1, GetLastError(), ret); >> + ok(errno == 0xdeadbeef, "test_tr2_sys__Copy_file(): test %d >> errno expect 0xdeadbeef, got %d\n", i+1, errno); > Please check the return value of the function explicitly. Currently > your implementation returns error while it should succeed and it's not > visible in the tests. Because the last error handling is not correct in CopyFile[1], I check GetLastError() instead of values. Do you have any good idea? [1]: https://bugs.winehq.org/show_bug.cgi?id=38772

June 29, 2015

Re: d3drm/tests: Add test for IDirect3DRM*::CreateDeviceFromD3D (try 9).

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 https://testbot.winehq.org/JobDetails.pl?Key=14826 Your paranoid android. === build (build) === Patch failed to apply

June 29, 2015

Re: Wiki is down

by Jeremy Newman

I've updated the DNS. Might take a bit for your respective caches to update Also, you can email me directly for these sorts of changes. Or, you know, just wait for me to discover the site is down on my own. Your choice. :-p -N On 06/28/2015 09:10 AM, Dimi Paun wrote: > Yes, the DNS entry needs to change. > > wiki.winehq.org <http://wiki.winehq.org> points to www.lattica. com > > we need to change it to point to: > > winewiki.lattica.com <http://winewiki.lattica.com>. > > Thanks, > > > On Sat, Jun 27, 2015 at 9:01 AM Austin English <austinenglish(a)gmail.com > <mailto:austinenglish(a)gmail.com>> wrote: > > On Jun 27, 2015 3:33 PM, "Rosanne DiMesio" <dimesio(a)earthlink.net > <mailto:dimesio(a)earthlink.net>> wrote: > > > > Http://wiki.winehq.org leads to a bluehost.com > <http://bluehost.com> page that says "There is no website configured > at this address" and links to specific wiki pages produce 404: File > not found. > > > > -- > > Rosanne DiMesio <dimesio(a)earthlink.net > <mailto:dimesio(a)earthlink.net>> > > +Dimi > > > >

June 29, 2015

Re: [PATCH 3/3] d3drm/tests: Add test for IDirect3DRM*::CreateDeviceFromD3D (try 8).

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-06-28 um 11:14 schrieb Aaryaman Vasishta: > + if (FAILED(IDirectDraw2_CreateSurface(ddraw, &surface_desc, ds, NULL))) > + continue; > + > + hr = IDirectDrawSurface_AddAttachedSurface(surface, *ds); > + ok(SUCCEEDED(hr), "Failed to attach depth buffer, hr %#x.\n", hr); > + if (FAILED(hr)) > + continue; You're leaking the depth surface in this case. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVkUYtAAoJEN0/YqbEcdMwCt4P/09yNvjELjIyH5T/5HRdUWm4 LoJLtK/OhmlwdaHCvsShqImde4IjszHm/wzL7sIokNxXJ9zDOvmFPjQ+8jcyBJoG ZPNGB7o2QT1RqVhgXRBJkAGWkrG7Pwd6oTShibKmbxySsTVuol62hcjqSZkZshCz 6iO/9MwhzW1KdpHrVGbXcKAQZzPg8rsqDb+97k5350FYdJFFd0935WOCUiNfHOuu iqAm6V3sJbcQ3SBxJN5l2vPLG2m4j40307fEFNJor9jOkFOGv9RnxzGCp2O+gUcV h6n6l/bmznT/lVl4IedUFYkxIQUgdGIMoUXZfgs5MZZYhuaUrR9XxeNlv7oJrcVQ i1fp3pjEQldw7X4KQXqlwzOn7CvVZTtSqH4D4Glyn/3rHFD0V9fIWINg6Fj/Nd73 fZf8TAHM45WqQ+9ueVmnaYGdkUhN0bq8JDE2QfCKUfXNIOC8j5vLgJ7NC5YhC7xc WtOAsXxF7RRVmbQbfMUYOWJcMWR9XUbTNGyWQhWsiWx92rz3n1m7C3e+K9fp2F2P Eybs30FWbPuKuBxvEy0Ws1bA1xY+my1iD/mQN4YEc1aet2X/YpZ4EqYSWOzwKLM/ FuAchmQFwo7Hyr+9nrAc47441ENMBdQQ/dpAS90wGy/uQPJURj08LFt3EWDjXy/O bkMhBscSpcPpizrksvna =U/er -----END PGP SIGNATURE-----

June 29, 2015

Re: vbscript: Treat empty regexp pattern the same way as NULL pattern.

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 https://testbot.winehq.org/JobDetails.pl?Key=14818 Your paranoid android. === w2000pro (32 bit) === No test summary line found === wxppro (32 bit) === No test summary line found === w2003std (32 bit) === No test summary line found === wvistau64 (32 bit) === No test summary line found === w2008s64 (32 bit) === No test summary line found === w7u (32 bit) === No test summary line found === w8 (32 bit) === No test summary line found === wvistau64 (64 bit) === No test summary line found === w2008s64 (64 bit) === No test summary line found

June 29, 2015

Re: ntdll: Randomize security cookie when available (try 4)

by Alexandre Julliard

André Hentschel <nerv(a)dawncrow.de> writes: > @@ -1278,6 +1281,26 @@ static NTSTATUS map_image( HANDLE hmapping, int fd, char *base, SIZE_T total_siz > } > } > > + > + /* randomize security cookie */ > + > + if (IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG < nt->OptionalHeader.NumberOfRvaAndSizes && > + (pos = nt->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG].VirtualAddress)) > + { > + IMAGE_LOAD_CONFIG_DIRECTORY *loadcfg = (IMAGE_LOAD_CONFIG_DIRECTORY *)(ptr + pos); > + ULONG_PTR *cookie = (ULONG_PTR *)loadcfg->SecurityCookie; > + struct timeval tv; > + > + gettimeofday( &tv, NULL ); > + srand( tv.tv_usec * getpid() ); > + *cookie = rand(); That's not much better. It doesn't make sense to reset the seed on every call. -- Alexandre Julliard julliard(a)winehq.org

June 29, 2015

Re: [PATCH]winecfg: Double click in dlls list to edit overides. (try 4)

by Nikolay Sivov

On 29.06.2015 12:35, Hao Peng wrote: > Add a feature to winecfg. User can double click in dlls list to edit > overides of selected item. > > superseded all earlier patches. > > --- > programs/winecfg/libraries.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > + case CBN_DBLCLK: > + if (LOWORD(wParam) == IDC_DLLS_LIST) > + { > + int index; > + POINT p; > + if (GetCursorPos(&p) && ScreenToClient((HWND)lParam, &p)) > + { > + index = SendDlgItemMessageW(hDlg, IDC_DLLS_LIST, > + LB_ITEMFROMPOINT, 0, MAKELPARAM(p.x, p.y)); > + if (!HIWORD(index)) > + on_edit_click(hDlg); > + } > + } > + break; Correct me if I'm wrong, but I don't think you get CBN_DBLCLK unless clicked on an item. Quick test with ControlSpy on Windows confirms that. And by the time you get CBN_DBLCLK something is always selected I think, which makes all that cursor complexity redundant.

June 29, 2015

Re: [1/2] user32/tests: Add some tests to show that WS_CLIPCHILDREN doesn't exclude children from update region.

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 https://testbot.winehq.org/JobDetails.pl?Key=14817 Your paranoid android. === wvistau64 (64 bit msg) === msg.c:10561: Test failed: expected PeekMessage to return FALSE, got 1

June 29, 2015

Re: [1/2] qcap: Distinguish interface and implementation pointer for VfwCapture output pin.

by Sebastian Lackner

On 29.06.2015 12:50, Nikolay Sivov wrote: > On 29.06.2015 13:13, Sebastian Lackner wrote: >> Thanks for the feedback. In this case pOutputPin is basically part of the same object, >> not an arbitrary external interface. It is constructed with VfwPin_Construct -> >> BaseOutputPin_Construct, and also holds a reference to the critical section of the parent >> object for example, so there is no clear distinction between public/private interfaces >> anyway. >> >> Most other code parts which use BaseOutputPin_Construct store the result in something like: >> (IPin **)&This->store_impl_pointer_here where the variable already has the corresponding >> implementation type. Should we change it here too? Using only the public interface is not possible, >> take a look at for example the following code part, where a private member is set from a >> different interface: >> >> --- snip --- >> This->driver_info = qcap_driver_init( This->pOutputPin, >> var.__VARIANT_NAME_1.__VARIANT_NAME_2.__VARIANT_NAME_3.ulVal ); >> if (This->driver_info) >> { >> pin = impl_from_IPin(This->pOutputPin); >> pin->driver_info = This->driver_info; // <--- >> --- snip --- > > I personally prefer to use public interface, usually it makes things more clear, and code doesn't depend on implementation changes. > > If in this particular case it's not possible then yes, we should store impl pointer instead of interface pointer. Okay, I'll resend an updated version later then. > > By the way, variant thing looks a little insane, assuming intention was to get V_UI4(&var), which is not necessary right on its own as qcap_driver_init() takes USHORT argument. > Yes, I also noticed it. Changes to this part should probably be in a separate patch though.

June 29, 2015

Re: [1/2] qcap: Distinguish interface and implementation pointer for VfwCapture output pin.

by Nikolay Sivov

On 29.06.2015 13:13, Sebastian Lackner wrote: > On 29.06.2015 09:09, Nikolay Sivov wrote: >> On 29.06.2015 9:08, Sebastian Lackner wrote: >>> - BasePin *pin; >>> + VfwPinImpl *pin; >>> >>> TRACE("destroying everything\n"); >>> if (This->init) >>> @@ -226,10 +236,10 @@ static ULONG WINAPI VfwCapture_Release(IBaseFilter * iface) >>> qcap_driver_stop(This->driver_info, &This->filter.state); >>> qcap_driver_destroy(This->driver_info); >>> } >>> - pin = (BasePin*) This->pOutputPin; >>> - if (pin->pConnectedTo != NULL) >>> + pin = impl_from_IPin(This->pOutputPin); >>> + if (pin->pin.pin.pConnectedTo != NULL) >>> { >>> - IPin_Disconnect(pin->pConnectedTo); >>> + IPin_Disconnect(pin->pin.pin.pConnectedTo); >> >> This is not better than it is now, 'pOutputPin' is IPin* so you should be using public methods. > > Thanks for the feedback. In this case pOutputPin is basically part of the same object, > not an arbitrary external interface. It is constructed with VfwPin_Construct -> > BaseOutputPin_Construct, and also holds a reference to the critical section of the parent > object for example, so there is no clear distinction between public/private interfaces > anyway. > > Most other code parts which use BaseOutputPin_Construct store the result in something like: > (IPin **)&This->store_impl_pointer_here where the variable already has the corresponding > implementation type. Should we change it here too? Using only the public interface is not possible, > take a look at for example the following code part, where a private member is set from a > different interface: > > --- snip --- > This->driver_info = qcap_driver_init( This->pOutputPin, > var.__VARIANT_NAME_1.__VARIANT_NAME_2.__VARIANT_NAME_3.ulVal ); > if (This->driver_info) > { > pin = impl_from_IPin(This->pOutputPin); > pin->driver_info = This->driver_info; // <--- > --- snip --- I personally prefer to use public interface, usually it makes things more clear, and code doesn't depend on implementation changes. If in this particular case it's not possible then yes, we should store impl pointer instead of interface pointer. By the way, variant thing looks a little insane, assuming intention was to get V_UI4(&var), which is not necessary right on its own as qcap_driver_init() takes USHORT argument. > > >> >>> if (IsEqualIID(riid, &IID_IUnknown) || IsEqualIID(riid, &IID_IPin)) >>> - *ppv = This; >>> + *ppv = &This->pin.pin.IPin_iface; >> >> This could as well say 'iface'. >> >> > > Is there really a clear preference? I personally always preferred the long form so far, > to make it a bit more clear when many different interfaces are returned. > No, I don't think there is. In this case it would be shorter, but only because of strmbase nesting level. Both ways are better than 'This', so it's up to author I suppose.

June 29, 2015

Re: [1/2] qcap: Distinguish interface and implementation pointer for VfwCapture output pin.

by Sebastian Lackner

On 29.06.2015 09:09, Nikolay Sivov wrote: > On 29.06.2015 9:08, Sebastian Lackner wrote: >> - BasePin *pin; >> + VfwPinImpl *pin; >> >> TRACE("destroying everything\n"); >> if (This->init) >> @@ -226,10 +236,10 @@ static ULONG WINAPI VfwCapture_Release(IBaseFilter * iface) >> qcap_driver_stop(This->driver_info, &This->filter.state); >> qcap_driver_destroy(This->driver_info); >> } >> - pin = (BasePin*) This->pOutputPin; >> - if (pin->pConnectedTo != NULL) >> + pin = impl_from_IPin(This->pOutputPin); >> + if (pin->pin.pin.pConnectedTo != NULL) >> { >> - IPin_Disconnect(pin->pConnectedTo); >> + IPin_Disconnect(pin->pin.pin.pConnectedTo); > > This is not better than it is now, 'pOutputPin' is IPin* so you should be using public methods. Thanks for the feedback. In this case pOutputPin is basically part of the same object, not an arbitrary external interface. It is constructed with VfwPin_Construct -> BaseOutputPin_Construct, and also holds a reference to the critical section of the parent object for example, so there is no clear distinction between public/private interfaces anyway. Most other code parts which use BaseOutputPin_Construct store the result in something like: (IPin **)&This->store_impl_pointer_here where the variable already has the corresponding implementation type. Should we change it here too? Using only the public interface is not possible, take a look at for example the following code part, where a private member is set from a different interface: --- snip --- This->driver_info = qcap_driver_init( This->pOutputPin, var.__VARIANT_NAME_1.__VARIANT_NAME_2.__VARIANT_NAME_3.ulVal ); if (This->driver_info) { pin = impl_from_IPin(This->pOutputPin); pin->driver_info = This->driver_info; // <--- --- snip --- > >> if (IsEqualIID(riid, &IID_IUnknown) || IsEqualIID(riid, &IID_IPin)) >> - *ppv = This; >> + *ppv = &This->pin.pin.IPin_iface; > > This could as well say 'iface'. > > Is there really a clear preference? I personally always preferred the long form so far, to make it a bit more clear when many different interfaces are returned.

June 29, 2015

Re: [PATCH 4/6]: msvcp110: Add tr2_sys__Stat implementation and test.

by Piotr Caban

On 06/27/15 19:50, YongHaoHu wrote: > +static file_type stat_set_error(int *err_code) > +{ > + file_type ret; > + *err_code = ERROR_SUCCESS; > + > + switch(GetLastError()) { > + case ERROR_FILE_NOT_FOUND: > + case ERROR_BAD_NETPATH: > + case ERROR_INVALID_NAME: > + case ERROR_BAD_PATHNAME: > + case ERROR_PATH_NOT_FOUND: > + ret = file_not_found; > + break; > + case ERROR_BAD_DEV_TYPE: > + ret = regular_file; > + break; > + default: > + ret = status_unknown; Shouldn't err_code be set to GetLastError in default case? You end up setting it to ERROR_SUCCESS in every case except if path is NULL. Why are you handling ERROR_BAD_DEV_TYPE error here? > + TRACE("(%s %p)\n", debugstr_a(path), err_code); > + DWORD attr; This will produce a compilation warning. Cheers, Piotr

June 29, 2015

Re: [PATCH 2/6]: msvcp110: Add tr2_sys__Rename implementation and test.

by Piotr Caban

On 06/27/15 19:50, YongHaoHu wrote: > + ok(GetFileInformationByHandle(h2, &info2), "GetFileInformationByHandle failed\n"); > + CloseHandle(h2); > + ok(info1.nFileIndexHigh == info2.nFileIndexHigh > + && info1.nFileIndexLow == info2.nFileIndexLow, > + "test_tr2_sys__Rename(): test %d expect two files equivalent\n", i+1); This test is not working when run from VirtualBox share. Could you please run the tests inside temp directroy? Thanks, Piotr

June 29, 2015

Re: [PATCH 1/6]: msvcp110: Add tr2_sys__Copy_file implementation and test.

by Piotr Caban

On 06/27/15 19:49, YongHaoHu wrote: > + ret = p_tr2_sys__Copy_file(tests[i].source, tests[i].dest, tests[i].fail_if_exists); > + ok(ret == GetLastError(), "test_tr2_sys__Copy_file(): test %d expect: %d, got %d\n", i+1, GetLastError(), ret); > + ok(errno == 0xdeadbeef, "test_tr2_sys__Copy_file(): test %d errno expect 0xdeadbeef, got %d\n", i+1, errno); Please check the return value of the function explicitly. Currently your implementation returns error while it should succeed and it's not visible in the tests. Thanks, Piotr

June 29, 2015

Re: [1/2] qcap: Distinguish interface and implementation pointer for VfwCapture output pin.

by Michael Stefaniuc

On 06/29/2015 08:08 AM, Sebastian Lackner wrote: > --- dlls/qcap/vfwcapture.c | 42 > ++++++++++++++++++++++++++---------------- 1 file changed, 26 > insertions(+), 16 deletions(-) > > diff --git a/dlls/qcap/vfwcapture.c b/dlls/qcap/vfwcapture.c index > 7804a3d..31620c0 100644 --- a/dlls/qcap/vfwcapture.c +++ > b/dlls/qcap/vfwcapture.c @@ -557,7 +567,7 @@ PPB_Load( > IPersistPropertyBag * iface, IPropertyBag *pPropBag, > var.__VARIANT_NAME_1.__VARIANT_NAME_2.__VARIANT_NAME_3.ulVal ); if > (This->driver_info) { - pin = (VfwPinImpl > *)This->pOutputPin; + pin = > impl_from_IPin(This->pOutputPin); impl_from_IFoo() is just for the interface pointer of a method. For internal stuff the object pointer should be passed or stored instead. For application provided interface pointers an unsafe_impl_from_IFoo() should be used. bye michael

June 29, 2015

Re: qmgr: Fix WINHTTP_OPTION_CONTEXT_VALUE data pointer (Coverity)

by Hans Leidekker

> On 27 Jun 2015, at 09:57, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Hans, does it look right? Yes, thanks.

June 29, 2015

Re: [1/2] qcap: Distinguish interface and implementation pointer for VfwCapture output pin.

by Nikolay Sivov

On 29.06.2015 9:08, Sebastian Lackner wrote: > - BasePin *pin; > + VfwPinImpl *pin; > > TRACE("destroying everything\n"); > if (This->init) > @@ -226,10 +236,10 @@ static ULONG WINAPI VfwCapture_Release(IBaseFilter * iface) > qcap_driver_stop(This->driver_info, &This->filter.state); > qcap_driver_destroy(This->driver_info); > } > - pin = (BasePin*) This->pOutputPin; > - if (pin->pConnectedTo != NULL) > + pin = impl_from_IPin(This->pOutputPin); > + if (pin->pin.pin.pConnectedTo != NULL) > { > - IPin_Disconnect(pin->pConnectedTo); > + IPin_Disconnect(pin->pin.pin.pConnectedTo); This is not better than it is now, 'pOutputPin' is IPin* so you should be using public methods. > if (IsEqualIID(riid, &IID_IUnknown) || IsEqualIID(riid, &IID_IPin)) > - *ppv = This; > + *ppv = &This->pin.pin.IPin_iface; This could as well say 'iface'.

June 29, 2015

Re: [2/2] windowscodecs: Distinguish interface and implementation pointer for IcnsEncoder.

by Nikolay Sivov

On 29.06.2015 9:08, Sebastian Lackner wrote: > --- > dlls/windowscodecs/icnsformat.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/dlls/windowscodecs/icnsformat.c b/dlls/windowscodecs/icnsformat.c > index 5ce44a2..3f24e91 100644 > --- a/dlls/windowscodecs/icnsformat.c > +++ b/dlls/windowscodecs/icnsformat.c > @@ -179,7 +179,7 @@ static ULONG WINAPI IcnsFrameEncode_Release(IWICBitmapFrameEncode *iface) > if (This->icns_image != NULL) > HeapFree(GetProcessHeap(), 0, This->icns_image); > > - IUnknown_Release((IUnknown*)This->encoder); > + IUnknown_Release(&This->encoder->IWICBitmapFrameEncode_iface); > HeapFree(GetProcessHeap(), 0, This); > } > > @@ -489,7 +489,7 @@ static HRESULT WINAPI IcnsEncoder_QueryInterface(IWICBitmapEncoder *iface, REFII > if (IsEqualIID(&IID_IUnknown, iid) || > IsEqualIID(&IID_IWICBitmapEncoder, iid)) > { > - *ppv = This; > + *ppv = &This->IWICBitmapEncoder_iface; > } > else > { > @@ -639,7 +639,7 @@ static HRESULT WINAPI IcnsEncoder_CreateNewFrame(IWICBitmapEncoder *iface, > frameEncode->committed = FALSE; > *ppIFrameEncode = &frameEncode->IWICBitmapFrameEncode_iface; > This->outstanding_commits++; > - IUnknown_AddRef((IUnknown*)This); > + IUnknown_AddRef(&This->IWICBitmapEncoder_iface); > > end: > LeaveCriticalSection(&This->lock); > @@ -725,8 +725,8 @@ HRESULT IcnsEncoder_CreateInstance(REFIID iid, void** ppv) > InitializeCriticalSection(&This->lock); > This->lock.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": IcnsEncoder.lock"); > > - ret = IUnknown_QueryInterface((IUnknown*)This, iid, ppv); > - IUnknown_Release((IUnknown*)This); > + ret = IUnknown_QueryInterface(&This->IWICBitmapEncoder_iface, iid, ppv); > + IUnknown_Release(&This->IWICBitmapEncoder_iface); > > return ret; > } > Please use proper wrappers for that.

June 29, 2015

Re: Wiki is down

by Dimi Paun

Yes, the DNS entry needs to change. wiki.winehq.org points to www.lattica. com we need to change it to point to: winewiki.lattica.com. Thanks, On Sat, Jun 27, 2015 at 9:01 AM Austin English <austinenglish(a)gmail.com> wrote: > On Jun 27, 2015 3:33 PM, "Rosanne DiMesio" <dimesio(a)earthlink.net> wrote: > > > > Http://wiki.winehq.org leads to a bluehost.com page that says "There is > no website configured at this address" and links to specific wiki pages > produce 404: File not found. > > > > -- > > Rosanne DiMesio <dimesio(a)earthlink.net> > > +Dimi >

June 28, 2015

Re: [PATCH 1/3] d3drm/tests: Add test for IDirect3DRM*::CreateDeviceFromClipper (unmodified resend).

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, Here's something for future reference. I believe Henri mentioned the same thing for a different place already. I don't think a resend is necessary for this, you can fix this when you add this test for d3drm1 and d3drm3. Am 2015-06-27 um 18:38 schrieb Aaryaman Vasishta: > + hr = IDirectDrawSurface_GetClipper(d3drm_primary, &d3drm_clipper); > + ok(hr == DD_OK, "Cannot get attached clipper from primary surface (hr = %x).\n", hr); > + if (SUCCEEDED(hr)) > + { > + ok(d3drm_clipper == clipper, "Expected clipper returned == %p, got %p.\n", clipper , d3drm_clipper); > + IDirectDrawClipper_Release(d3drm_clipper); > + } You don't really need the if (SUCCEEDED(hr)) here. You're unconditionally expecting GetClipper to return a clipper in the ok() line above. If it doesn't return one the test already failed, and there's no need to bother about preventing follow-up failures or crashes. (Yeah, we have many instances in the code of things like these, e.g. if (SUCCEEDED(hr)) after BeginScene / EndScene. They should ideally be fixe d.) > + if (d3drm_primary) > + IDirectDrawSurface_Release(d3drm_primary); Same thing here. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVj5yEAAoJEN0/YqbEcdMwi+oP/0uFOap9QRVWMILPN/PrupVO g3IR6KBx+QNf/mzkOEyP6rQVbZx0UUlHlQ16v0ftziediL73qpQfRKSIxf135THH ji6+DK0xHUvBNSz6XDSZjDky/cTtnTO0IRdRqZlL4Cd+y46GF04hwoiuIijnx1Fd +AHurqTvEBgYepc+oEPgQRh2q/OApxUs3kMTPXzK9rPIp2SKZwpcJ8NMkYhaGMRC FXEOq/lfaanQbgQdcPqavMgHBDCOR9xvhgSsPktUlCtltsrNJ/1INyRHHfbt8F1k Yg5yfXdxvaThkWy5pNlAeatPZFhYzJIiyECvol7MXymHSP4zMDlF+WoG9V21fHOR aYCA5lplQY3P7+0r0Nv7rQgtqWoo/mx6ntgm3Etl7MBKV5+ED8RNVPCWCkBV4j/y EXWJe8RLj+a0pHIA+sTop0Yym+F00/6SPH6sJcHnqqSo1W34FFnpzH5ddV3oNst0 8fVjYNLyQwdo9k2z5DVf1ZJQk87dk3Rg6bzZpofwhOd3NEmNr1f8kMDLCYNbs5GK jr1QdyLtFCpxoqqDu1iE2174SZgECXkDU6DJQE99BVtNs9QcIx9pqNXmHbLXXDyX akHSKWeDQrZpMOSZI/XZxlXhhVdxeGRJGdkZmzr4H1Qpu4kHA5OgOaXBGVRIoUNg ii/ChTzxObM40BdpuaH+ =5Y9W -----END PGP SIGNATURE-----

June 28, 2015

Re: [PATCH 3/6]: msvcp110: Add tr2_sys__Statvfs implementation and test.

by YongHaoHu

Hi, On 15/6/28 上午1:59, Nikolay Sivov wrote: > On 27.06.2015 20:50, YongHaoHu wrote: > >> +typedef struct { >> + ULONGLONG capacity; >> + ULONGLONG free; >> + ULONGLONG available; >> +} space_info; >> + > > Is this documented somewhere? Yeah, it was documented in MSDN[1]. However, the order of the structure is not correct in the document according to my test. What's more, there is a document about the structure in cppreference.com[2], whose order is right. > >> + info.capacity = ((ULONGLONG)(total.HighPart)<< 32) + >> total.LowPart; >> + info.free = ((ULONGLONG)(free.HighPart)<< 32) + free.LowPart; >> + info.available = ((ULONGLONG)(available.HighPart)<< 32) + >> available.LowPart; > > I don't think you'll need this if you used QuadPart. Yes, thank you for your comment. :) [1]: https://msdn.microsoft.com/en-us//library/hh874909.aspx [2]: http://en.cppreference.com/w/cpp/experimental/fs/space_info

June 28, 2015

Re: [PATCH 3/3] d3drm/tests: Add test for IDirect3DRM*::CreateDeviceFromD3D (try 7).

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-06-27 um 18:38 schrieb Aaryaman Vasishta: > +static IDirect3DDevice2 *create_device(IDirectDraw2 *ddraw, HWND window, IDirectDrawSurface **ds) > ... > + if (FAILED(IDirectDraw2_CreateSurface(ddraw, &surface_desc, ds, NULL))) > + continue; > + > + hr = IDirectDrawSurface_AddAttachedSurface(surface, *ds); > + ok(SUCCEEDED(hr), "Failed to attach depth buffer, hr %#x.\n", hr); > + IDirectDrawSurface_Release(*ds); should release the depth stencil surface in the success case because you're passing it out of the function. The other patches are OK. I'll ack them on monday on IRC, including a fixed version of patch 3 if it is on wine-patches by then. Cheers, Stefan -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVjwwxAAoJEN0/YqbEcdMwz+sP/1+Lme2ItbNjmvKnCwtSWdul tcMr9DkNrjkyUMsM1Glzgoll5XphGRj+4M6ZnPTrLU5i4qlZ8w6MWm8kkrQK3h12 KWhfYEhWBahqBTDva7dulThM6TvzHrvq7pK8ObfmDdWlchhv4Y89hksSIlIdGUD8 7QuEx7EvcutcEnaNFZI5yCtOGEvUNAK4GYzFPWYNnFjDpgq7567TBh5oXdFkyQsT 3wKOf8Uxt5O9/2f5E+7cYyOLC+BR6YM4uGarnma2eqQIzTUOXySm7UYA0gJ3KRw6 9OtzWUyIPw5Iz2CkVkP5M5o9srtkSVungsCg6aQSQIcbGaedvhu9K0G4fFJxVqWJ zmntVVp4yQzP+yov9NHEYOvX1rAkO9V3RudeVfe2sytxHNYYC7hwTR8lXLaM0WF6 fbWx32k8+6o0JP02PgkZFHBhlYZLN5a1deYFTpUFsW0YhUKNrRXAwV9MuVMihreC MmNbnE7DnnrX/vYkxh6MHAnkCFM6/uMy9M8jvTEuzpr0mhCfjn50mbv/lgUK3WiV leUda/RCBmfiM4YPJkhV3qytikRL5EPouB5LcvCR7xjpKT83TBnKOphCRgLPVBrR 2uuJ5orV8M+V9g67zXYibkaeh3yBr8w8vuvTz/L638zkdjshJJmQ/L/51FL0t9Qk 1FEipapwnAjq4NOKJvJi =+VK0 -----END PGP SIGNATURE-----

June 27, 2015

Re: [PATCH 3/6]: msvcp110: Add tr2_sys__Statvfs implementation and test.

by Nikolay Sivov

On 27.06.2015 20:50, YongHaoHu wrote: > +typedef struct { > + ULONGLONG capacity; > + ULONGLONG free; > + ULONGLONG available; > +} space_info; > + Is this documented somewhere? > + info.capacity = ((ULONGLONG)(total.HighPart)<< 32) + total.LowPart; > + info.free = ((ULONGLONG)(free.HighPart)<< 32) + free.LowPart; > + info.available = ((ULONGLONG)(available.HighPart)<< 32) + available.LowPart; I don't think you'll need this if you used QuadPart.

June 27, 2015