August 2017 - Wine-devel - list.winehq.org

Re: [PATCH resend 1/2] krnl386.exe16: Use toupper() instead of RtlUpperChar().
by Alexandre Julliard 12 Aug '17

12 Aug '17

Zebediah Figura <z.figura12(a)gmail.com> writes: > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/krnl386.exe16/ne_module.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) I don't think that's a good idea. Why do you need that? -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH v3 2/5] ieproxy: Add stub DLL.
by Jacek Caban 11 Aug '17

11 Aug '17

On 10.08.2017 20:52, Zebediah Figura wrote: > --- a/loader/wine.inf.in > +++ b/loader/wine.inf.in > @@ -2559,6 +2559,7 @@ HKLM,%CurrentVersion%\Telephony\Country List\998,"SameAreaRule",,"G" > 11,,msxml6.dll > 11,,shdocvw.dll > 16422,Internet Explorer,iexplore.exe > +16422,Internet Explorer,ieproxy.dll > 16422,Windows Media Player,wmplayer.exe > 16422,Windows NT\Accessories,wordpad.exe > 11,,* > @@ -2613,6 +2614,7 @@ HKLM,%CurrentVersion%\Telephony\Country List\998,"SameAreaRule",,"G" > 16410,Microsoft, > 16412,Microsoft, > 16422,Internet Explorer,iexplore.exe > +16422,Internet Explorer,ieproxy.dll Are you sure about it? It's in regular system32 (and syswow64) on my Win10. Jacek

2 2

Re: [PATCH v5 1/5] include: Add initial ieautomation.idl
by Jacek Caban 11 Aug '17

11 Aug '17

On 11.08.2017 16:10, Zebediah Figura wrote: > +cpp_quote("#define INTERNETEXPLORERCONFIGURATION_HOST 0x00000001") This should be a value of INTERNETEXPLORERCONFIGURATION enum. Jacek

1 0

Re: [PATCH v4 1/5] include: Add initial ieautomation.idl
by Jacek Caban 11 Aug '17

11 Aug '17

On 11.08.2017 15:54, Zebediah Figura wrote: > + */ > + > +import "unknwn.idl"; One more thing I didn't notice earlier: this should be ocidl.idl. Jacek

1 0

Re: [v3 5/5] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Carlos Palminha 11 Aug '17

11 Aug '17

