April 2014 - Wine-Devel - list.winehq.org

Re: [PATCH 5/5] ddraw/tests: Try to make test_coop_level_mode_set() more reliable.

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=6316 Your paranoid android. === w2000pro (32 bit ddraw2) === The test timed out === w2000pro (32 bit ddraw1) === The test timed out === w2000pro (32 bit ddraw7) === The test timed out === w2000pro (32 bit ddraw4) === The test timed out

April 14, 2014

Re: [advapi32] add stub for GetTraceLoggerHandle (try 2)

by Nikolay Sivov

On 4/14/2014 12:11, Vijay Kiran Kamuju wrote: > +TRACEHANDLE WINAPI GetTraceLoggerHandle( PVOID lpBuffer ) > +{ > + FIXME("(%p) stub\n", lpBuffer); > + return ERROR_INVALID_PARAMETER; > +} It's still not fixed.

April 14, 2014

Re: [advapi32] add stub for GetTraceLoggerHandle (try 2)

by Dmitry Timoshkov

Vijay Kiran Kamuju <infyquest(a)gmail.com> wrote: > +TRACEHANDLE WINAPI GetTraceLoggerHandle( PVOID lpBuffer ) > +{ > + FIXME("(%p) stub\n", lpBuffer); > + return ERROR_INVALID_PARAMETER; > +} What this API is supposed to return? -- Dmitry.

April 14, 2014

Re: typo fixes

by Nikolay Sivov

> +++ b/documentation/ChangeLog.ALPHA This file doesn't exist any more, so you need to rebase.

April 14, 2014

Re: typo fixes

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=6317 Your paranoid android. === build (build) === Patch failed to apply

April 14, 2014

typo fixes

by Veres Lajos

Hi, I attached a patch with a few typo fixes. I created it a few months ago... I hope it applies easily. Thanks! Best regards, -- Veres Lajos vlajos(a)gmail.com +36 20 438 5909

April 13, 2014

Re: [advapi32] Add stub for GetTraceLoggerHandle

by Vijay Kiran Kamuju

Yeah, I forgot to edit the comments. If the function fails, it returns INVALID_HANDLE_VALUE. To get extended error information, call theGetLastError function. Please check the msdn link below http://msdn.microsoft.com/en-us/library/windows/desktop/aa363897(v=vs.85).a… Thanks, Vijay On Sun, Apr 13, 2014 at 1:57 PM, Nikolay Sivov <bunglehead(a)gmail.com> wrote: >> >> /****************************************************************************** >> + * EnableTrace [ADVAPI32.@] >> + */ >> +TRACEHANDLE WINAPI GetTraceLoggerHandle( PVOID lpBuffer ) >> +{ >> + FIXME("(%p) stub\n", lpBuffer); >> + return ERROR_INVALID_PARAMETER; >> +} > > It returns trace handle, not an error code. Comment is wrong too. > >

April 13, 2014

Re: [advapi32] Add stub for GetTraceLoggerHandle

by Vijay Kiran Kamuju

Ooops my mistake forgot to edit some other stuff This api retrieves the handle of the event tracing session. http://msdn.microsoft.com/en-us/library/windows/desktop/aa363897(v=vs.85).a… This fixes the http://bugs.winehq.org/show_bug.cgi?id=35835 On Sun, Apr 13, 2014 at 2:03 PM, Dmitry Timoshkov <dmitry(a)baikal.ru> wrote: > Vijay Kiran Kamuju <infyquest(a)gmail.com> wrote: > >> /****************************************************************************** >> + * EnableTrace [ADVAPI32.@] >> + */ > > What does this belong to? > >> +TRACEHANDLE WINAPI GetTraceLoggerHandle( PVOID lpBuffer ) >> +{ >> + FIXME("(%p) stub\n", lpBuffer); >> + return ERROR_INVALID_PARAMETER; >> +} > > What this API is supposed to return? > >> + >> +ULONG WINAPI EnableTrace( ULONG enable, ULONG flag, ULONG level, LPCGUID guid, TRACEHANDLE hSession ) >> +/****************************************************************************** > > What's that? > > -- > Dmitry.

