On Sun, Jun 12, 2016 at 8:45 PM, Aaryaman Vasishta < jem456.vasishta@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