Re: [PATCH v2 0/16] MR4046: windows.media.mediacontrol: Implement some IMusicDisplayProperties and IMusicDisplayProperties2 properties.

[Rémi Bernon (＠rbernon)]

Rémi Bernon (@rbernon) commented about dlls/windows.media.mediacontrol/main.c:
> +{
> - FIXME( "iface %p, value %p stub!\n", iface, value );
> - return E_NOTIMPL;
>
> +}
> -
>
> +static HRESULT WINAPI display_updater_get_MusicProperties( ISystemMediaTransportControlsDisplayUpdater *iface, IMusicDisplayProperties **value )
> +{
> - struct music_properties *impl;
> -
> - TRACE( "iface %p, value %p\n", iface, value );
> -
> - if (!(impl = calloc( 1, sizeof(*impl) ))) return E_OUTOFMEMORY;
> -
> - impl->IMusicDisplayProperties_iface.lpVtbl = &music_properties_vtbl;
> - impl->IMusicDisplayProperties2_iface.lpVtbl = &music_properties2_vtbl;
> - impl->ref = 2;

I missed it previously but I don't think you should initialize ref count to 2. If the tests show that kind of refcount (although I don't think testing this is very useful), then it's very likely that the objects shouldn't be created on demand but rather be created upfront and owned by their parent until it is destroyed.

It is probably also fine to still create them on-demand, but in all cases initialize ref to 1, so that there's no leak when an application release their only reference.