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

Re: [PATCH 1/5] d3drm/tests: Some tests for creating texture from ddraw surface
by Henri Verbeet 05 Jun '17

05 Jun '17

On 5 June 2017 at 01:07, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> > --- > dlls/d3drm/tests/d3drm.c | 188 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 188 insertions(+) > It looks like a good part of this was inspired by the existing d3drm tests, and it doesn't appear to be any worse than those. However, we'd like to raise the standards a little. I.e., in the future, please use the d3d11 and ddraw tests as a reference, instead of the existing d3drm tests.

1 0

Re: [PATCH] shell32: Implement FCIDM_SHVIEW_CREATELINK
by Nikolay Sivov 05 Jun '17

05 Jun '17

On 02.06.2017 17:24, Aric Stewart wrote: > > Presently the wine file explorer has a create shortcut entry that > does nothing. This implements the FCIDM_SHVIEW_CREATELINK command. > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/shell32/shlview_cmenu.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 50 insertions(+) > > + if (!SUCCEEDED(CoCreateInstance( &CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER, > + &IID_IShellLinkW, (LPVOID*)&shelllink))) > + { > + ERR("couldn't create ShellLink object\n"); > + return; > + } This should be FAILED(). You can use IShellLink_Constructor() instead of going through generic object creation. > + SHGetSpecialFolderPathW(NULL, root, CSIDL_STARTMENU, TRUE); > + lnkfile=HeapAlloc(GetProcessHeap(), 0, (lstrlenW(root)+1+lstrlenW(wszFilename)+5) * sizeof(*lnkfile)); > + lstrcpyW(lnkfile, root); > + lstrcatW(lnkfile, backslashW); > + lstrcatW(lnkfile, wszFilename); > + lstrcatW(lnkfile, lnkW); Why is it created in Start Menu dir? I think usual behavior is to create it in target directory. > + if (!SUCCEEDED(IShellLinkW_QueryInterface(shelllink, &IID_IPersistFile, (LPVOID*)&persistfile))) > + { > + ERR("couldn't get IPersistFile interface\n"); > + IShellLinkW_Release(shelllink); > + HeapFree(GetProcessHeap(), 0, lnkfile); > + return; > + } FAILED() again, but really it can't fail. Also please LPVOID -> void*.

2 2

Re: [PATCH] msxml3: Remove CRs in domtext_put_data and add them in domtext_get_xml.
by Nikolay Sivov 05 Jun '17

05 Jun '17

This needs more tests, for different msxml object versions, and for different input document. It's possible that get_xml() returns original document, so test loading non trivial document is necessary. I remember looking at this some time ago, not for this specific report but in general, and my conclusion was that the only way to generate matching document is to duplicate node dumping in our code.

1 0

Re: kernel32: Fix WriteConsoleOutputCharacter[AW] spec file entries
by Alexandre Julliard 02 Jun '17

02 Jun '17

André Hentschel <nerv(a)dawncrow.de> writes: > @@ -1612,8 +1612,8 @@ > @ stdcall WriteConsoleInputW(long ptr long ptr) > @ stdcall WriteConsoleOutputA(long ptr long long ptr) > @ stdcall WriteConsoleOutputAttribute(long ptr long long ptr) > -@ stdcall WriteConsoleOutputCharacterA(long ptr long long ptr) > -@ stdcall WriteConsoleOutputCharacterW(long ptr long long ptr) > +@ stdcall WriteConsoleOutputCharacterA(long str long long ptr) > +@ stdcall WriteConsoleOutputCharacterW(long wstr long long ptr) It's questionable to use string types on potentially non null-terminated strings. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 2/8] comcat: label ordinals
by Alexandre Julliard 02 Jun '17

02 Jun '17

Austin English <austinenglish(a)gmail.com> writes: > Signed-off-by: Austin English <austinenglish(a)gmail.com> > --- > dlls/comcat/comcat.spec | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/dlls/comcat/comcat.spec b/dlls/comcat/comcat.spec > index 5b5f7b06cb..781b07437c 100644 > --- a/dlls/comcat/comcat.spec > +++ b/dlls/comcat/comcat.spec > @@ -1,4 +1,4 @@ > -@ stdcall -private DllCanUnloadNow() > -@ stdcall -private DllGetClassObject(ptr ptr ptr) ole32.DllGetClassObject > -@ stdcall -private DllRegisterServer() > -@ stdcall -private DllUnregisterServer() > +1 stdcall -private DllGetClassObject(ptr ptr ptr) ole32.DllGetClassObject > +2 stdcall -private DllCanUnloadNow() > +3 stdcall -private DllRegisterServer() > +4 stdcall -private DllUnregisterServer() It doesn't make much sense to define ordinals for such functions. -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH] user32: Keep tracking if we just created the menu.
by Huw Davies 02 Jun '17

02 Jun '17