April 13, 2014

Re: [advapi32] Add stub for GetTraceLoggerHandle

by Dmitry Timoshkov

Vijay Kiran Kamuju <infyquest(a)gmail.com> wrote: > /****************************************************************************** > + * EnableTrace [ADVAPI32.@] > + */ What does this belong to? > +TRACEHANDLE WINAPI GetTraceLoggerHandle( PVOID lpBuffer ) > +{ > + FIXME("(%p) stub\n", lpBuffer); > + return ERROR_INVALID_PARAMETER; > +} What this API is supposed to return? > + > +ULONG WINAPI EnableTrace( ULONG enable, ULONG flag, ULONG level, LPCGUID guid, TRACEHANDLE hSession ) > +/****************************************************************************** What's that? -- Dmitry.

April 13, 2014

Re: [advapi32] Add stub for GetTraceLoggerHandle

by Nikolay Sivov

> /****************************************************************************** > + * EnableTrace [ADVAPI32.@] > + */ > +TRACEHANDLE WINAPI GetTraceLoggerHandle( PVOID lpBuffer ) > +{ > + FIXME("(%p) stub\n", lpBuffer); > + return ERROR_INVALID_PARAMETER; > +} It returns trace handle, not an error code. Comment is wrong too.

April 13, 2014

Recent 64-bit kernel breaks Win16 apps

by Jan Janecek

See the discussion on LKML: http://lkml.org/lkml/2014/4/11/514

April 12, 2014

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 3

by Nikolay Sivov

On 4/12/2014 07:33, Jeff Latimer wrote: > +static const IWMMetadataEditorVtbl VT_Editor = { > + WMCreateEditor_QueryInterface, > + WMCreateEditor_AddRef, > + WMCreateEditor_Release, > + WMCreateEditor_open, > + WMCreateEditor_flush, > + WMCreateEditor_close > +}; Please use better naming, for method prefix it could be WMMetadataEditor_, for vtable - WMMetadataEditorVtbl. > +HRESULT WINAPI WMCreateEditor_flush(IWMMetadataEditor *iface) All these should be static. > HRESULT WINAPI WMCreateEditor(IWMMetadataEditor **editor) > { > - FIXME("(%p): stub\n", editor); > + WMMetadataEditor *metadata_editor; > + TRACE("(%p)\n", editor); > > - *editor = NULL; You're removing important like for out pointer initialization.

April 12, 2014

Re: [1/2]include: Implement part of nserror.h - Try 3

by Nikolay Sivov

> +#define NS_S_CALLPENDING_HRESULT_TYPEDEF_(0x000D0000L) I don't think you need L suffix at all.

April 12, 2014

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 3

by Nikolay Sivov

On 4/12/2014 07:33, Jeff Latimer wrote: > Free the implementation rather that the interface > --- > dlls/wmvcore/wmvcore_main.c | 99 > +++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 96 insertions(+), 3 deletions(-) > +HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface) > +{ > + WMMetadataEditor *This = impl_from_IWMMetadataEditor(iface); > + FIXME("(%p)\n", iface); > + heap_free(This); > + return S_OK; > +} > + > +static ULONG WINAPI WMCreateEditor_Release(IWMMetadataEditor *iface) > +{ > + WMMetadataEditor *This = impl_from_IWMMetadataEditor(iface); > + ULONG ref = InterlockedDecrement(&This->ref); > + TRACE("(%p) ref=%d\n", iface, ref); > + > + if (ref == 0) WMCreateEditor_close(iface); > + return ref; > +} > + > +HRESULT WINAPI WMCreateEditor_open(IWMMetadataEditor *iface, const WCHAR *filename) > +{ > + if (!filename) return E_POINTER; > + FIXME("(%p) filename=%s\n", iface, debugstr_w(filename)); > + return NS_E_FILE_OPEN_FAILED; > +} > + > +HRESULT WINAPI WMCreateEditor_flush(IWMMetadataEditor *iface) > +{ > + FIXME("(%p)\n", iface); > + HeapFree(GetProcessHeap(), 0, iface); > + return S_OK; > +} > + This is wrong. You can't free implementation pointer like that, only Release() should do it. Also please use natural interface method order for implementation functions and don't make them call each other. So close(), open() and flush() should all return E_NOTIMPL, and have a 'stub' in a fixme message.

