Signed-off-by: Jactry Zeng jzeng@codeweavers.com --- dlls/comsvcs/comsvcs_private.h | 2 + dlls/comsvcs/property.c | 277 ++++++++++++++++++++++++++++++++- dlls/comsvcs/tests/property.c | 122 +++++++++++++++ 3 files changed, 399 insertions(+), 2 deletions(-)
diff --git a/dlls/comsvcs/comsvcs_private.h b/dlls/comsvcs/comsvcs_private.h index 8a67bcffd8..eee6e9938c 100644 --- a/dlls/comsvcs/comsvcs_private.h +++ b/dlls/comsvcs/comsvcs_private.h @@ -37,6 +37,7 @@ enum tid_t { NULL_tid, ISharedPropertyGroupManager_tid, + ISharedPropertyGroup_tid, LAST_tid };
@@ -44,6 +45,7 @@ static REFIID tid_ids[] = { &IID_NULL, &IID_ISharedPropertyGroupManager, + &IID_ISharedPropertyGroup, };
HRESULT get_typeinfo(enum tid_t tid, ITypeInfo **typeinfo); diff --git a/dlls/comsvcs/property.c b/dlls/comsvcs/property.c index 0fc78e233f..6026fd8cc1 100644 --- a/dlls/comsvcs/property.c +++ b/dlls/comsvcs/property.c @@ -20,9 +20,21 @@
WINE_DEFAULT_DEBUG_CHANNEL(comsvcs);
+struct property_group +{ + ISharedPropertyGroup ISharedPropertyGroup_iface; + ISharedPropertyGroupManager *parent; + LONG isolation, release; + LONG refcount; + BSTR name; +}; + struct group_manager { ISharedPropertyGroupManager ISharedPropertyGroupManager_iface; + struct property_group *property_groups; + size_t capacity; + size_t count; LONG refcount; };
@@ -33,6 +45,200 @@ static inline struct group_manager *impl_from_ISharedPropertyGroupManager(IShare return CONTAINING_RECORD(iface, struct group_manager, ISharedPropertyGroupManager_iface); }
+static inline struct property_group *impl_from_ISharedPropertyGroup(ISharedPropertyGroup *iface) +{ + return CONTAINING_RECORD(iface, struct property_group, ISharedPropertyGroup_iface); +} + +static inline BOOL comsvcs_array_reserve(void **elements, size_t *capacity, size_t count, size_t size) +{ + size_t new_capacity, max_capacity; + void *new_elements; + + if (count <= *capacity) + return TRUE; + + max_capacity = ~(SIZE_T)0 / size; + if (count > max_capacity) + return FALSE; + + new_capacity = max(4, *capacity); + while (new_capacity < count && new_capacity <= max_capacity / 2) + new_capacity *= 2; + if (new_capacity < count) + new_capacity = max_capacity; + + if (!*elements) + new_elements = heap_alloc_zero(new_capacity * size); + else + new_elements = heap_realloc(*elements, new_capacity * size); + if (!new_elements) + return FALSE; + + *elements = new_elements; + *capacity = new_capacity; + + return TRUE; +} + +static HRESULT WINAPI property_group_QueryInterface(ISharedPropertyGroup *iface, REFIID riid, void **out) +{ + struct property_group *group = impl_from_ISharedPropertyGroup(iface); + + TRACE("iface %p, riid %s, out %p.\n", iface, debugstr_guid(riid), out); + + *out = NULL; + if (IsEqualGUID(riid, &IID_IUnknown) + || IsEqualGUID(riid, &IID_IDispatch) + || IsEqualGUID(riid, &IID_ISharedPropertyGroup)) + { + *out = &group->ISharedPropertyGroup_iface; + IUnknown_AddRef((IUnknown*)*out); + + return S_OK; + } + + WARN("%s not implemented.\n", debugstr_guid(riid)); + return E_NOINTERFACE; +} + +static ULONG WINAPI property_group_AddRef(ISharedPropertyGroup *iface) +{ + struct property_group *group = impl_from_ISharedPropertyGroup(iface); + ULONG refcount = InterlockedIncrement(&group->refcount); + + TRACE("%p increasing refcount to %u.\n", iface, refcount); + + return refcount; +} + +static ULONG WINAPI property_group_Release(ISharedPropertyGroup *iface) +{ + struct property_group *group = impl_from_ISharedPropertyGroup(iface); + ULONG refcount = InterlockedDecrement(&group->refcount); + + TRACE("%p decreasing refcount to %u.\n", iface, refcount); + + if (!refcount) + { + struct group_manager *manager = impl_from_ISharedPropertyGroupManager(group->parent); + size_t count, index; + + SysFreeString(group->name); + + index = group - manager->property_groups; + manager->count--; + count = manager->count - index; + if (count) + memmove(&manager->property_groups[index], &manager->property_groups[index + 1], count * sizeof(*manager->property_groups)); + + ISharedPropertyGroupManager_Release(group->parent); + } + + return refcount; +} + +static HRESULT WINAPI property_group_GetTypeInfoCount(ISharedPropertyGroup *iface, UINT *info) +{ + TRACE("iface %p, info %p.\n", iface, info); + + if (!info) + return E_INVALIDARG; + *info = 1; + return S_OK; +} + +static HRESULT WINAPI property_group_GetTypeInfo(ISharedPropertyGroup *iface, UINT index, LCID lcid, ITypeInfo **info) +{ + HRESULT hr; + + TRACE("iface %p, index %u, lcid %u, info %p.\n", iface, index, lcid, info); + + if (index) + return DISP_E_BADINDEX; + + hr = get_typeinfo(ISharedPropertyGroup_tid, info); + if (SUCCEEDED(hr)) + ITypeInfo_AddRef(*info); + return hr; +} + +static HRESULT WINAPI property_group_GetIDsOfNames(ISharedPropertyGroup *iface, REFIID riid, LPOLESTR *names, UINT count, + LCID lcid, DISPID *dispid) +{ + ITypeInfo *typeinfo; + HRESULT hr; + + TRACE("iface %p, riid %s, names %p, count %u, lcid %u, dispid %p.\n", + iface, debugstr_guid(riid), names, count, lcid, dispid); + + hr = get_typeinfo(ISharedPropertyGroup_tid, &typeinfo); + if (SUCCEEDED(hr)) + { + hr = ITypeInfo_GetIDsOfNames(typeinfo, names, count, dispid); + ITypeInfo_Release(typeinfo); + } + + return hr; +} + +static HRESULT WINAPI property_group_Invoke(ISharedPropertyGroup *iface, DISPID member, REFIID riid, LCID lcid, + WORD flags, DISPPARAMS *params, VARIANT *result, EXCEPINFO *except, UINT *argerr) +{ + ITypeInfo *typeinfo; + HRESULT hr; + + TRACE("iface %p, member %u, riid %s, lcid %u, flags %x, params %p, result %p, except %p, argerr %p.\n", + iface, member, debugstr_guid(riid), lcid, flags, params, result, except, argerr); + + hr = get_typeinfo(ISharedPropertyGroup_tid, &typeinfo); + if (SUCCEEDED(hr)) + hr = ITypeInfo_Invoke(typeinfo, iface, member, flags, params, result, except, argerr); + return hr; +} + +static HRESULT WINAPI property_group_CreatePropertyByPosition(ISharedPropertyGroup *iface, int index, + VARIANT_BOOL *exists, ISharedProperty **property) +{ + FIXME("iface %p, index %d, exisits %p, property %p: stub.\n", + iface, index, exists, property); + return E_NOTIMPL; +} + +static HRESULT WINAPI property_group_get_PropertyByPosition(ISharedPropertyGroup *iface, int index, ISharedProperty **property) +{ + FIXME("iface %p, index %d, property %p: stub.\n", iface, index, property); + return E_NOTIMPL; +} + +static HRESULT WINAPI property_group_CreateProperty(ISharedPropertyGroup *iface, BSTR name, VARIANT_BOOL *exists, + ISharedProperty **property) +{ + FIXME("iface %p, name %s, exists %p, property %p: stub.\n", iface, debugstr_w(name), exists, property); + return E_NOTIMPL; +} + +static HRESULT WINAPI property_group_get_Property(ISharedPropertyGroup *iface, BSTR name, ISharedProperty **property) +{ + FIXME("iface %p, name %s, property %p: stub.\n", iface, debugstr_w(name), property); + return E_NOTIMPL; +} + +static const ISharedPropertyGroupVtbl property_group_vtbl = +{ + property_group_QueryInterface, + property_group_AddRef, + property_group_Release, + property_group_GetTypeInfoCount, + property_group_GetTypeInfo, + property_group_GetIDsOfNames, + property_group_Invoke, + property_group_CreatePropertyByPosition, + property_group_get_PropertyByPosition, + property_group_CreateProperty, + property_group_get_Property, +}; + static HRESULT WINAPI group_manager_QueryInterface(ISharedPropertyGroupManager *iface, REFIID riid, void **out) { struct group_manager *manager = impl_from_ISharedPropertyGroupManager(iface); @@ -73,6 +279,7 @@ static ULONG WINAPI group_manager_Release(ISharedPropertyGroupManager *iface)
if (!refcount) { + heap_free(manager->property_groups); heap_free(manager); group_manager = NULL; } @@ -139,12 +346,68 @@ static HRESULT WINAPI group_manager_Invoke(ISharedPropertyGroupManager *iface, D return hr; }
+static struct property_group *find_propery_group(struct group_manager *manager, BSTR name) +{ + int index; + + for (index = 0; index < manager->count; index++) + { + if (!lstrcmpW(manager->property_groups[index].name, name)) + return &manager->property_groups[index]; + } + return NULL; +} + static HRESULT WINAPI group_manager_CreatePropertyGroup(ISharedPropertyGroupManager *iface, BSTR name, LONG *isolation, LONG *release, VARIANT_BOOL *exists, ISharedPropertyGroup **group) { - FIXME("iface %p, name %s, isolation %p, release %p, exists %p, group %p: stub.\n", + struct group_manager *manager = impl_from_ISharedPropertyGroupManager(iface); + struct property_group *property_group; + HRESULT hr; + + TRACE("iface %p, name %s, isolation %p, release %p, exists %p, group %p.\n", iface, debugstr_w(name), isolation, release, exists, group); - return E_NOTIMPL; + + if (!name) + return E_POINTER; + + if (*isolation > 1 || *release > 1) + return E_INVALIDARG; + + if (*isolation || *release) + FIXME("Unsopported mode: isolation %d, release %d.\n", *isolation, *release); + + property_group = find_propery_group(manager, name); + if (!property_group) + { + if (!comsvcs_array_reserve((void **)&manager->property_groups, &manager->capacity, manager->count + 1, + sizeof(*manager->property_groups))) + return E_OUTOFMEMORY; + + property_group = &manager->property_groups[manager->count]; + property_group->ISharedPropertyGroup_iface.lpVtbl = &property_group_vtbl; + property_group->parent = iface; + property_group->isolation = *isolation; + property_group->release = *release; + property_group->refcount = 1; + property_group->name = SysAllocString(name); + + manager->count++; + *exists = FALSE; + *group = &property_group->ISharedPropertyGroup_iface; + ISharedPropertyGroupManager_AddRef(property_group->parent); + hr = S_OK; + } + else + { + *exists = TRUE; + *isolation = property_group->isolation; + *release = property_group->release; + hr = ISharedPropertyGroup_QueryInterface(&property_group->ISharedPropertyGroup_iface, + &IID_ISharedPropertyGroup, (void **)group); + } + + return hr; }
static HRESULT WINAPI group_manager_get_Group(ISharedPropertyGroupManager *iface, BSTR name, ISharedPropertyGroup **group) @@ -191,6 +454,16 @@ HRESULT WINAPI group_manager_create(IClassFactory *iface, IUnknown *outer, REFII
group_manager->ISharedPropertyGroupManager_iface.lpVtbl = &group_manager_vtbl; group_manager->refcount = 1; + group_manager->capacity = 0; + group_manager->count = 0; + group_manager->property_groups = NULL; + + if (!comsvcs_array_reserve((void **)&group_manager->property_groups, &group_manager->capacity, 2, + sizeof(*group_manager->property_groups))) + { + heap_free(group_manager); + return E_OUTOFMEMORY; + } } hr = ISharedPropertyGroupManager_QueryInterface(&group_manager->ISharedPropertyGroupManager_iface, riid, out);
diff --git a/dlls/comsvcs/tests/property.c b/dlls/comsvcs/tests/property.c index 6d78cd39cd..9573def737 100644 --- a/dlls/comsvcs/tests/property.c +++ b/dlls/comsvcs/tests/property.c @@ -164,11 +164,133 @@ static void test_interfaces(void) ISharedPropertyGroupManager_Release(manager); }
+static void test_property_group(void) +{ + ISharedPropertyGroup *group, *group1; + ISharedPropertyGroupManager *manager; + ULONG refcount, expected_refcount; + LONG isolation, release; + VARIANT_BOOL exists; + IDispatch *dispatch; + HRESULT hr; + BSTR name; + struct test_name_id test_name_ids[] = + { + {L"CreatePropertyByPosition", 0x1}, + {L"PropertyByPosition", 0x2}, + {L"CreateProperty", 0x3}, + {L"Property", 0x4}, + }; + + hr = CoCreateInstance(&CLSID_SharedPropertyGroupManager, NULL, CLSCTX_INPROC_SERVER, + &IID_ISharedPropertyGroupManager, (void **)&manager); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + hr = ISharedPropertyGroupManager_CreatePropertyGroup(manager, NULL, &isolation, &release, &exists, &group); + ok(hr == E_POINTER, "Got hr %#x.\n", hr); + + name = SysAllocString(L"testgroupname"); + isolation = 2; + release = 0; + hr = ISharedPropertyGroupManager_CreatePropertyGroup(manager, name, &isolation, &release, &exists, &group); + ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr); + + /* Crash on Windows */ + if (0) + { + ISharedPropertyGroupManager_CreatePropertyGroup(manager, name, NULL, &release, &exists, &group1); + ISharedPropertyGroupManager_CreatePropertyGroup(manager, name, &isolation, NULL, &exists, &group1); + ISharedPropertyGroupManager_CreatePropertyGroup(manager, name, &isolation, &release, NULL, &group1); + } + + isolation = 0; + release = 2; + hr = ISharedPropertyGroupManager_CreatePropertyGroup(manager, name, &isolation, &release, &exists, &group); + ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr); + + name = SysAllocString(L"testgroupname"); + isolation = 0; + release = 0; + exists = TRUE; + expected_refcount = get_refcount(manager) + 1; + hr = ISharedPropertyGroupManager_CreatePropertyGroup(manager, name, &isolation, &release, &exists, &group); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(!exists, "Got unexpected value %d.\n", exists); + refcount = get_refcount(group); + ok(refcount == 1, "Got unexpected refcount: %d.\n", refcount); + refcount = get_refcount(manager); + 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); + ISharedPropertyGroup_AddRef(group); + refcount = get_refcount(manager); + ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount); + ISharedPropertyGroup_Release(group); + refcount = get_refcount(manager); + ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount); + + /* Create an existing group */ + isolation = 1; + release = 1; + expected_refcount = get_refcount(manager); + hr = ISharedPropertyGroupManager_CreatePropertyGroup(manager, name, &isolation, &release, &exists, &group1); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(!!exists, "Got unexpected value %d.\n", exists); + ok(!isolation, "Got unexpected value %d.\n", isolation); + ok(!release, "Got unexpected value %d.\n", release); + ok(group == group1, "Got unexpected pointer: %p.\n", group1); + refcount = get_refcount(group); + ok(refcount == 2, "Got unexpected refcount: %d.\n", refcount); + refcount = get_refcount(manager); + ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount); + ISharedPropertyGroup_Release(group1); + refcount = get_refcount(group); + ok(refcount == 1, "Got unexpected refcount: %d.\n", refcount); + SysFreeString(name); + + name = SysAllocString(L"testgroupname2"); + isolation = 0; + release = 1; + exists = TRUE; + expected_refcount = get_refcount(manager) + 1; + hr = ISharedPropertyGroupManager_CreatePropertyGroup(manager, name, &isolation, &release, &exists, &group1); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(!exists, "Got unexpected value %d.\n", exists); + refcount = get_refcount(group1); + todo_wine ok(refcount == 2, "Got unexpected refcount: %d.\n", refcount); + refcount = get_refcount(manager); + ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount); + SysFreeString(name); + + hr = ISharedPropertyGroup_QueryInterface(group, &IID_IDispatch, (void **)&dispatch); + ok(hr == S_OK, "Got hr %#x.\n", hr); + TEST_TYPEINFO(dispatch, test_name_ids, ARRAY_SIZE(test_name_ids), &IID_ISharedPropertyGroup); + IDispatch_Release(dispatch); + + expected_refcount = get_refcount(manager); + ISharedPropertyGroup_Release(group1); + refcount = get_refcount(manager); + todo_wine ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount); + expected_refcount = get_refcount(manager) - 1; + ISharedPropertyGroup_Release(group1); + refcount = get_refcount(manager); + todo_wine ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount); + + expected_refcount = get_refcount(manager) - 1; + ISharedPropertyGroup_Release(group); + refcount = get_refcount(manager); + ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount); + + ISharedPropertyGroupManager_Release(manager); +} + START_TEST(property) { CoInitialize(NULL);
test_interfaces(); + test_property_group();
CoUninitialize(); }
On 8/17/20 3:54 PM, Jactry Zeng wrote:
- if (!refcount)
- {
struct group_manager *manager = impl_from_ISharedPropertyGroupManager(group->parent);size_t count, index;SysFreeString(group->name);index = group - manager->property_groups;manager->count--;count = manager->count - index;if (count)memmove(&manager->property_groups[index], &manager->property_groups[index + 1], count * sizeof(*manager->property_groups));ISharedPropertyGroupManager_Release(group->parent);- }
That's not ideal to have back link to manager like that, but I see tests show that it might be happening. It's probably easier to keep a list instead, you can then unlink and release parent, potentially with some locking later. You can also store 'struct group_manager*' directly there, so you don't have to do an impl_from_*().
- name = SysAllocString(L"testgroupname2");
- isolation = 0;
- release = 1;
- exists = TRUE;
- expected_refcount = get_refcount(manager) + 1;
- hr = ISharedPropertyGroupManager_CreatePropertyGroup(manager, name, &isolation, &release, &exists, &group1);
- ok(hr == S_OK, "Got hr %#x.\n", hr);
- ok(!exists, "Got unexpected value %d.\n", exists);
- refcount = get_refcount(group1);
- todo_wine ok(refcount == 2, "Got unexpected refcount: %d.\n", refcount);
- refcount = get_refcount(manager);
- ok(refcount == expected_refcount, "Got refcount: %d, expected %d.\n", refcount, expected_refcount);
- SysFreeString(name);
That's a weird bit. It might be important to try different release modes to see what's going on.
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=77244
Your paranoid android.
=== debiant (32 bit report) ===
comsvcs: property.c:127: Test failed: Got hr 0x80040154. property.c:131: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b34).
Report validation errors: comsvcs:property crashed (c0000005)
=== debiant (32 bit French report) ===
comsvcs: property.c:127: Test failed: Got hr 0x80040154. property.c:131: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b34).
Report validation errors: comsvcs:property crashed (c0000005)
=== debiant (32 bit Japanese:Japan report) ===
comsvcs: property.c:127: Test failed: Got hr 0x80040154. property.c:131: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b34).
Report validation errors: comsvcs:property crashed (c0000005)
=== debiant (32 bit Chinese:China report) ===
comsvcs: property.c:127: Test failed: Got hr 0x80040154. property.c:131: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b34).
Report validation errors: comsvcs:property crashed (c0000005)
=== debiant (32 bit WoW report) ===
comsvcs: property.c:127: Test failed: Got hr 0x80040154. property.c:131: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b34).
Report validation errors: comsvcs:property crashed (c0000005)
=== debiant (64 bit WoW report) ===
comsvcs: property.c:127: Test failed: Got hr 0x80040154. property.c:131: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b34).
Report validation errors: comsvcs:property crashed (c0000005)
-
Jactry Zeng -
Marvin -
Nikolay Sivov