27 Jul
2016
27 Jul
'16
6:51 p.m.
On 2016-07-19 19:08, Aaryaman Vasishta wrote:
+ if (FAILED(hr =
IDirect3DRMDevice_QueryInterface(device, &IID_IDirect3DRMDevice3, (void
**)&device3)))
+ return hr;
+
+ if (FAILED(hr = IDirect3DRMFrame_QueryInterface(camera, &IID_IDirect3DRMFrame3,
(void **)&camera3)))
+ {
+ IDirect3DRMDevice3_Release(device3);
+ return hr;
+ }
+ IDirect3DRMDevice_Release(device);
+ IDirect3DRMFrame_Release(camera);
This releases references you don't own.
Same in the V2 -> V3 thunk.
+ if (!device || !camera)
+ return D3DRMERR_BADOBJECT;
+ if (!viewport)
+ return D3DRMERR_BADVALUE;
These checks are redundant, Init() does that as
well.
+ ok(ref4 == frame_ref, "Expected ref4 ==
frame_ref, got frame_ref = %u, ref4 = %u.\n", frame_ref, ref4);
+
+ device_ref = get_refcount((IUnknown *)device1);
+ frame_ref = get_refcount((IUnknown *)frame);
The get_refcount here seems
redundant because you've just shown that it is back to where it used to be.
27 Jul
27 Jul
7:27 p.m.
New subject: [PATCH 2/6] d3drm: Implement IDirect3DRM*::CreateViewport. (v2 resend)
On Thu, Jul 28, 2016 at 12:21 AM, Stefan Dösinger <stefandoesinger(a)gmail.com
Agreed for version 3 of CreateViewport, but this this apply for version 1/2
as well? Since I'm dereferencing these interfaces there I thought the
checks were justified.
Right, these are definitely redundant.
Thanks for the review!
Cheers,
Aaryaman
wrote:
+ if
(!device || !camera)
+ return D3DRMERR_BADOBJECT;
+ if (!viewport)
+ return D3DRMERR_BADVALUE;
These checks are redundant, Init() does that as
well.
+ ok(ref4 == frame_ref, "Expected ref4 ==
frame_ref, got frame_ref =
%u, ref4 = %u.\n", frame_ref, ref4);
+
+ device_ref = get_refcount((IUnknown *)device1);
+ frame_ref = get_refcount((IUnknown *)frame);
The get_refcount here seems
redundant because you've just shown that it is
back to where it used to be.
7:32 p.m.
New subject: [PATCH 2/6] d3drm: Implement IDirect3DRM*::CreateViewport. (v2 resend)
On 2016-07-27 20:27, Aaryaman Vasishta wrote:
On Thu, Jul 28, 2016 at 12:21 AM, Stefan Dösinger
<stefandoesinger(a)gmail.com <mailto:[email protected]>> wrote:
No, in 1 and 2 you need them as you say. Otherwise
you'll crash when you
try to call QI.
+ if (!device || !camera)
+ return D3DRMERR_BADOBJECT;
+ if (!viewport)
+ return D3DRMERR_BADVALUE;
These checks are redundant, Init() does
that as well.
Agreed for version 3 of CreateViewport, but this this apply for version
1/2 as well? Since I'm dereferencing these interfaces there I thought
the checks were justified.
2851
days inactive
2851
days old
2 comments
2 participants
participants (2)
-
Aaryaman Vasishta -
Stefan Dösinger