On 8/30/20 6:03 PM, Jactry Zeng wrote:
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; + + 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); They have named constants for modes. Also typo in fixme message. + + EnterCriticalSection(&manager->cs); + + property_group = find_propery_group(manager, name); + if (!property_group) + { + property_group = heap_alloc(sizeof(*property_group)); + if (!property_group) + { + LeaveCriticalSection(&manager->cs); + return E_OUTOFMEMORY; + } + property_group->ISharedPropertyGroup_iface.lpVtbl = &property_group_vtbl; + property_group->parent = manager; + property_group->isolation = *isolation; + property_group->release = *release; + property_group->refcount = 1; + property_group->name = SysAllocString(name); + if (!property_group->name) + { + LeaveCriticalSection(&manager->cs); + heap_free(property_group); + return E_OUTOFMEMORY; + } + + list_add_tail(&manager->property_groups, &property_group->entry); + + *exists = FALSE; + ISharedPropertyGroupManager_AddRef(&property_group->parent->ISharedPropertyGroupManager_iface); + } + else + { + *exists = TRUE; + *isolation = property_group->isolation; + *release = property_group->release; + ISharedPropertyGroup_AddRef(&property_group->ISharedPropertyGroup_iface); + } + *group = &property_group->ISharedPropertyGroup_iface; + + LeaveCriticalSection(&manager->cs); + + return S_OK; I think it would be more readable if lock/unlock was around a smaller block, for example using a helper that creates a new group instance, to avoid multiple error handling branches.