Re: [v2 PATCH] dmime: Query the Sound buffer for supported interfaces

[Michael Stefaniuc]

Hello Alistair,

On 06/28/2017 08:46 AM, Alistair Leslie-Hughes wrote:
> v2: Fix compile warning.
> Signed-off-by: Alistair Leslie-Hughes
>
> dlls/dmime/audiopath.c   | 18 +++++++-----------
>  dlls/dmime/tests/dmime.c | 23 +++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/dlls/dmime/audiopath.c b/dlls/dmime/audiopath.c
> index 04f0af03d0..211f2156f2 100644
> --- a/dlls/dmime/audiopath.c
> +++ b/dlls/dmime/audiopath.c
> @@ -137,17 +137,13 @@ static HRESULT WINAPI IDirectMusicAudioPathImpl_GetObjectInPath (IDirectMusicAud
>          case DMUS_PATH_BUFFER:
>            if (This->pDSBuffer)
>            {
> -
>        if (IsEqualIID (iidInterface, &IID_IDirectSoundBuffer8)) {
> -
>          IDirectSoundBuffer8_QueryInterface (This->pDSBuffer, &IID_IDirectSoundBuffer8, ppObject);
> -
>          TRACE("returning %p\n",*ppObject);
> -
>          return S_OK;
> -
>        } else if (IsEqualIID (iidInterface, &IID_IDirectSound3DBuffer)) {
> -
>          IDirectSoundBuffer8_QueryInterface (This->pDSBuffer, &IID_IDirectSound3DBuffer, ppObject);
> -
>          TRACE("returning %p\n",*ppObject);
> -
>          return S_OK;
> -
>        } else {
> -
>          FIXME("Bad iid\n");
> -
>        }

you need to add tests for all the interfaces supported by the
DirectSoundBuffer object too: IID_IDirectSoundNotify,
IID_IDirectSound3DBuffer, IID_IKsPropertySet.

But if native supports the query for those too this can be simplified
even further to a
    return IDirectSoundBuffer8_QueryInterface(This->pDSBuffer,
iidInterface, ppObject);

The extra tracing isn't really needed as
IDirectSoundBuffer8_QueryInterface() already does that. And any serious
dmusic tracing needs to include the dsound debug channels anyway.
> -
>          if(IDirectSoundBuffer8_QueryInterface (This->pDSBuffer, iidInterface, ppObject) == S_OK) {

Just in case you insist on keeping the extra tracing:
"if" is not a function so please have a space between it and the "(".
And no space between the function name and "(".
> -
>              TRACE("returning %p\n",*ppObject);

Please add a space after the ","

thanks
bye
    michael
> -
>              return S_OK;
> -
>          }
> -
> -
>          WARN("Unsupported iid %s\n", debugstr_guid(iidInterface));
> -
>          return E_NOINTERFACE;
>      }
>      break;
>
> diff --git a/dlls/dmime/tests/dmime.c b/dlls/dmime/tests/dmime.c
> index a154eac50b..13c6afbfec 100644
> --- a/dlls/dmime/tests/dmime.c
> +++ b/dlls/dmime/tests/dmime.c
> @@ -23,6 +23,7 @@
>  #include <wine/test.h>
>  #include <dmusici.h>
>  #include <audioclient.h>
> +#include <guiddef.h>
>
> #define ARRAY_SIZE(a) (sizeof(a)/sizeof((a)[0]))
>
> @@ -45,8 +46,11 @@ static void test_COM_audiopath(void)
>      IDirectMusicAudioPath *dmap;
>      IUnknown *unk;
>      IDirectMusicPerformance8 *performance;
> -
>
> IDirectSoundBuffer *dsound;
> -
>
> IDirectSoundBuffer8 *dsound8;
> ULONG refcount;
> HRESULT hr;
> -
>
> DWORD buffer = 0;
>
> hr = CoCreateInstance(&CLSID_DirectMusicPerformance, NULL, CLSCTX_INPROC_SERVER,
>         &IID_IDirectMusicPerformance8, (void**)&performance);
>
> @@ -89,6 +93,25 @@ static void test_COM_audiopath(void)
>      ok(refcount == 5, "refcount == %u, expected 5\n", refcount);
>      refcount = IUnknown_Release(unk);
> - hr = IDirectMusicAudioPath_GetObjectInPath(dmap, DMUS_PCHANNEL_ALL, DMUS_PATH_BUFFER, buffer, &GUID_NULL,
> -
>            0, &IID_IDirectSoundBuffer, (void**)&dsound);
> - ok(hr == S_OK, "Failed: %08x\n", hr);
> - IDirectSoundBuffer_Release(dsound);
> -
> - hr = IDirectMusicAudioPath_GetObjectInPath(dmap, DMUS_PCHANNEL_ALL, DMUS_PATH_BUFFER, buffer, &GUID_NULL,
> -
>            0, &IID_IDirectSoundBuffer8, (void**)&dsound8);
> - ok(hr == S_OK, "Failed: %08x\n", hr);
> - IDirectSoundBuffer8_Release(dsound8);
> -
> - hr = IDirectMusicAudioPath_GetObjectInPath(dmap, DMUS_PCHANNEL_ALL, DMUS_PATH_BUFFER, buffer, &GUID_NULL,
> -
>            0, &IID_IUnknown, (void**)&unk);
> - ok(hr == S_OK, "Failed: %08x\n", hr);
> - IUnknown_Release(unk);
> -
> - hr = IDirectMusicAudioPath_GetObjectInPath(dmap, DMUS_PCHANNEL_ALL, DMUS_PATH_BUFFER, buffer, &GUID_NULL,
> -
>            0, &GUID_NULL, (void**)&dsound8);
> - ok(hr == E_NOINTERFACE, "Failed: %08x\n", hr);
> - while (IDirectMusicAudioPath_Release(dmap) > 1); /* performance has a reference too */
>    IDirectMusicPerformance8_CloseDown(performance);
>    IDirectMusicPerformance8_Release(performance);