September 2017 - Wine-Devel

Re: [PATCH] comctl32/listview: Send NM_RELEASEDCAPTURE after the window focus has changed

by Nikolay Sivov

What use case does this fix? With this patch notification order is more broken than without. On Windows when clicking on item of unfocused control gives roughly: NM_RELEASEDCAPTURE LVN_ITEMCHANGING/LVN_ITEMCHANGED NM_CUSTOMDRAW NM_SETFOCUS NM_CLICK Without this patch we currently do: NM_RELEASEDCAPTURE LVN_ITEMCHANGIN/LVN_ITEMCHANGED NM_SETFOCUS NM_CLICK With the patch: LVN_ITEMCHANGING/LVN_ITEMCHANGED NM_SETFOCUS NM_RELEASECAPTURE NM_CLICK (in both cases on Wine sequence is interleaved with some NM_CUSTOMDRAW)

Sept. 8, 2017

Re: webservices: Make alloc_float_text() static.

by Hans Leidekker

On Fri, 2017-09-08 at 02:24 +0200, Francois Gouget wrote: > dlls/webservices/reader.c | 2 +- > dlls/webservices/webservices_private.h | 1 - > 2 files changed, 1 insertion(+), 2 deletions(-) > > It was introduced in 82306e84 on 2017-07-20 and has not been used > outside reader.c since then. Ignore this patch if there are ongoing > plans to use it elsewhere. It will be used in the writer at some point.

Sept. 8, 2017

Re: msi: Make MSI_ProcessMessageVerbatim() static.

by Hans Leidekker

On Fri, 2017-09-08 at 02:12 +0200, Francois Gouget wrote: > diff --git a/dlls/msi/msipriv.h b/dlls/msi/msipriv.h > index f316c0cce74..1bb0a849eed 100644 > --- a/dlls/msi/msipriv.h > +++ b/dlls/msi/msipriv.h > @@ -872,7 +872,6 @@ extern UINT MSI_SetInstallLevel( MSIPACKAGE *package, int iInstallLevel ) DECLSP > extern MSIPACKAGE *MSI_CreatePackage( MSIDATABASE *, LPCWSTR ) DECLSPEC_HIDDEN; > extern UINT MSI_OpenPackageW( LPCWSTR szPackage, MSIPACKAGE **pPackage ) DECLSPEC_HIDDEN; > extern UINT MSI_SetTargetPathW( MSIPACKAGE *, LPCWSTR, LPCWSTR ) DECLSPEC_HIDDEN; > -extern INT MSI_ProcessMessageVerbatim( MSIPACKAGE *, INSTALLMESSAGE, MSIRECORD * ) DECLSPEC_HIDDEN; > extern INT MSI_ProcessMessage( MSIPACKAGE *, INSTALLMESSAGE, MSIRECORD * ) DECLSPEC_HIDDEN; > extern MSICONDITION MSI_EvaluateConditionW( MSIPACKAGE *, LPCWSTR ) DECLSPEC_HIDDEN; > extern UINT MSI_GetComponentStateW( MSIPACKAGE *, LPCWSTR, INSTALLSTATE *, INSTALLSTATE * ) DECLSPEC_HIDDEN; > diff --git a/dlls/msi/package.c b/dlls/msi/package.c > index 47164613209..a88609a3dd8 100644 > --- a/dlls/msi/package.c > +++ b/dlls/msi/package.c > @@ -1444,6 +1444,8 @@ UINT msi_set_original_database_property( MSIDATABASE *db, const WCHAR *package ) > return r; > } > > +static INT MSI_ProcessMessageVerbatim(MSIPACKAGE*,INSTALLMESSAGE,MSIRECORD*); I'd prefer moving this function (and internal_ui_handler) up.

Sept. 8, 2017

Re: [PATCH 3/3] shell32: Implement FolderItems_get_Count.

by Nikolay Sivov

On 08.09.2017 8:29, Alex Henrie wrote: > > static HRESULT WINAPI FolderItemsImpl_get_Count(FolderItems3 *iface, LONG *count) > { > - FIXME("(%p,%p)\n", iface, count); > + FolderItemsImpl *This = impl_from_FolderItems(iface); > > - return E_NOTIMPL; > + TRACE("(%p,%p)\n", iface, count); > + > + *count = PathIsDirectoryW(V_BSTR(&This->dir)) ? This->item_count : 0; > + return S_OK; > } > This looks too complicated. Why can't we always return item_count without additional checks?