April 12, 2014

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 3

by Dmitry Timoshkov

Qian Hong <fracting(a)gmail.com> wrote: > >> +HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface) > >> +{ > >> + WMMetadataEditor *This = impl_from_IWMMetadataEditor(iface); > >> + FIXME("(%p)\n", iface); > >> + heap_free(This); > >> + return S_OK; > >> +} > > > > Why not pass 'This' to WMCreateEditor_close() directly? Why a FIXME? > > There is still a stray empty line after WMCreateEditor(). > > > > I think it is correct to pass 'iface' to WMCreateEditor_close, which > is a public interface: > http://msdn.microsoft.com/en-us/library/windows/desktop/dd757235(v=vs.85).a… Well, I had an impression that it's just an internal helper, thanks for the clarification. In that case freeing the implementation looks wrong. -- Dmitry.

April 12, 2014

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 3

by Qian Hong

Hi Jeff, On Sat, Apr 12, 2014 at 11:57 AM, Dmitry Timoshkov <dmitry(a)baikal.ru> wrote: > Jeff Latimer <lats(a)yless4u.com.au> wrote: > >> +HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface) >> +{ >> + WMMetadataEditor *This = impl_from_IWMMetadataEditor(iface); >> + FIXME("(%p)\n", iface); >> + heap_free(This); >> + return S_OK; >> +} > > Why not pass 'This' to WMCreateEditor_close() directly? Why a FIXME? > There is still a stray empty line after WMCreateEditor(). > I think it is correct to pass 'iface' to WMCreateEditor_close, which is a public interface: http://msdn.microsoft.com/en-us/library/windows/desktop/dd757235(v=vs.85).a… However, I doubt it is correct to free the object in either ::Close() or ::Flush(), do you have any evidence/testcase showing that the current implementation is correct? It seems we should free the object in ::Release() but not in ::Close() and ::Flush() -- Regards, Qian Hong - http://www.winehq.org

April 12, 2014

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 3

by Dmitry Timoshkov

Jeff Latimer <lats(a)yless4u.com.au> wrote: > +HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface) > +{ > + WMMetadataEditor *This = impl_from_IWMMetadataEditor(iface); > + FIXME("(%p)\n", iface); > + heap_free(This); > + return S_OK; > +} Why not pass 'This' to WMCreateEditor_close() directly? Why a FIXME? There is still a stray empty line after WMCreateEditor(). -- Dmitry.

April 12, 2014

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 3

by Qian Hong

Hi Jeff, Thanks for the improvement, but it seems you forgot to remove the empty line.

April 12, 2014

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 2

by Jeff Latimer

I had already sent Try 2 before I read your comments. Try 3 should fix that. On 04/12/2014 01:28 PM, Dmitry Timoshkov wrote: > Jeff Latimer <lats(a)yless4u.com.au> wrote: > >> +HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface) >> +{ >> + FIXME("iface=%p\n", iface); >> + HeapFree(GetProcessHeap(), 0, iface); >> + return S_OK; >> +} > Still doesn't free the right thing. >

April 12, 2014

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 2

by Dmitry Timoshkov

Jeff Latimer <lats(a)yless4u.com.au> wrote: > +HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface) > +{ > + FIXME("iface=%p\n", iface); > + HeapFree(GetProcessHeap(), 0, iface); > + return S_OK; > +} Still doesn't free the right thing. -- Dmitry.

April 12, 2014

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation

by Dmitry Timoshkov

