Re: [PATCH 3/4] dmband: Remove the extraneous IUnknown from DMBandTrack

[Jacek Caban]

Hi Michael,

On 06/26/15 00:18, Michael Stefaniuc wrote:
> @@ -97,22 +41,47 @@ static inline IDirectMusicBandTrack *impl_from_IDirectMusicTrack8(IDirectMusicTr
>  }
>
> static HRESULT WINAPI IDirectMusicTrack8Impl_QueryInterface(IDirectMusicTrack8 *iface, REFIID riid,
> -
>    void **ppobj)
> -
>    void **ret_iface)
>
> {
>      IDirectMusicBandTrack *This = impl_from_IDirectMusicTrack8(iface);
> - return IUnknown_QueryInterface ((LPUNKNOWN)&This->UnknownVtbl, riid, ppobj);
> -
> - if (IsEqualIID(riid, &IID_IUnknown) || IsEqualIID(riid, &IID_IDirectMusicTrack) ||
> -
>        IsEqualIID(riid, &IID_IDirectMusicTrack8))
> -
>    *ret_iface = iface;
> - else if (IsEqualIID(riid, &IID_IPersistStream))
> -
>    *ret_iface = &This->PersistStreamVtbl;
> - else {
> -
>    WARN("(%p, %s, %p): not found\n", This, debugstr_dmguid(riid), ret_iface);
> -
>    return E_NOINTERFACE;
> - }

The patch is obviously fine and the problem exists in current code, but
QI is supposed to set output interface on error and such assumption is
common for callers. It would be nice to fix that when you rewrite QI ;)

Cheers,
Jacek

[PATCH 3/4] dmband: Remove the extraneous IUnknown from DMBandTrack

[Michael Stefaniuc]

On 06/26/2015 12:39 PM, Jacek Caban wrote:
> Hi Michael,
>
> On 06/26/15 00:18, Michael Stefaniuc wrote:
> > @@ -97,22 +41,47 @@ static inline IDirectMusicBandTrack *impl_from_IDirectMusicTrack8(IDirectMusicTr
> >  }
> >
> > static HRESULT WINAPI IDirectMusicTrack8Impl_QueryInterface(IDirectMusicTrack8 *iface, REFIID riid,
> > -
> >    void **ppobj)
> > -
> >    void **ret_iface)
> >
> > {
> >      IDirectMusicBandTrack *This = impl_from_IDirectMusicTrack8(iface);
> > - return IUnknown_QueryInterface ((LPUNKNOWN)&This->UnknownVtbl, riid, ppobj);
> > -
> > - if (IsEqualIID(riid, &IID_IUnknown) || IsEqualIID(riid, &IID_IDirectMusicTrack) ||
> > -
> >        IsEqualIID(riid, &IID_IDirectMusicTrack8))
> > -
> >    *ret_iface = iface;
> > - else if (IsEqualIID(riid, &IID_IPersistStream))
> > -
> >    *ret_iface = &This->PersistStreamVtbl;
> > - else {
> > -
> >    WARN("(%p, %s, %p): not found\n", This, debugstr_dmguid(riid), ret_iface);
> > -
> >    return E_NOINTERFACE;
> > - }
>
> The patch is obviously fine and the problem exists in current code, but
> QI is supposed to set output interface on error and such assumption is
> common for callers. It would be nice to fix that when you rewrite QI ;)

Thanks Jacek,

that was just a copy and paste fail on my site as I copied it from
band.c and there I did it correctly.

bye
    michael