Signed-off-by: Jactry Zeng jzeng@codeweavers.com
--- v3: - Fix some leaks. - Add more tests. - Check SysAllocString() result in CreateProperty(). v2: No changes. --- dlls/comsvcs/comsvcs_private.h | 2 + dlls/comsvcs/property.c | 250 ++++++++++++++++++++++++++++++++- dlls/comsvcs/tests/property.c | 81 +++++++++++ 3 files changed, 331 insertions(+), 2 deletions(-)
diff --git a/dlls/comsvcs/comsvcs_private.h b/dlls/comsvcs/comsvcs_private.h index eee6e9938c..3eeb9d751a 100644 --- a/dlls/comsvcs/comsvcs_private.h +++ b/dlls/comsvcs/comsvcs_private.h @@ -38,6 +38,7 @@ enum tid_t NULL_tid, ISharedPropertyGroupManager_tid, ISharedPropertyGroup_tid, + ISharedProperty_tid, LAST_tid };
@@ -46,6 +47,7 @@ static REFIID tid_ids[] = &IID_NULL, &IID_ISharedPropertyGroupManager, &IID_ISharedPropertyGroup, + &IID_ISharedProperty, };
HRESULT get_typeinfo(enum tid_t tid, ITypeInfo **typeinfo); diff --git a/dlls/comsvcs/property.c b/dlls/comsvcs/property.c index a73964d288..b2b3920a2d 100644 --- a/dlls/comsvcs/property.c +++ b/dlls/comsvcs/property.c @@ -21,6 +21,13 @@
WINE_DEFAULT_DEBUG_CHANNEL(comsvcs);
+struct shared_property +{ + ISharedProperty ISharedProperty_iface; + LONG refcount; + BSTR name; +}; + struct group_manager { ISharedPropertyGroupManager ISharedPropertyGroupManager_iface; @@ -37,10 +44,44 @@ struct property_group LONG isolation, release; struct list entry; BSTR name; + struct shared_property *properties; + size_t capacity, count; + CRITICAL_SECTION cs; };
static struct group_manager *group_manager = NULL;
+static 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 inline struct group_manager *impl_from_ISharedPropertyGroupManager(ISharedPropertyGroupManager *iface) { return CONTAINING_RECORD(iface, struct group_manager, ISharedPropertyGroupManager_iface); @@ -51,6 +92,139 @@ static inline struct property_group *impl_from_ISharedPropertyGroup(ISharedPrope return CONTAINING_RECORD(iface, struct property_group, ISharedPropertyGroup_iface); }
+static inline struct shared_property *impl_from_ISharedProperty(ISharedProperty *iface) +{ + return CONTAINING_RECORD(iface, struct shared_property, ISharedProperty_iface); +} + +static HRESULT WINAPI shared_property_QueryInterface(ISharedProperty *iface, REFIID riid, void **out) +{ + struct shared_property *property = impl_from_ISharedProperty(iface); + + TRACE("iface %p, riid %s, out %p.\n", iface, debugstr_guid(riid), out); + + if (IsEqualGUID(riid, &IID_IUnknown) + || IsEqualGUID(riid, &IID_IDispatch) + || IsEqualGUID(riid, &IID_ISharedProperty)) + { + *out = &property->ISharedProperty_iface; + IUnknown_AddRef((IUnknown *)*out); + return S_OK; + } + + WARN("%s not implemented.\n", debugstr_guid(riid)); + *out = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI shared_property_AddRef(ISharedProperty *iface) +{ + struct shared_property *property = impl_from_ISharedProperty(iface); + ULONG refcount = InterlockedIncrement(&property->refcount); + + TRACE("%p increasing refcount to %u.\n", iface, refcount); + + return refcount; +} + +static ULONG WINAPI shared_property_Release(ISharedProperty *iface) +{ + struct shared_property *property = impl_from_ISharedProperty(iface); + ULONG refcount = InterlockedDecrement(&property->refcount); + + TRACE("%p decreasing refcount to %u.\n", iface, refcount); + + if (!refcount) + { + SysFreeString(property->name); + } + + return refcount; +} + +static HRESULT WINAPI shared_property_GetTypeInfoCount(ISharedProperty *iface, UINT *info) +{ + TRACE("iface %p, info %p.\n", iface, info); + + if (!info) + return E_INVALIDARG; + *info = 1; + return S_OK; +} + +static HRESULT WINAPI shared_property_GetTypeInfo(ISharedProperty *iface, UINT index, LCID lcid, ITypeInfo **info) +{ + TRACE("iface %p, index %u, lcid %u, info %p.\n", iface, index, lcid, info); + + if (index) + return DISP_E_BADINDEX; + + return get_typeinfo(ISharedProperty_tid, info); +} + +static HRESULT WINAPI shared_property_GetIDsOfNames(ISharedProperty *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(ISharedProperty_tid, &typeinfo); + if (SUCCEEDED(hr)) + { + hr = ITypeInfo_GetIDsOfNames(typeinfo, names, count, dispid); + ITypeInfo_Release(typeinfo); + } + + return hr; +} + +static HRESULT WINAPI shared_property_Invoke(ISharedProperty *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(ISharedProperty_tid, &typeinfo); + if (SUCCEEDED(hr)) + { + hr = ITypeInfo_Invoke(typeinfo, iface, member, flags, params, result, except, argerr); + ITypeInfo_Release(typeinfo); + } + + return hr; +} + +static HRESULT WINAPI shared_property_get_Value(ISharedProperty *iface, VARIANT *value) +{ + FIXME("iface %p, value %p: stub.\n", iface, value); + return E_NOTIMPL; +} + +static HRESULT WINAPI shared_property_put_Value(ISharedProperty *iface, VARIANT value) +{ + FIXME("iface %p, value %s: stub.\n", iface, debugstr_variant(&value)); + return E_NOTIMPL; +} + +static const ISharedPropertyVtbl shared_property_vtbl = +{ + shared_property_QueryInterface, + shared_property_AddRef, + shared_property_Release, + shared_property_GetTypeInfoCount, + shared_property_GetTypeInfo, + shared_property_GetIDsOfNames, + shared_property_Invoke, + shared_property_get_Value, + shared_property_put_Value +}; + static HRESULT WINAPI property_group_QueryInterface(ISharedPropertyGroup *iface, REFIID riid, void **out) { struct property_group *group = impl_from_ISharedPropertyGroup(iface); @@ -93,12 +267,14 @@ static ULONG WINAPI property_group_Release(ISharedPropertyGroup *iface) struct group_manager *manager = group->parent;
SysFreeString(group->name); + heap_free(group->properties);
EnterCriticalSection(&manager->cs); list_remove(&group->entry); LeaveCriticalSection(&manager->cs);
ISharedPropertyGroupManager_Release(&manager->ISharedPropertyGroupManager_iface); + DeleteCriticalSection(&group->cs); heap_free(group); }
@@ -163,6 +339,18 @@ static HRESULT WINAPI property_group_Invoke(ISharedPropertyGroup *iface, DISPID return hr; }
+static struct shared_property *find_shared_property(struct property_group *group, BSTR name) +{ + int index; + + for (index = 0; index < group->count; index++) + { + if (!lstrcmpW(group->properties[index].name, name)) + return &group->properties[index]; + } + return NULL; +} + static HRESULT WINAPI property_group_CreatePropertyByPosition(ISharedPropertyGroup *iface, int index, VARIANT_BOOL *exists, ISharedProperty **property) { @@ -180,8 +368,54 @@ static HRESULT WINAPI property_group_get_PropertyByPosition(ISharedPropertyGroup 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; + struct property_group *group = impl_from_ISharedPropertyGroup(iface); + struct shared_property *shared_property; + BSTR property_name; + HRESULT hr; + + TRACE("iface %p, name %s, exists %p, property %p.\n", iface, debugstr_w(name), exists, property); + + EnterCriticalSection(&group->cs); + + shared_property = find_shared_property(group, name); + if (!shared_property) + { + property_name = SysAllocString(name); + if (!property_name) + { + heap_free(shared_property); + LeaveCriticalSection(&group->cs); + return E_OUTOFMEMORY; + } + + if (!comsvcs_array_reserve((void **)&group->properties, &group->capacity, group->count + 1, + sizeof(*group->properties))) + { + SysFreeString(property_name); + LeaveCriticalSection(&group->cs); + return E_OUTOFMEMORY; + } + + shared_property = &group->properties[group->count]; + shared_property->refcount = 1; + shared_property->name = property_name; + shared_property->ISharedProperty_iface.lpVtbl = &shared_property_vtbl; + + group->count++; + *exists = FALSE; + *property = &shared_property->ISharedProperty_iface; + hr = S_OK; + } + else + { + *exists = TRUE; + hr = ISharedProperty_QueryInterface(&shared_property->ISharedProperty_iface, + &IID_ISharedProperty, (void **)property); + } + + LeaveCriticalSection(&group->cs); + + return hr; }
static HRESULT WINAPI property_group_get_Property(ISharedPropertyGroup *iface, BSTR name, ISharedProperty **property) @@ -349,6 +583,17 @@ static HRESULT WINAPI group_manager_CreatePropertyGroup(ISharedPropertyGroupMana property_group->isolation = *isolation; property_group->release = *release; property_group->refcount = 1; + property_group->capacity = 0; + property_group->count = 0; + property_group->properties = NULL; + if (!comsvcs_array_reserve((void **)&property_group->properties, &property_group->capacity, 2, + sizeof(*property_group->properties))) + { + heap_free(property_group); + LeaveCriticalSection(&manager->cs); + return E_OUTOFMEMORY; + } + property_group->name = SysAllocString(name); if (!property_group->name) { @@ -356,6 +601,7 @@ static HRESULT WINAPI group_manager_CreatePropertyGroup(ISharedPropertyGroupMana heap_free(property_group); return E_OUTOFMEMORY; } + InitializeCriticalSection(&property_group->cs);
list_add_tail(&manager->property_groups, &property_group->entry);
diff --git a/dlls/comsvcs/tests/property.c b/dlls/comsvcs/tests/property.c index 7321bbcfe6..1f62656e7a 100644 --- a/dlls/comsvcs/tests/property.c +++ b/dlls/comsvcs/tests/property.c @@ -320,12 +320,93 @@ static void test_property_group(void) ISharedPropertyGroupManager_Release(manager); }
+static void test_shared_property(void) +{ + ISharedProperty *property, *property1; + ISharedPropertyGroupManager *manager; + ULONG refcount, expected_refcount; + ISharedPropertyGroup *group; + BSTR name, property_name; + LONG isolation, release; + VARIANT_BOOL exists; + IDispatch *dispatch; + HRESULT hr; + static const struct test_name_id test_name_ids[] = + { + {L"Value", DISPID_VALUE}, + }; + + hr = CoCreateInstance(&CLSID_SharedPropertyGroupManager, NULL, CLSCTX_INPROC_SERVER, + &IID_ISharedPropertyGroupManager, (void **)&manager); + ok(hr == S_OK, "Got hr %#x.\n", hr); + + name = SysAllocString(L"testgroupname"); + isolation = 0; + release = 0; + exists = FALSE; + hr = ISharedPropertyGroupManager_CreatePropertyGroup(manager, name, &isolation, &release, &exists, &group); + ok(hr == S_OK, "Got hr %#x.\n", hr); + SysFreeString(name); + + property_name = SysAllocString(L"testpropertyname"); + exists = TRUE; + expected_refcount = get_refcount(group); + hr = ISharedPropertyGroup_CreateProperty(group, property_name, &exists, &property); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(!exists, "Got unexpected value: %d.\n", exists); + refcount = get_refcount(group); + ok(refcount == expected_refcount, "Got refcount: %u, expected %u.\n", refcount, expected_refcount); + + hr = ISharedPropertyGroup_get_PropertyByPosition(group, 0, &property1); + todo_wine ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr); + hr = ISharedPropertyGroup_get_PropertyByPosition(group, 1, &property1); + todo_wine ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr); + + exists = FALSE; + expected_refcount = get_refcount(property) + 1; + hr = ISharedPropertyGroup_CreateProperty(group, property_name, &exists, &property1); + ok(hr == S_OK, "Got hr %#x.\n", hr); + ok(!!exists, "Got unexpected value: %d.\n", exists); + refcount = get_refcount(property); + ok(refcount == expected_refcount, "Got refcount: %u, expected %u.\n", refcount, expected_refcount); + expected_refcount--; + ISharedProperty_Release(property1); + refcount = get_refcount(property); + ok(refcount == expected_refcount, "Got refcount: %u, expected %u.\n", refcount, expected_refcount); + + expected_refcount = get_refcount(property) + 1; + property1 = NULL; + hr = ISharedPropertyGroup_get_Property(group, property_name, &property1); + todo_wine ok(hr == S_OK, "Got hr %#x.\n", hr); + todo_wine ok(property1 == property, "Got wrong pointer %p.\n", property); + refcount = get_refcount(property); + todo_wine ok(refcount == expected_refcount, "Got refcount: %u, expected %u.\n", refcount, expected_refcount); + if (property1) + ISharedProperty_Release(property1); + SysFreeString(property_name); + + property_name = SysAllocString(L"Testpropertyname"); + hr = ISharedPropertyGroup_get_Property(group, property_name, &property1); + todo_wine ok(hr == E_INVALIDARG, "Got hr %#x.\n", hr); + SysFreeString(property_name); + + hr = ISharedProperty_QueryInterface(property, &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_ISharedProperty); + IDispatch_Release(dispatch); + + ISharedProperty_Release(property); + ISharedPropertyGroup_Release(group); + ISharedPropertyGroupManager_Release(manager); +} + START_TEST(property) { CoInitialize(NULL);
test_interfaces(); test_property_group(); + test_shared_property();
CoUninitialize(); }
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=77763
Your paranoid android.
=== debiant (32 bit report) ===
comsvcs: property.c:130: Test failed: Got hr 0x80040154. property.c:134: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b4d).
Report validation errors: comsvcs:property crashed (c0000005)
=== debiant (32 bit French report) ===
comsvcs: property.c:130: Test failed: Got hr 0x80040154. property.c:134: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b4d).
Report validation errors: comsvcs:property crashed (c0000005)
=== debiant (32 bit Japanese:Japan report) ===
comsvcs: property.c:130: Test failed: Got hr 0x80040154. property.c:134: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b4d).
Report validation errors: comsvcs:property crashed (c0000005)
=== debiant (32 bit Chinese:China report) ===
comsvcs: property.c:130: Test failed: Got hr 0x80040154. property.c:134: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b4d).
Report validation errors: comsvcs:property crashed (c0000005)
=== debiant (32 bit WoW report) ===
comsvcs: property.c:130: Test failed: Got hr 0x80040154. property.c:134: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b4d).
Report validation errors: comsvcs:property crashed (c0000005)
=== debiant (64 bit WoW report) ===
comsvcs: property.c:130: Test failed: Got hr 0x80040154. property.c:134: Test failed: Got hr 0x80040154. Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00403b4d).
Report validation errors: comsvcs:property crashed (c0000005)
On 8/30/20 6:04 PM, Jactry Zeng wrote:
@@ -37,10 +44,44 @@ struct property_group LONG isolation, release; struct list entry; BSTR name;
- struct shared_property *properties;
- size_t capacity, count;
- CRITICAL_SECTION cs;
};
According to docs they have two parallel lists/arrays, one indexed, another named, that don't interact. The one you're introducing does not have to use an array.
-
Jactry Zeng -
Marvin -
Nikolay Sivov