On Wed, May 31, 2017 at 02:34:06PM -0500, Zebediah Figura wrote: > Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> > --- > dlls/user32/menu.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/dlls/user32/menu.c b/dlls/user32/menu.c > index a65f01eaa5..c52b723f76 100644 > --- a/dlls/user32/menu.c > +++ b/dlls/user32/menu.c > @@ -2597,7 +2597,10 @@ static BOOL MENU_ButtonDown( MTRACKER* pmt, HMENU hPtMenu, UINT wFlags ) > > /* If the popup menu is not already "popped" */ > if (!(ptmenu->items[pos].fState & MF_MOUSESELECT)) > + { > pmt->hCurrentMenu = MENU_ShowSubPopup( pmt->hOwnerWnd, hPtMenu, FALSE, wFlags ); > + return TRUE; > + } > } > > /* A click on an item or anywhere on a popup keeps tracking going */ So under what conditions does the if statement directly below this hunk not get triggered? Huw.

2 4

Re: [PATCH] fusion: use correct size for ZeroMemory (GCC7)
by Dmitry Timoshkov 02 Jun '17

02 Jun '17

Marcus Meissner <meissner(a)suse.de> wrote: > diff --git a/dlls/fusion/tests/asmname.c b/dlls/fusion/tests/asmname.c > index 5fb14a4829..21cd4874ed 100644 > --- a/dlls/fusion/tests/asmname.c > +++ b/dlls/fusion/tests/asmname.c > @@ -358,7 +358,7 @@ static void test_assembly_name_props_line(IAssemblyName *name, > to_widechar(expect, vals[i].val); > > size = MAX_PATH; > - ZeroMemory(str, MAX_PATH); > + ZeroMemory(str, sizeof(str)); > hr = IAssemblyName_GetProperty(name, i, str, &size); > to_multibyte(val, str); Same problem with IAssemblyName_GetProperty() and 'size' argumenet as for the previous patch. -- Dmitry.

2 1

Re: [PATCH] use correct size for ZeroMemory (GCC7)
by Dmitry Timoshkov 02 Jun '17

02 Jun '17

Marcus Meissner <meissner(a)suse.de> wrote: > --- a/dlls/fusion/tests/asmname.c > +++ b/dlls/fusion/tests/asmname.c > @@ -358,7 +358,7 @@ static void test_assembly_name_props_line(IAssemblyName *name, > to_widechar(expect, vals[i].val); > > size = MAX_PATH; > - ZeroMemory(str, MAX_PATH); > + ZeroMemory(str, sizeof(str)); > hr = IAssemblyName_GetProperty(name, i, str, &size); > to_multibyte(val, str); According to the implementation of IAssemblyName_GetProperty() 'size' assignment needs to be fixed as well. And IAssemblyName_GetProperty() implementation should be fixed to actually take 'size' into account. -- Dmitry.

1 0

Re: [PATCH 3/4] quartz: fix crash in FilterGraph2_Render
by Andrew Eikum 02 Jun '17

02 Jun '17

Can you add a test for this? Andrew On Thu, May 25, 2017 at 04:54:49PM +0200, Miklós Máté wrote: > Note: there are a lot of other places as well where the null pointer handling > is present in the msdn docs but missing in wine. > > Signed-off-by: Miklós Máté <mtmkls(a)gmail.com> > --- > dlls/quartz/filtergraph.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/dlls/quartz/filtergraph.c b/dlls/quartz/filtergraph.c > index a143b05d83..e26eb199c8 100644 > --- a/dlls/quartz/filtergraph.c > +++ b/dlls/quartz/filtergraph.c > @@ -1279,6 +1279,9 @@ static HRESULT WINAPI FilterGraph2_Render(IFilterGraph2 *iface, IPin *ppinOut) > > TRACE("(%p/%p)->(%p)\n", This, iface, ppinOut); > > + if (!ppinOut) > + return E_POINTER; > + > if (TRACE_ON(quartz)) > { > PIN_INFO PinInfo; > -- > 2.11.0 > > >

3 2

Re: [PATCH 1/4] quartz: AsyncReader is always expected to have output pin
by Andrew Eikum 01 Jun '17

01 Jun '17

Thanks for working on this. I tested your patch sequence against a number of games as well as embedding videos in MSOffice 2007 and 2010 and found no regressions. I do have some comments, though, in-line below. Is this something you could write a test for? E.g. create the filter and then query for its output pin immediately after creation. On Thu, May 25, 2017 at 04:54:47PM +0200, Miklós Máté wrote: > Even between create() and Load(). This fixes a crash in DSPlayer. > > Signed-off-by: Miklós Máté <mtmkls(a)gmail.com> > --- > dlls/quartz/filesource.c | 31 ++++++++++++++++++++----------- > 1 file changed, 20 insertions(+), 11 deletions(-) > > diff --git a/dlls/quartz/filesource.c b/dlls/quartz/filesource.c > index ed7e5ee201..200463ff5b 100644 > --- a/dlls/quartz/filesource.c > +++ b/dlls/quartz/filesource.c > @@ -73,7 +73,8 @@ static const IFileSourceFilterVtbl FileSource_Vtbl; > static const IAsyncReaderVtbl FileAsyncReader_Vtbl; > static const IAMFilterMiscFlagsVtbl IAMFilterMiscFlags_Vtbl; > > -static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin); > +static HRESULT FileAsyncReader_Construct(IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin); > +static HRESULT FileAsyncReader_SetFile(IAsyncReader * iface, HANDLE hFile); > > static const WCHAR mediatype_name[] = { > 'M', 'e', 'd', 'i', 'a', ' ', 'T', 'y', 'p', 'e', 0 }; > @@ -416,6 +417,7 @@ static const BaseFilterFuncTable BaseFuncTable = { > HRESULT AsyncReader_create(IUnknown * pUnkOuter, LPVOID * ppv) > { > AsyncReader *pAsyncRead; > + HRESULT hr; > > if( pUnkOuter ) > return CLASS_E_NOAGGREGATION; > @@ -429,7 +431,8 @@ HRESULT AsyncReader_create(IUnknown * pUnkOuter, LPVOID * ppv) > > pAsyncRead->IFileSourceFilter_iface.lpVtbl = &FileSource_Vtbl; > pAsyncRead->IAMFilterMiscFlags_iface.lpVtbl = &IAMFilterMiscFlags_Vtbl; > - pAsyncRead->pOutputPin = NULL; > + hr = FileAsyncReader_Construct(&pAsyncRead->filter.IBaseFilter_iface, &pAsyncRead->filter.csFilter, &pAsyncRead->pOutputPin); > + BaseFilterImpl_IncrementPinVersion(&pAsyncRead->filter); If this fails, I think we should free resources and return the error immediately. > > pAsyncRead->pszFileName = NULL; > pAsyncRead->pmt = NULL; > @@ -438,7 +441,7 @@ HRESULT AsyncReader_create(IUnknown * pUnkOuter, LPVOID * ppv) > > TRACE("-- created at %p\n", pAsyncRead); > > - return S_OK; > + return hr; > } > > /** IUnknown methods **/ > @@ -606,7 +609,7 @@ static ULONG WINAPI FileSource_Release(IFileSourceFilter * iface) > > static HRESULT WINAPI FileSource_Load(IFileSourceFilter * iface, LPCOLESTR pszFileName, const AM_MEDIA_TYPE * pmt) > { > - HRESULT hr; > + HRESULT hr = E_FAIL; > HANDLE hFile; > IAsyncReader * pReader = NULL; > AsyncReader *This = impl_from_IFileSourceFilter(iface); > @@ -625,16 +628,15 @@ static HRESULT WINAPI FileSource_Load(IFileSourceFilter * iface, LPCOLESTR pszFi > return HRESULT_FROM_WIN32(GetLastError()); > } > > - /* create pin */ > - hr = FileAsyncReader_Construct(hFile, &This->filter.IBaseFilter_iface, &This->filter.csFilter, &This->pOutputPin); > - BaseFilterImpl_IncrementPinVersion(&This->filter); > > - if (SUCCEEDED(hr)) > + if (This->pOutputPin) Since we now create the output pin during filter creation, this should never be non-NULL, right? > hr = IPin_QueryInterface(This->pOutputPin, &IID_IAsyncReader, (LPVOID *)&pReader); > > /* store file name & media type */ > if (SUCCEEDED(hr)) > { > + FileAsyncReader_SetFile(pReader, hFile); > + > CoTaskMemFree(This->pszFileName); > if (This->pmt) > FreeMediaType(This->pmt); > @@ -939,7 +941,7 @@ static const BaseOutputPinFuncTable output_BaseOutputFuncTable = { > BaseOutputPinImpl_BreakConnect > }; > > -static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin) > +static HRESULT FileAsyncReader_Construct(IBaseFilter * pBaseFilter, LPCRITICAL_SECTION pCritSec, IPin ** ppPin) > { > PIN_INFO piOutput; > HRESULT hr; > @@ -952,9 +954,8 @@ static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter > > if (SUCCEEDED(hr)) > { > - FileAsyncReader *pPinImpl = (FileAsyncReader *)*ppPin; > + FileAsyncReader *pPinImpl = impl_from_IPin(*ppPin); > pPinImpl->IAsyncReader_iface.lpVtbl = &FileAsyncReader_Vtbl; > - pPinImpl->hFile = hFile; > pPinImpl->bFlushing = FALSE; > pPinImpl->sample_list = NULL; > pPinImpl->handle_list = NULL; > @@ -965,6 +966,14 @@ static HRESULT FileAsyncReader_Construct(HANDLE hFile, IBaseFilter * pBaseFilter > return hr; > } > > +static HRESULT FileAsyncReader_SetFile(IAsyncReader * iface, HANDLE hFile) > +{ > + FileAsyncReader *This = impl_from_IAsyncReader(iface); > + > + This->hFile = hFile; > + return S_OK; > +} > + Why is this a separate function instead of just setting hFile in FileSource_Load? > /* IAsyncReader */ > > static HRESULT WINAPI FileAsyncReader_QueryInterface(IAsyncReader * iface, REFIID riid, LPVOID * ppv) > -- > 2.11.0 > > >

2 1