Re: wmvcore: Stub implementation of IWMMetadataEditor interface

[Nikolay Sivov]

On 10/10/2013 14:58, Jeff Latimer wrote:

---
  dlls/wmvcore/Makefile.in|   2 +-
  dlls/wmvcore/wmvcore_main.c | 100
+++-
  include/wmsdkidl.idl|  11 ++---
  3 files changed, 105 insertions(+), 8 deletions(-)
+typedef struct MetadataEditorImpl {
+IWMMetadataEditor MetadataEditor_iface;
+LONG ref;
+} MetadataEditorImpl;
+static inline MetadataEditorImpl *impl_from_MetadataEditor(IWMMetadataEditor 
*iface)

Please follow naming convetions for interfaces.


+HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface)
+{
+FIXME(iface=%p\n, iface);
+HeapFree(GetProcessHeap(), 0, iface);
+return S_OK;
+}
+HRESULT WINAPI WMCreateEditor_flush(IWMMetadataEditor *iface)
+{
+FIXME(iface=%p\n, iface);
+HeapFree(GetProcessHeap(), 0, iface);
+return S_OK;
+}

This looks totally wrong.

Also please keep implementation functions order and interface methods 
order the same.

Re: [2/2] wmvcore: Stub implementation of IWMMetadataEditor interface (try2)

[Nikolay Sivov]

On 3/11/2013 16:36, Jeff Latimer wrote:

+if (!ppv) return E_INVALIDARG;

You don't need this most likely.


+IUnknown_AddRef((IUnknown*)*ppv);

A cleaner way is to AddRef original iface, but that's a matter of taste.


+static ULONG WINAPI WMCreateEditor_AddRef(IWMMetadataEditor *iface)
+{
+FIXME(iface=%p\n, iface);
+return 2;
+}
+
+static ULONG WINAPI WMCreateEditor_Release(IWMMetadataEditor *iface)
+{
+FIXME(iface=%p\n, iface);
+HeapFree(GetProcessHeap(), 0, iface);
+return 1;
+}

What's this supposed to mean?


+HRESULT WINAPI WMCreateEditor_flush(IWMMetadataEditor *iface)
+{
+FIXME(iface=%p\n, iface);
+HeapFree(GetProcessHeap(), 0, iface);
+return S_OK;
+}
+
+HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface)
+{
+FIXME(iface=%p\n, iface);
+HeapFree(GetProcessHeap(), 0, iface);
+return S_OK;
+}

I don't think so.


  HRESULT WINAPI WMCreateEditor(IWMMetadataEditor **editor)
  {
+IWMMetadataEditor *This;
  FIXME((%p): stub\n, editor);
  
-*editor = NULL;

+This = HeapAlloc(GetProcessHeap(), 0, sizeof(IWMMetadataEditor));
+if (!This) return E_OUTOFMEMORY;
  
-return E_NOTIMPL;

+This-lpVtbl = Editor_vtable;
+*editor = This;
+
+return S_OK;
  }

That's not how it's done, there's plenty of examples already.


-};
+}

That change is ok I think.