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

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

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

[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).as...

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

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

[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).as...

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.