On Thu, Apr 30, 2015 at 1:11 PM, Stefan Dösinger <stefandoesinger@gmail.com> wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 2015-04-29 um 23:47 schrieb Aaryaman Vasishta:
> - if (!refcount)
> + if (!refcount && !InterlockedDecrement(&d3drm->numIfaces))
> HeapFree(GetProcessHeap(), 0, d3drm);
This works, but this way you have the code that destroys the object 3
times. Not much of an issue as long as it is just a HeapFree, but I
think it would be nicer to have a separate function. Either add a
d3drm_destroy function or a pair of d3drm_add_iface and
d3drm_release_iface functions that also take care of
InterlockedIncrement / Decrement for iface_count)

> - { &IID_IDirect3DRM3, &IID_IDirect3DRM3, S_OK, TRUE },
> - { &IID_IDirect3DRM2, &IID_IDirect3DRM2, S_OK, TRUE },
> + { &IID_IDirect3DRM3, &IID_IDirect3DRM3, S_OK, FALSE },
> + { &IID_IDirect3DRM2, &IID_IDirect3DRM2, S_OK, FALSE },
Afaics all the TODOs are gone, so you can remove this field from the
table and the todo_wine code from test_qi(). Either in the same patch
or a separate one.

I was thinking to remove the TODO's in a separate patch, as both the frame tests and d3drm tests use it. Is that okay? Hope AJ agrees with this change :)

> - LONG ref;
> + LONG ref1, ref2, ref3, numIfaces;
> };
Please use iface_count. Yeah, it's just a plain style thing, but d3drm
is nicely consistent so far in not using CamelCase (except in cases
where Microsoft determined the name).

(Yesterday on IRC I said num_ifaces. Henri says xxx_count is the way
he prefers.)

I don't like CamelCase either, glad to use iface_count :)

> + if (refcount == 1) InterlockedIncrement(&d3drm->numIfaces);
Another minor style issue: Please put the statement on a separate
line, in consistency with the existing code.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

I'll make the changes and resend the patch as try-2

Thank you!

Jam

iQIcBAEBAgAGBQJVQdzBAAoJEN0/YqbEcdMw5WAP/0H52sqgEXMdSmh6sXH/dZMQ
nTZY9uRhElUbb3r6D6HG0ylit994pPaq887t9BSl8F4vbjsPXLjruDbVSwoScCfp
rIDKy9xET+ek4p5TSFC9cX1BFAiC2VKN7YYWmr/wHKVPZc+aFudU9ykac02bi9p2
Oe+9wMBcFbwv33UhNL+2dugA4+0yj6egfPJWmju9C6ff6CPGqgXH70rHsrSf1AYs
VkNPRhGpfI9BxojBChVtgc0zwPM1HnfOwfmKWCW75YNgNg9lL0o+bdoy2wCcxiLq
FYwLLT2o9K+RnPQqRDhdJBKWwY8VrJI/fHZEURRau7RW1ia26j4UrrsAdXTQwhPs
Iy8A63pYOxyZsphq5KJ8cwGABYIlBPUG0lXZQRgKh1Jw8f39HIR5cfnu1at/fPSc
Cw6tfL/t+g4KeHaZDWZnROwyp704kIZGgvSaPuJjwkPtHh2MN14cp9KQj389kCUM
4KfV6FxsAByRuJ8ur1HSaV2OeIOKOJkaDTzaiiMqc581A0awOeCy5SuAUSrQu4IE
4gAIjryIJXd+kTDw46y6OW5UNXHdNJKGWAMiih25yLBi0BLBfUiW0U7qW5Lcerls
e7zefeUYNT+Umj3WU4sDu6B8B6auSZgKrLu9JAwYH2RUhKX9R+z3uTY25VUbJkoP
9WJfJJuosIA5zqLbjxu+
=/BI/
-----END PGP SIGNATURE-----