Re: [PATCH 2/3] d3drm: Implement IDirect3DRMFrame*::GetScene. (v6)

[Henri Verbeet]

On 12 August 2016 at 01:09, Aaryaman Vasishta  wrote:
> - IDirect3DRMFrame3_AddRef(*scene = &frame->IDirect3DRMFrame3_iface);

Actually, sorry, you probably shouldn't do that in d3drm. The d3drm
headers are written by hand, so IDirect3DRMFrame3_AddRef() is a macro
instead of an inline function. That means that when this expands the
assignment happens twice.

[PATCH 2/3] d3drm: Implement IDirect3DRMFrame*::GetScene. (v6)

[Aaryaman Vasishta]

On Mon, Aug 15, 2016 at 3:37 PM, Henri Verbeet  wrote:
> On 12 August 2016 at 01:09, Aaryaman Vasishta 
> wrote:
> > - IDirect3DRMFrame3_AddRef(*scene = &frame->IDirect3DRMFrame3_iface);
>
> Actually, sorry, you probably shouldn't do that in d3drm. The d3drm
> headers are written by hand, so IDirect3DRMFrame3_AddRef() is a macro
> instead of an inline function. That means that when this expands the
> assignment happens twice.
>
> No problem. Thanks for the explanation :)

You can sign-off on v5 of this patch, then? (125544)

Cheers,
Aaryaman

[PATCH 2/3] d3drm: Implement IDirect3DRMFrame*::GetScene. (v6)

[Aaryaman Vasishta]

Sorry, I meant 125481 (v4). Should I resend that one?

On Mon, Aug 15, 2016 at 3:47 PM, Aaryaman Vasishta <
[e-mail]> wrote:
> On Mon, Aug 15, 2016 at 3:37 PM, Henri Verbeet  wrote:
> > On 12 August 2016 at 01:09, Aaryaman Vasishta 
> > wrote:
> > > - IDirect3DRMFrame3_AddRef(*scene = &frame->IDirect3DRMFrame3_ifac
> >
> > e);
> > Actually, sorry, you probably shouldn't do that in d3drm. The d3drm
> > headers are written by hand, so IDirect3DRMFrame3_AddRef() is a macro
> > instead of an inline function. That means that when this expands the
> > assignment happens twice.
> >
> > No problem. Thanks for the explanation :)
>
> You can sign-off on v5 of this patch, then? (125544)
>
> Cheers,
> Aaryaman

[PATCH 2/3] d3drm: Implement IDirect3DRMFrame*::GetScene. (v6)

[Henri Verbeet]

On 15 August 2016 at 12:19, Aaryaman Vasishta  wrote:
> Sorry, I meant 125481 (v4). Should I resend that one?

Well no, that one has other issues. This patch looks otherwise fine to
me, but you should split

IDirect3DRMFrame3_AddRef(*scene = &frame->IDirect3DRMFrame3_iface);

into

*scene = &frame->IDirect3DRMFrame3_iface;
    IDirect3DRMFrame3_AddRef(*scene);