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

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 3
by Nikolay Sivov 12 Apr '14

12 Apr '14

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.

1 0

Re: [1/2]include: Implement part of nserror.h - Try 3
by Nikolay Sivov 12 Apr '14

12 Apr '14

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

1 0

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 3
by Nikolay Sivov 12 Apr '14

12 Apr '14

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.

1 0

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 3
by Dmitry Timoshkov 12 Apr '14

12 Apr '14

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.

2 2

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 3
by Qian Hong 11 Apr '14

11 Apr '14

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

1 0

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation - Try 2
by Dmitry Timoshkov 11 Apr '14

11 Apr '14

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.

2 1

Re: [2/2]wmvcore: add WMMetadataEditor stub implementation
by Dmitry Timoshkov 11 Apr '14

11 Apr '14

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.

1 0

Re: [1/2]include: Implement part of nserror.h
by Qian Hong 11 Apr '14

11 Apr '14

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

1 0

Implementing (parts of) BindImageEx
by Bernhard Reiter 11 Apr '14

11 Apr '14

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 I'm trying to implement parts of imagehlp's BindImageEx so that at least freezing Python scripts to .exe files (as done by cx_Freeze or py2exe) works -- see [1] (from whose comments #24 and #25 most of this message is copied), and their implementations of the StatusRoutine function [2, 3]. In both cases, BindImageEx is called with BIND_NO_BOUND_IMPORTS | BIND_NO_UPDATE | BIND_ALL_IMAGES as Flags, which I think means that no actual changes to the image's virtual address table are done, but they're only used to determine which DLLs are used by the image. So for cx_Freeze, it would be apparently enough for Wine's BindImageEx stub to iterate over all DLLs whose functions are used by the image, i.e. the exe or dll passed as the ImageName parameter, and call StatusRoutine with the Reason parameter set to BindImportModule, and DllName set to the DLL, while the "Parameter" and "virtualAddress" arguments aren't required for it to work. Py2exe also requires StatusRoutine called with BindImportProcedure as Reason, and will in that case check if Parameter is set to PyImport_ImportModule before it does something, so I think there should be another loop iterating over a DLL's functions, nested in the one mentioned above, and which invokes StatusRoutine with Status set to BindImportProcedure, and with the current function's name as Parameter. I think I have found out the crucial function required to implement this to be ImageDirectoryEntryToDataEx [4] (with dir set to IMAGE_DIRECTORY_ENTRY_IMPORT). Similar iterations like the ones sketched here seem to be found in fixup_imports() (in dlls/ntdll/loader.c) or dump_dir_imported_functions() (in tools/winedump/pe.c, though that isn't using win API functions). So I've tried to implement what I've described above, but unfortunately, I can't seem to figure out the very first part required: obtaining something that can be used as the "base" parameter for ImageDirectoryEntryToDataEx from the ImageName string passed to BindImageEx. I've tried LoadLibraryA(ImageName), LoadLibraryExA(ImageName, NULL, LOAD_LIBRARY_AS_DATAFILE), and MapAndLoad(), but unfortunately, all of them seem to either import all the PE's symbols in a fashion that collides with the already loaded ones (in particular in a python-freezing context, like running "wine C:\\Python27\\python.exe setup.py build" with the test case attached to [1]), or not load them in a way that can be used for ImageDirectoryEntryToDataEx. I'm hoping for someone to help me figure out how to do this... Bernhard [1] http://bugs.winehq.org/show_bug.cgi?id=3591 [2] https://bitbucket.org/anthony_tuininga/cx_freeze/src/de3e8865616a1048d17e95… [3] http://sourceforge.net/p/py2exe/svn/HEAD/tree/trunk/py2exe/source/py2exe_ut… [4] http://source.winehq.org/WineAPI/.html -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBCAAGBQJTRa6OAAoJEKYfw19Gd1lo210P/jfWL7wxK1xDviOur3tXhGfE xPduK9CzkirMVKhSCzmCMEeW7o+ZUulOaAyUkpPVc+H9odiiTLM+4KbEp6z9P4FH 3egzgUB84+a+twZGBgTTvlEeZ5XXsvsTy3MCEiMWNbU3NUmU0DMDO2oAGP2XRG9B 55mfggCTaGQhZL11P6zYyNmb1x+oNK5eLdrYshLPuRa7fbnErtA4zYGc+tGbJBBX mTouKETgGYl+2UoYRZzxctXhsOL0AtUST75hbkh+vvD5J/e/ekYRuHVmmQ91mpbv Wdn1FnQFsTjMvdN0S8frWLFNi7gWcysItoHo9xx65E5hfIXq1tuV9VPcJTYwc+4D op8o8BHno45/K3J1VjJfqbWv4fWI7R0dQTO+igARtszux8r5zt0Wla+8TYly152w ozRMqNC7w+u8JSIPmCTL4mU8+ZwDL+Rl41lRNRVMEda/8E4EGnmwBEo6jK/NvLUJ 1/FXizp7EeNKrMLcANmQF9t/WG4xWKVTrpjWwmOVgu2tdZ099u9LqefjOxRZ0vVC qJO0PJmtlhnZsiBUEIk/x8xZ8KlXmG15FYGx/QI9phn9B5jYks7J++Xzn3PmxlqE YadhCZv5RkK/SLNxiGH726rq0dGwS7NSx3aLNaW5Noj2OuDzj57BZr1nxVGmpezy d0kdLN2Il8I/XGgezhao =wM12 -----END PGP SIGNATURE-----

2 3

Re: include: Add dvoice.h
by Dmitry Timoshkov 11 Apr '14

11 Apr '14

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. -- Dmitry.

2 1