Re: [PATCH 1/5] comsvcs: Add ISharedPropertyGroupManager stub.

[Nikolay Sivov]

On 8/17/20 3:53 PM, Jactry Zeng wrote:
> +HRESULT WINAPI group_manager_create(IClassFactory *iface, IUnknown *outer, REFIID riid, void **out)
> +{
> - HRESULT hr;
> -
> - if (outer)
> -
>    return CLASS_E_NOAGGREGATION;
> -
> - if (!group_manager)
> - {
> -
>    group_manager = heap_alloc(sizeof(*group_manager));
> -
>    if (!group_manager)
> -
>    {
> -
>        out = NULL;
> -
>        return E_OUTOFMEMORY;
> -
>    }
> -
> -
>    group_manager->ISharedPropertyGroupManager_iface.lpVtbl = &group_manager_vtbl;
> -
>    group_manager->refcount = 1;
> - }
> - hr = ISharedPropertyGroupManager_QueryInterface(&group_manager->ISharedPropertyGroupManager_iface, riid, out);
> -
> - return hr;
>
> +}

If the intent is to have single instance of this object, such
initialization needs to be safe.
> static void test_interfaces(void)
> +{
> - ISharedPropertyGroupManager *manager, *manager1;
> - ULONG refcount, expected_refcount;
> - IDispatch *dispatch;
> - IUnknown *unk;
> - HRESULT hr;
> -
> - hr = CoCreateInstance(&CLSID_SharedPropertyGroupManager, &test_outer, CLSCTX_INPROC_SERVER,
> -
>        &IID_ISharedPropertyGroupManager, (void **)&manager);
> - ok(hr == CLASS_E_NOAGGREGATION, "Got hr %#x.\n", hr);
> -
> - hr = CoCreateInstance(&CLSID_SharedPropertyGroupManager, NULL, CLSCTX_INPROC_SERVER,
> -
>        &IID_ISharedPropertyGroupManager, (void **)&manager);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> -
> - expected_refcount = get_refcount(manager) + 1;
> - hr = CoCreateInstance(&CLSID_SharedPropertyGroupManager, NULL, CLSCTX_INPROC_SERVER,
> -
>        &IID_ISharedPropertyGroupManager, (void **)&manager1);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - refcount = get_refcount(manager1);
> - ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount);
> - refcount = get_refcount(manager);
> - ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount);
> - ISharedPropertyGroupManager_Release(manager1);
> -
> - hr = CoCreateInstance(&CLSID_SharedPropertyGroupManager, NULL, CLSCTX_INPROC_SERVER,
> -
>        &IID_IUnknown, (void **)&unk);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - refcount = get_refcount(unk);
> - ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount);
> - refcount = get_refcount(manager);
> - ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount);
> -
> - expected_refcount++;
> - hr = CoCreateInstance(&CLSID_SharedPropertyGroupManager, NULL, CLSCTX_INPROC_SERVER,
> -
>        &IID_IDispatch, (void **)&dispatch);
> - ok(hr == S_OK, "Got hr %#x.\n", hr);
> - refcount = get_refcount(dispatch);
> - ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount);
> - refcount = get_refcount(manager);
> - ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount);
> -
> - IDispatch_Release(dispatch);
> - IUnknown_Release(unk);
> - ISharedPropertyGroupManager_Release(manager);
>
> +}

You only need to create for let's say IUnknown and query result, you
then can expect that CoCreateInstance() will succeed for supported
interfaces. Does this have a test for single instance?
> - [
> -
>    object,
> -
>    hidden,
> -
>    local,
> -
>    uuid(2a005c01-a5de-11cf-9e66-00aa00a3f464),
> -
>    pointer_default(unique)
> - ]
> - interface ISharedProperty : IDispatch
> - {
> -
>    [id(0x00000000), propget]
> -
>    HRESULT Value([out, retval] VARIANT *value);
> -
>    [id(0x00000000), propput]
> -
>    HRESULT Value([in] VARIANT value);
> - };

This could be DISPID_VALUE. And no need for trailing semicolon for
interfaces.
> - [
> -
>    object,
> -
>    hidden,
> -
>    local,
> -
>    uuid(2a005c0d-a5de-11cf-9e66-00aa00a3f464),
> -
>    pointer_default(unique)
> - ]
> - interface ISharedPropertyGroupManager : IDispatch
> - {
> -
>    [id(0x00000001)]
> -
>    HRESULT CreatePropertyGroup([in] BSTR name, [in, out] LONG *isolation, [in, out] LONG *release,
> -
>            [out] VARIANT_BOOL *exists, [out, retval] ISharedPropertyGroup **group);
> -
>    [id(0x00000002), propget]
> -
>    HRESULT Group([in] BSTR name, [out, retval] ISharedPropertyGroup **group);
> -
>    [id(0xfffffffc), propget]
> -
>    HRESULT _NewEnum([out, retval] IUnknown **retval);
> - };

Similar this one is DISPID_NEWENUM.