[PATCH 1/3] d3drm: Add stubs for IDirect3DRMFrame interface. (v2)

[Aaryaman Vasishta]

On Sun, Jun 12, 2016 at 8:45 PM, Aaryaman Vasishta <
jem456.vasishta at gmail.com> wrote

> > +static HRESULT WINAPI d3drm_frame1_AddChild(IDirect3DRMFrame *iface,
>> IDirect3DRMFrame *child)
>> > +{
>> > +    struct d3drm_frame *frame = impl_from_IDirect3DRMFrame(iface);
>> > +    IDirect3DRMFrame3 *child3;
>> > +    HRESULT hr;
>> > +
>> > +    TRACE("iface %p, child %p.\n", iface, child);
>> > +
>> > +    if (!child)
>> > +        return D3DRMERR_BADOBJECT;
>> > +    hr = IDirect3DRMFrame_QueryInterface(child,
>> &IID_IDirect3DRMFrame3, (void **)&child3);
>> > +    if (hr != S_OK)
>> > +        return D3DRMERR_BADOBJECT;
>> > +    IDirect3DRMFrame_Release(child);
>> > +
>> > +    return IDirect3DRMFrame3_AddChild(&frame->IDirect3DRMFrame3_iface,
>> child3);
>> > +}
>> This doesn't seem right. Right now you're releasing a reference you don't
>> own (child's frame1) and leak one (child's frame3). Is there a reason why
>> you're using QueryInterface instead of accessing
>> child->IDirect3DRMFrame3_iface?
>>
>
>> If you stick to QI I think this should be more like this (error checking
>> excluded):
>> QI(child, IID_child3, &child3);
>> hr = Frame3_AddChild(frame, child3);
>> IDirect3DRMFrame3_Release(child3);
>>
>> d3drm_frame2_AddChild has the same problem, this is where your code seems
>> to come from.
>>
> I didn't write this (copied from version 3), though your approach does
> make sense. I could fix this in a separate patch if needed.
>
> I think the reasoning behind that approach could be that all versions
share the same refcount, so it really doesn't matter which version is
released, as long as the recount is maintained.  Also the refcount tests
for AddChild are present in test_Frame, so the leak is intentional.

Cheers,
Aaryaman
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20160612/644297f6/attachment.html>