Sept. 8, 2017

Re: [PATCH 2/3] shell32: Implement FolderItems_Item.

by Nikolay Sivov

On 08.09.2017 8:29, Alex Henrie wrote: > typedef struct { > FolderItems3 FolderItems3_iface; > LONG ref; > + VARIANT dir; > + WCHAR **item_filenames; > + LONG item_count; > } FolderItemsImpl; If 'dir' is always a string maybe store it as such? > static HRESULT WINAPI FolderItemsImpl_Item(FolderItems3 *iface, VARIANT index, FolderItem **ppid) > { > - FIXME("(%p,%s,%p)\n", iface, debugstr_variant(&index), ppid); > + FolderItemsImpl *This = impl_from_FolderItems(iface); > + WCHAR path_str[MAX_PATH]; > + VARIANT path_var; > + HRESULT ret; > + > + TRACE("(%p,%s,%p)\n", iface, debugstr_variant(&index), ppid); > > *ppid = NULL; > - return E_NOTIMPL; > + > + if (!PathIsDirectoryW(V_BSTR(&This->dir))) > + return S_FALSE; How could it not be a directory? > + TRACE("(%p,%s,%p)\n", iface, debugstr_variant(&index), ppid); Could add some spaces in argument list? Most of the wine code is doing that I think. > + if (!PathCombineW(path_str, V_BSTR(&This->dir), This->item_filenames[V_I4(&index)])) > + return E_OUTOFMEMORY; Why E_OUTOFMEMORY? > + do > + { > + if (!strcmpW(file_info.cFileName, dot) || !strcmpW(file_info.cFileName, dot_dot)) > + continue; > + > + if (This->item_count >= item_size) > + { > + item_size *= 2; > + filenames = HeapReAlloc(GetProcessHeap(), 0, This->item_filenames, item_size * sizeof(WCHAR*)); > + if (!filenames) > + goto fail; > + This->item_filenames = filenames; > + } > + > + This->item_filenames[This->item_count] = strdupW(file_info.cFileName); > + if (!This->item_filenames[This->item_count]) > + goto fail; > + This->item_count++; > + } > + while (FindNextFileW(first_file, &file_info)); Do we have tests showing that it takes a snapshot like that? For example if new file is created after FolderItems was created, is it accessible with BSTR index for example?

Sept. 8, 2017

Re: [PATCH 1/3] shell32/tests: Add tests for FolderItems_Item and FolderItems_get_Count.

by Nikolay Sivov

On 08.09.2017 8:29, Alex Henrie wrote: > + for (i = 0; i < sizeof(file_defs)/sizeof(file_defs[0]); i++) > + { > + switch (file_defs[i].type) > + { > + case DIRECTORY: > + r = CreateDirectoryA(file_defs[i].name, NULL); > + ok(r, "CreateDirectory failed: %08x\n", GetLastError()); > + PathCombineA(buf, file_defs[i].name, "foo.txt"); > + file = CreateFileA(buf, 0, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed: %08x\n", GetLastError()); > + CloseHandle(file); > + break; > + > + case EMPTY: > + file = CreateFileA(file_defs[i].name, 0, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed: %08x\n", GetLastError()); > + CloseHandle(file); > + break; > + > + case RANDOM: > + file = CreateFileA(file_defs[i].name, GENERIC_WRITE, 0, NULL, > + CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > + ok(file != INVALID_HANDLE_VALUE, "CreateFile failed: %08x\n", GetLastError()); > + for (j = 0; j < sizeof(buf); j++) > + buf[j] = rand() % 256; > + r = WriteFile(file, buf, sizeof(buf), &dwcount, NULL); > + ok(r, "WriteFile failed: %08x\n", GetLastError()); > + CloseHandle(file); > + break; > + } > + } Why would it matter if file is empty or not? And if it does for some reason, why filling it with random content? > + MultiByteToWideChar(CP_ACP, 0, file_defs[0].name, -1, wstr, MAX_PATH); > + V_VT(&var) = VT_BSTR; > + V_BSTR(&var) = SysAllocString(wstr); I understand why you have test names as single byte strings, but this construct is used more than once, so maybe add a helper for that. Could be similar to _bstr_() in msxml test, but without global pointer array. > + for (i = -10; i < 10; i++) > + { > + V_VT(&var) = VT_ERROR; > + V_ERROR(&var) = i; VT_ERROR is used for optional arguments, what does [-10,10) range mean? I think it's enough to test some HRESULT-style failure code. > + ok(!!bstr, "bstr is null\n"); > + ok(!lstrcmpW(bstr, cur_dir), > + "expected %s, got %s\n", wine_dbgstr_w(cur_dir), wine_dbgstr_w(bstr)); First check seems redundant.

Sept. 8, 2017

Re: [PATCH 1/3] d3dx9/tests: Add test for null shaders in effect.

by Matteo Bruni

2017-09-01 20:19 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > +static void test_effect_null_shader(void) > +{ > + D3DPRESENT_PARAMETERS present_parameters = {0}; > + IDirect3DDevice9 *device; > + ID3DXEffect *effect; > + D3DXPASS_DESC desc; > + IDirect3D9 *d3d; > + D3DXHANDLE pass; > + ULONG refcount; > + HWND window; > + HRESULT hr; > + > + /* Creating a fresh device because the existing device can have invalid render states > + from previous tests. If IDirect3DDevice9_ValidateDevice() returns certain error codes, > + native ValidateTechnique() fails. */ We generally want to be sure that each test is independent from the others so creating a separate device for each test function is a good idea regardless. Your point about ValidateDevice() is very interesting though, actually it makes a lot of sense and it probably means that ValidateTechnique() should delegate a good chunk of its work to ValidateDevice() (not surprising but somehow I forgot about it). It would be interesting to extend the d3d9 ValidateDevice() tests at some point.

Sept. 7, 2017

Re: [1/2] ntdll: Implement set_cpu_context for ARM64

by Alexandre Julliard

André Hentschel <nerv(a)dawncrow.de> writes: > + "ldr x30, [x17, #0xf8]\n\t" > + "ldr x16, [x17, #0x100]\n\t" > + "mov sp, x16\n\t" > + "ldr x16, [x17, #0x108]\n\t" > + "br x16\n\t" You'd also need to restore x16 and x17, this is used in signal handlers. -- Alexandre Julliard julliard(a)winehq.org

Sept. 7, 2017

Re: [v5 PATCH] wmvcore/tests: add tests for WMReader methods

by Jefferson Carpenter

Okay, that makes sense. For the implementation, should libav be used, or ought wine to have its own implementation of the codecs? On Tue, Sep 5, 2017 at 1:43 PM, Alexandre Julliard <julliard(a)winehq.org> wrote: > Jefferson Carpenter <jeffersoncarpenter2(a)gmail.com> writes: > > > Without a large binary file, how would we test the successful > > processing of an audio or video file? We could use an IWMWriter to > > create it, but then we wouldn't be testing for conformance against the > > Windows API. > > In general, to test something like a file loader, you'd want to test > multiple files with various contents, including invalid values for > header fields and the like. This is better done by generating them > instead of including pre-built binaries. Like you said, using IWMWriter > wouldn't guarantee that we are testing the right thing, so we'd most > likely want to generate them by hand somehow. > > Again, this would be better done along with the implementation, so we > can get a better idea of what cases would be interesting to test. > > -- > Alexandre Julliard > julliard(a)winehq.org >

Sept. 6, 2017

Re: [PATCH] secur32: Split push/pull schannel transports

by Anton Romanov

On Wed, Sep 6, 2017 at 7:50 AM, Alexandre Julliard <julliard(a)winehq.org> wrote: > Anton Romanov <theli.ua(a)gmail.com> writes: > >> So, it's been a month now. Is there something I could do to have this accepted? > > It's assigned to Jacek for review, but he's on vacation until tomorrow I > believe, so it may be a few more days until he gets around to it. Sorry > about the delay. I see, thanks!

Sept. 6, 2017