Hi, My patch with test is producing a strange result in Test Bot. I'm getting a ERROR_NOT_ENOUGH_MEMORY when calling LoadLibraryExA only in Vista and Win2008 64bit. Any hint? Regards, C.Palminha On 11-08-2017 11:39, Carlos Palminha wrote: > Test LIBRARY_SEARCH_* flags when loading libraries. > Uses helper from path.c to create the test dll file. > > Signed-off-by: Carlos Palminha <palminha(a)synopsys.com> > --- > v3: > - Improved check for Win2003 cases > > v2: > - Split the tests affecting different files > - Tests checks for Win2003 cases > > dlls/kernel32/tests/module.c | 132 ++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 131 insertions(+), 1 deletion(-) > > diff --git a/dlls/kernel32/tests/module.c b/dlls/kernel32/tests/module.c > index bb22c24316c..96dd3277c1b 100644 > --- a/dlls/kernel32/tests/module.c > +++ b/dlls/kernel32/tests/module.c > @@ -407,6 +407,136 @@ static void testLoadLibraryEx(void) > FreeLibrary(hmodule); > } > > +/* use some functions and strcutures from loader.c */ > +extern DWORD create_test_dll( const IMAGE_DOS_HEADER *dos_header, UINT dos_size, > + const IMAGE_NT_HEADERS *nt_header, const char *dll_name ); > +extern const IMAGE_NT_HEADERS nt_header_template; > + > +static void testLoadLibraryExSearchFlags(void) > +{ > + HMODULE hmodule; > + char tmpPath[MAX_PATH]; > + char dllName[] = "testFlags.dll"; > + char tmpFullFile[MAX_PATH]; > + char sysFullFile[MAX_PATH]; > + IMAGE_DOS_HEADER dos_header; > + IMAGE_NT_HEADERS nt_header; > + ULONG file_size; > + char fullPath[MAX_PATH]; > + DWORD version; > + BOOL is_win2003 = FALSE; > + > + GetTempPathA(MAX_PATH, tmpPath); > + lstrcpyA(tmpFullFile, tmpPath); > + lstrcatA(tmpFullFile, dllName); > + > + /* check for required function */ > + if (!pSetDllDirectoryA) > + { > + win_skip("SetDllDirectoryA not available\n"); > + return; > + } > + > + /* check if we are in win2003... flags are not supported */ > + version = GetVersion(); > + if ((LOBYTE(LOWORD(version)) == 5) && (HIBYTE(LOWORD(version))==2)) > + is_win2003 = TRUE; > + > + /* try to load test dll from test path before creating it*/ > + SetLastError(0xdeadbeef); > + pSetDllDirectoryA(tmpPath); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS); > + ok (hmodule == NULL, "Test library %s should not be loaded from %s\n", dllName, tmpPath); > + ok( GetLastError() == ERROR_MOD_NOT_FOUND || GetLastError() == ERROR_DLL_NOT_FOUND || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + pSetDllDirectoryA(NULL); > + > + /* create test library */ > + lstrcpyA(fullPath, tmpPath); > + lstrcatA(fullPath, dllName); > + > + dos_header.e_magic = IMAGE_DOS_SIGNATURE; > + dos_header.e_lfanew = sizeof(dos_header); > + > + nt_header = nt_header_template; > + > + nt_header.FileHeader.NumberOfSections = 1; > + nt_header.FileHeader.SizeOfOptionalHeader = sizeof(IMAGE_OPTIONAL_HEADER); > + nt_header.OptionalHeader.SectionAlignment = 0x1000; > + nt_header.OptionalHeader.FileAlignment = 0x1000; > + nt_header.OptionalHeader.SizeOfImage = sizeof(dos_header) + sizeof(nt_header_template) + sizeof(IMAGE_SECTION_HEADER) + 0x1000; > + nt_header.OptionalHeader.SizeOfHeaders = sizeof(dos_header) + sizeof(nt_header_template) + sizeof(IMAGE_SECTION_HEADER); > + > + file_size = create_test_dll(&dos_header, sizeof(dos_header), &nt_header, fullPath); > + ok( file_size, "could not create '%s'\n",fullPath); > + > + /* try to load test dll without specifying the test path */ > + SetLastError(0xdeadbeef); > + pSetDllDirectoryA(NULL); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS); > + ok (hmodule == NULL, "Test library should not be loaded from 'NULL' path\n"); > + ok( GetLastError() == ERROR_MOD_NOT_FOUND || GetLastError() == ERROR_DLL_NOT_FOUND || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + > + /* try to load test dll from test path */ > + SetLastError(0xdeadbeef); > + pSetDllDirectoryA(tmpPath); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS); > + if(is_win2003) > + ok (hmodule == NULL, "Test library should not be loaded from %s in Win2003 (flag not supported)\n", tmpPath); > + else > + ok (hmodule != NULL, "Could not load test library %s from %s\n", dllName, tmpPath); > + ok(GetLastError() == 0xdeadbeef || GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected 0xdeadbeef or ERROR_SUCCESS or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + pSetDllDirectoryA(NULL); > + > + /* load non-existing test dll from system32 */ > + SetLastError(0xdeadbeef); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); > + ok(hmodule == NULL, "%s should not be loaded\n", dllName); > + ok( GetLastError() == ERROR_MOD_NOT_FOUND || GetLastError() == ERROR_DLL_NOT_FOUND || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + > + /* move library from temp path to system32 and delete from old path */ > + GetWindowsDirectoryA(sysFullFile,MAX_PATH); > + lstrcatA(sysFullFile,"\\system32\\"); > + lstrcatA(sysFullFile, dllName); > + MoveFileA(tmpFullFile,sysFullFile); > + DeleteFileA(tmpFullFile); > + > + /* try to load dll using more than one flag */ > + pSetDllDirectoryA(tmpPath); > + SetLastError(0xdeadbeef); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_USER_DIRS | LOAD_LIBRARY_SEARCH_SYSTEM32); > + if(is_win2003) > + ok (hmodule == NULL, "Test library should not be loaded from 'system32' in Win2003 (flag not supported)\n"); > + else > + ok (hmodule != NULL, "Library %s was expected to be loaded from 'system32'\n", dllName); > + ok(GetLastError() == 0xdeadbeef || GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected 0xdeadbeef or ERROR_SUCCESS or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + pSetDllDirectoryA(NULL); > + > + /* try to load test dll from system32 */ > + SetLastError(0xdeadbeef); > + hmodule = LoadLibraryExA(dllName, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); > + if(is_win2003) > + ok (hmodule == NULL, "Test library should not be loaded from 'system32' in Win2003 (flag not supported)\n"); > + else > + ok (hmodule != NULL, "Library %s was expected to be loaded from 'system32'\n", dllName); > + ok(GetLastError() == 0xdeadbeef || GetLastError() == ERROR_SUCCESS || GetLastError() == ERROR_INVALID_PARAMETER, > + "Expected 0xdeadbeef or ERROR_SUCCESS or ERROR_INVALID_PARAMETER (win2003), got %d\n", GetLastError()); > + FreeLibrary(hmodule); > + > + /* delete test dll from system32 directory */ > + DeleteFileA(sysFullFile); > + > +} > + > static void testGetDllDirectory(void) > { > CHAR bufferA[MAX_PATH]; > @@ -744,7 +874,6 @@ START_TEST(module) > WCHAR filenameW[MAX_PATH]; > > /* Test if we can use GetModuleFileNameW */ > - > SetLastError(0xdeadbeef); > GetModuleFileNameW(NULL, filenameW, MAX_PATH); > if (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) > @@ -766,6 +895,7 @@ START_TEST(module) > testLoadLibraryA_Wrong(); > testGetProcAddress_Wrong(); > testLoadLibraryEx(); > + testLoadLibraryExSearchFlags(); > testGetModuleHandleEx(); > testK32GetModuleInformation(); > } >