Jeff Latimer <lats(a)yless4u.com.au> wrote: > +HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface) > +{ > + FIXME("iface=%p\n", iface); > + HeapFree(GetProcessHeap(), 0, iface); > + return S_OK; > +} WMCreateEditor_close() should free the implementation not an interface pointer (although they accidently happen to be the same because of implementation structure layout, but once somebody moves/adds fields that may change and lead to unpredictable results). -- Dmitry.

April 12, 2014

Re: [1/2]include: Implement part of nserror.h

by Qian Hong

Hi Jeff, HRESULT WINAPI WMCreateEditor(IWMMetadataEditor **editor) { + WMMetadataEditor *metadata_editor; FIXME("(%p): stub\n", editor); Looks like now we need to use TRACE instead of FIXME here because WMCreateEditor is implemented. - *editor = NULL; + metadata_editor = HeapAlloc(GetProcessHeap(), 0, sizeof(WMMetadataEditor)); + if (!metadata_editor) return E_OUTOFMEMORY; - return E_NOTIMPL; + metadata_editor->IWMMetadataEditor_iface.lpVtbl = &VT_Editor; + metadata_editor->ref = 0; Looks like ref count should be 1 instead of 0. + *editor = &metadata_editor->IWMMetadataEditor_iface; + + return S_OK; } + Looks like this empty line is unwanted. -- Regards, Qian Hong - http://www.winehq.org

April 12, 2014

Re: Implementing (parts of) BindImageEx

by Bernhard Reiter

Am 2014-04-11 00:08, schrieb Bernhard Reiter: > Am 2014-04-10 23:32, schrieb André Hentschel: > >> Have you tried duplicating those functions and adjust them so they don't collide with something? (just as a proof that this will work?) >> > I'm afraid I don't quite understand what you mean by duplicating and > adjusting. (Maybe I should've mentioned that I'm actually pretty much of > a Wine (and Windows API, and PE format) newbie that still has to figure > out his way through theses things; especially all that RVAing still > seems pretty complex to me, and I don't really know how much of it I > need to apply when I'm not going to actually call those imported > functions...) > > Anyway, any help very much appreciated! > > Bernhard Okay, I think have a working stub. I'll post back later after some more polishing.

April 11, 2014

Re: include: Add dvoice.h

by Alistair Leslie-Hughes

On 11/04/2014 4:52 PM, Dmitry Timoshkov wrote: > Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote: > >> Changelog: >> include: Add dvoice.h > > Isn't this supposed to be generated from an .idl? Also which PSDK version > has this header? At least 7.1 doesn't contain it. The original file SDK doesn't have a IDL for this so I thought it wasn't needed. The header can be found in "Microsoft DirectX SDK (August 2007)" Best Regards Alistair Leslie-Hughes

April 11, 2014

Re: [1/3] taskschd/tests: Add some tests for task registration info.

by Dmitry Timoshkov

Nikolay Sivov <bunglehead(a)gmail.com> wrote: > >> For other tests when it returns NULL bstr it > >> would be cleaner to set pointer to some non-zero value to test that each > >> method actually resets it to NULL. > > That's a matter of taste. > Not really. You don't know if it's changed or not after the call if you > don't know what initial value was. > For series of calls like these: > > --- > > + hr = IRegistrationInfo_get_Documentation(reginfo, &bstr); > +todo_wine > + ok(hr == S_OK, "get_Documentation error %#x\n", hr); > +if (hr == S_OK) > + ok(!bstr, "expected NULL, got %s\n", wine_dbgstr_w(bstr)); > + hr = IRegistrationInfo_get_URI(reginfo, &bstr); > +todo_wine > + ok(hr == S_OK, "get_URI error %#x\n", hr); > +if (hr == S_OK) > + ok(!bstr, "expected NULL, got %s\n", wine_dbgstr_w(bstr)); > --- > > there's no way to tell did get_URI set it to NULL or not. In the case of error I agree, but if the method returned S_OK that means that it really returned something. -- Dmitry.

April 11, 2014