On Tue, Aug 9, 2016 at 9:23 PM, Henri Verbeet <hverbeet@gmail.com> wrote:

On 9 August 2016 at 03:16, Aaryaman Vasishta <jem456.vasishta@gmail.com> wrote:
>�� static HRESULT WINAPI d3drm_frame3_GetScene(IDirect3DRMFrame3 *iface, IDirect3DRMFrame3 **scene)
>�� {
> -�� FIXME("iface %p, scene %p stub!\n", iface, scene);
> +�� HRESULT hr;
> +�� IDirect3DRMFrame3 *frame = iface, *parent_frame = NULL;
>
> -�� return E_NOTIMPL;
> +�� TRACE("iface %p, scene %p.\n", iface, scene);
> +
> +�� if (!scene)
> +�� return D3DRMERR_BADVALUE;
> +
> +�� for (;;)
> +�� {
> +�� if (FAILED(hr = IDirect3DRMFrame3_GetParent(frame, &parent_frame)))
> +�� return hr;
> +�� if (parent_frame)
> +�� {
> +�� frame = parent_frame;
> +�� IDirect3DRMFrame3_Release(parent_frame);
> +�� }
> +�� else
> +�� break;
> +�� }
You could just do

�� if (!parent_frame)
�� break;

More importantly though, as Stefan already mentioned, the reference
counting is weird. I think the code would be much nicer if you just
used impl_from_IDirect3DRMFrame3() and then followed the frame->parent
pointers.

Right, since we have internal access to the struct, why not take advantage of it? Using the approach you described is just like traversing through a linked list, simple and easier to read.

Cheers,

Aaryaman