Re: xaudio2_7: Implement IXaudio2 interface as stub (try 4)

[Jacek Caban]

Hi Guillaume,

On 06/08/15 10:06, Guillaume Charifi wrote:
> Fixes https://bugs.winehq.org/show_bug.cgi?id=26808
>
> dlls/xaudio2_7/Makefile.in        |    2 
>  dlls/xaudio2_7/xaudio_dll.c       |  300
> ++++++++++++++++++++++++++++++++++++-
>  dlls/xaudio2_7/xaudio_classes.idl |   28 +++
>  3 files changed, 319 insertions(+), 11 deletions(-)
>
> xaudio_2.7.dll.IXAudio2_stub.patch
>
> diff --git a/dlls/xaudio2_7/Makefile.in b/dlls/xaudio2_7/Makefile.in
> index 853d1d6..6bdf362 100644
> --- a/dlls/xaudio2_7/Makefile.in
> +++ b/dlls/xaudio2_7/Makefile.in
> @@ -3,3 +3,5 @@ IMPORTS   = advapi32 kernel32 ole32 user32 uuid
>
> C_SRCS = \
>    xaudio_dll.c
> -
>
> +IDL_SRCS = xaudio_classes.idl

Adding registration IDL deserves a separated patch.
> diff --git a/dlls/xaudio2_7/xaudio_dll.c b/dlls/xaudio2_7/xaudio_dll.c
> index c8b7904..07c4780 100644
> --- a/dlls/xaudio2_7/xaudio_dll.c
> +++ b/dlls/xaudio2_7/xaudio_dll.c
> @@ -30,12 +30,15 @@
>  #include "wine/debug.h"
>  #include <propsys.h>
>  #include "initguid.h"
> -
>
> #include "xaudio2.h"
>
> WINE_DEFAULT_DEBUG_CHANNEL(xaudio2);
>
> static HINSTANCE instance;
>
> +static HRESULT XAudio2_create(IUnknown *pUnkOuter, LPVOID *ppObj);
> -
>
> BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
>  {
>      TRACE("(%p, %d, %p)\n", hinstDLL, fdwReason, lpvReserved);
> @@ -52,35 +55,310 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
>      return TRUE;
>  }
>
> -HRESULT WINAPI DllCanUnloadNow(void)
> +typedef struct {
> - IClassFactory IClassFactory_iface;
> - HRESULT (*pfnCreateInstance)(IUnknown *pUnkOuter, LPVOID *ppObj);

Please avoid using LP* types. This should be void**, but using
IUnknown** would be even better.
> +} IClassFactoryImpl;
> -
>
> +static inline IClassFactoryImpl *impl_from_IClassFactory(IClassFactory *iface)
>  {
> - return S_FALSE;
> - return CONTAINING_RECORD(iface, IClassFactoryImpl, IClassFactory_iface);
>
> +}
> -
>
> +static HRESULT WINAPI XAudio2CF_QueryInterface(IClassFactory *iface, REFIID riid, void **ppobj)
> +{
> - if(IsEqualGUID(riid, &IID_IUnknown)
> -
>        || IsEqualGUID(riid, &IID_IClassFactory))
> - {
> -
>    IClassFactory_AddRef(iface);
> -
>    *ppobj = iface;
> -
>    return S_OK;
> - }
> -
> - *ppobj = NULL;
> - WARN("(%p)->(%s, %p): interface not found\n", iface, debugstr_guid(riid), ppobj);
> - return E_NOINTERFACE;
>
> +}
> -
>
> +static ULONG WINAPI XAudio2CF_AddRef(IClassFactory *iface)
> +{
> - return 2;
>
> +}
> -
>
> +static ULONG WINAPI XAudio2CF_Release(IClassFactory *iface)
> +{
> - return 1;
>
> }
>
> +static HRESULT WINAPI XAudio2CF_CreateInstance(IClassFactory *iface, IUnknown *pOuter,
> -
>                                           REFIID riid, void **ppobj)
>
> +{
> - IClassFactoryImpl *This = impl_from_IClassFactory(iface);
> - HRESULT hr;
> - LPUNKNOWN punk;

Same here, use IUnknown* instead.
> -
> - TRACE("(%p)->(%p,%s,%p)\n", This, pOuter, debugstr_guid(riid), ppobj);
> -
> - *ppobj = NULL;
> - hr = This->pfnCreateInstance(pOuter, (LPVOID *) &punk);
> - if (FAILED(hr))
> -
>    return hr;
> -
> - hr = IUnknown_QueryInterface(punk, riid, ppobj);
> - IUnknown_Release(punk);
> - return hr;
>
> +}
> -
>
> +static HRESULT WINAPI XAudio2CF_LockServer(IClassFactory *iface, BOOL dolock)
> +{
> - IClassFactoryImpl *This = impl_from_IClassFactory(iface);
> - FIXME("(%p)->(%d): stub!\n", This, dolock);
> - return S_OK;
>
> +}
> -
>
> +static const IClassFactoryVtbl XAudio2CF_Vtbl =
> +{
> - XAudio2CF_QueryInterface,
> - XAudio2CF_AddRef,
> - XAudio2CF_Release,
> - XAudio2CF_CreateInstance,
> - XAudio2CF_LockServer
>
> +};
> -
>
> +static IClassFactoryImpl xaudio2_cf = { { &XAudio2CF_Vtbl }, XAudio2_create };
> -
>
> HRESULT WINAPI DllGetClassObject(REFCLSID rclsid, REFIID riid, LPVOID *ppv)
>  {
> - IClassFactory *factory = NULL;
> - TRACE("(%s, %s, %p)\n", debugstr_guid(rclsid), debugstr_guid(riid), ppv);
> - if (ppv == NULL) {
> -
>    WARN("invalid parameter\n");
> -
>    return E_INVALIDARG;
> - if(IsEqualGUID(rclsid, &CLSID_XAudio2)) {
> -
>    factory = &xaudio2_cf.IClassFactory_iface;

How about simply return IClassFactory_QueryInterface(...) here?
>  }
> - if(!factory) return CLASS_E_CLASSNOTAVAILABLE;
> - *ppv = NULL;
> - return IClassFactory_QueryInterface(factory, riid, ppv);
>
> +}
> - WARN("(%s, %s, %p): no class found.\n", debugstr_guid(rclsid),
> -
>     debugstr_guid(riid), ppv);
> - return CLASS_E_CLASSNOTAVAILABLE;
>
> +HRESULT WINAPI DllCanUnloadNow(void)
> +{
> - return S_FALSE;
>
> }
>
> HRESULT WINAPI DllRegisterServer(void)
>  {
>      TRACE("\n");
> - return __wine_register_resources( instance );
> - return __wine_register_resources(instance);
>
> }
>
> HRESULT WINAPI DllUnregisterServer(void)
>  {
>      TRACE("\n");
> - return __wine_unregister_resources( instance );
> - return __wine_unregister_resources(instance);
>
> +}
> -
>
> +typedef struct {
> - IXAudio2 IXAudio2Impl_iface;
> - LONG ref;
>
> +} IXAudio2Impl;
> -
>
> +static inline IXAudio2Impl *impl_from_IXAudio2(IXAudio2 *iface)
> +{
> - return CONTAINING_RECORD(iface, IXAudio2Impl, IXAudio2Impl_iface);
>
> }
> -
>
> +static const struct IXAudio2Vtbl XAudio2_Vtbl;

This is not needed. Please move XAudio2_create below the vtbl.
> +HRESULT XAudio2_create(IUnknown *pUnkOuter, LPVOID *ppObj)
> +{
> - IXAudio2Impl* object;
> -
> - TRACE("(%p, %p)\n", pUnkOuter, ppObj);
> -
> - if(pUnkOuter)
> -
>    return CLASS_E_NOAGGREGATION;
> -
> - object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(IXAudio2Impl));
> - if(!object)
> -
>    return E_OUTOFMEMORY;
> -
> - object->IXAudio2Impl_iface.lpVtbl = &XAudio2_Vtbl;
> - object->ref = 1;
> -
> - *ppObj = object;

You shouldn't assign object type here, even if the pointer is the same
ans interface. Use &object->IXAudio2Impl_iface instead.
> -
> - return S_OK;
>
> +}
> -
>
> +/*** IUnknown methods ***/
> +static HRESULT WINAPI IXAudio2Impl_QueryInterface(IXAudio2 *iface, REFIID riid, void **ppvObject)
> +{
> - IXAudio2Impl *This = impl_from_IXAudio2(iface);
> -
> - TRACE("(%p)->(%s, %p)\n", This, debugstr_guid(riid), ppvObject);
> -
> - if(IsEqualGUID(riid, &IID_IUnknown) ||
> -
>   IsEqualGUID(riid, &IID_IXAudio2))
> - {
> -
>    IXAudio2_AddRef(iface);
> -
>    *ppvObject = iface;
> -
>    return S_OK;
> - }
> -
> - ERR("(%p)->(%s,%p), not found\n", This,debugstr_guid(riid), ppvObject);

This should be FIXME instead (or WARN, it depends on how likely apps are
to call it with an interface that shouldn't be implemented). Also, QI is
expected to set ppvObject to NULL on error.

Thanks,
Jacek