1 0

Re: [PATCH v3 1/5] include: Add initial ieautomation.idl
by Jacek Caban 11 Aug '17

11 Aug '17

On 10.08.2017 20:52, Zebediah Figura wrote: > + > +cpp_quote("DEFINE_GUID(CLSID_InternetExplorerManager, 0xdf4fcc34, 0x067a, 0x4e0a, 0x83,0x52, 0x4a,0x1a,0x50,0x95,0x34,0x6e);") Please use coclass, like in PSDK, instead. Jacek

1 0

Re: [v3 5/5] kernel32/tests: Add tests for LoadLibrary with LIBRARY_SEARCH_* flags
by Marvin 11 Aug '17

11 Aug '17

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=32633 Your paranoid android. === wvistau64 (32 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === w2008s64 (32 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === wvistau64 (64 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8 === w2008s64 (64 bit module) === module.c:479: Test failed: Expected ERROR_MOD_NOT_FOUND or ERROR_DLL_NOT_FOUND (win9x) or ERROR_INVALID_PARAMETER (win2003), got 8

1 0

Re: [PATCH] comctl32/listview: Keep the active item selected after hwndEdit is destroyed
by Nikolay Sivov 11 Aug '17

11 Aug '17

Could you describe how should I be testing this?

2 1

Re: [PATCH v2 3/5] ieframe: Add stub implementation of IInternetExplorerManager.
by Jacek Caban 10 Aug '17

10 Aug '17

Hi Zebediah, On 08.08.2017 20:04, Zebediah Figura wrote: > HRESULT register_class_object(BOOL do_reg) > { > HRESULT hres; > > - static DWORD cookie; > + static DWORD ie_cookie, iem_cookie; > > if(do_reg) { > hres = CoRegisterClassObject(&CLSID_InternetExplorer, > (IUnknown*)&InternetExplorerFactory, CLSCTX_SERVER, > - REGCLS_MULTIPLEUSE|REGCLS_SUSPENDED, &cookie); > - if (FAILED(hres)) { > + REGCLS_MULTIPLEUSE, &ie_cookie); > + if (FAILED(hres)) > + ERR("failed to register object %08x\n", hres); > + hres = CoRegisterClassObject(&CLSID_InternetExplorerManager, > + (IUnknown*)&InternetExplorerManagerFactory, CLSCTX_SERVER, > + REGCLS_MULTIPLEUSE, &iem_cookie); > + if (FAILED(hres)) > ERR("failed to register object %08x\n", hres); > - return hres; > - } > - > - hres = CoResumeClassObjects(); > - if(SUCCEEDED(hres)) > - return hres; > - > - ERR("failed to resume object %08x\n", hres); > } > - > - return CoRevokeClassObject(cookie); > + else > + { > + hres = CoRevokeClassObject(ie_cookie); > + if (FAILED(hres)) > + ERR("failed to register object %08x\n", hres); > + hres = CoRevokeClassObject(iem_cookie); > + if (FAILED(hres)) > + ERR("failed to register object %08x\n", hres); > + } > + return hres; > } Are you sure that's how it's supposed to work? I didn't test it myself, but given how it's registered and documented, I'd expect that CLSID_InternetExplorerManager should live in a different process from CLSID_InternetExplorer. It should be easy to distinguish those processes with -startmanager command line argument. > +static HRESULT WINAPI InternetExplorerManager_CreateObject(IInternetExplorerManager *iface, DWORD config, LPCWSTR url, REFIID riid, void **ppv) > +{ > + FIXME("(%p)->(0x%x, %s, %s, %p) semi-stub\n", iface, config, debugstr_w(url), debugstr_guid(riid), ppv); > + > + return CoCreateInstance(&CLSID_InternetExplorer, NULL, CLSCTX_LOCAL_SERVER, riid, ppv); > +} Using CoCreateInstance is very questionable here (eg. due to the comment above). Also, a patch adding a stub should just return E_NOTIMPL. Implementation deserves a separated patch. I'd expect that it should just use internal ieframe functions to create a new IE instance. Also, I'd suggest to reorder patches to something like: 2, 4, 5, 1 (likely replaced by support for -startmanager command line argument), 3 and then a new patch implementing CreateObject. Thanks, Jacek

2 6

Re: [v3 PATCH 1/2] dwrite: Protect cached fontface list when accessed from multiple threads
by Sebastian Lackner 10 Aug '17

10 Aug '17

On 10.08.2017 08:45, Nikolay Sivov wrote: > Based on patch by Anton Romanov <theli.ua(a)gmail.com> > > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/dwrite/dwrite_private.h | 6 ++++-- > dlls/dwrite/font.c | 24 ++++++++++++--------- > dlls/dwrite/main.c | 50 +++++++++++++++++++++++++++++++++----------- > 3 files changed, 56 insertions(+), 24 deletions(-) > Isn't it also necessary to protect factory_cache_fontface()? By the way: An alternative method to implement proper synchronization (which does not require locking critical sections before InterlockedDecrement) would be to do something like: In the destructor: --- snip --- if (!ref) { EnterCriticalSection(...); list_remove(...); LeaveCriticalSection(...); ... } --- snip --- And when querying cached font faces: --- snip --- EnterCriticalSection(...); LIST_FOR_EACH_ENTRY(...) { ... /* the following code could also be hidden in QueryInterface */ if (InterlockedIncrement( &obj->refcount ) == 1) { InterlockedDecrement( &obj->refcount ); continue; } /* return it */ } LeaveCriticalSection(...); --- snip --- This is also what is done in Wines threadpool implementation [1], and has the advantage that the CS is not needed for every Release, only when the object is really destroyed. But in the end it is a matter of taste, for rpcrt4 Jacek also picked the "hold CS during InterlockedDecrement" method. [1] https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/ntdll/threadpool.c#l… Best regards, Sebastian

3 2