<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Apr 30, 2015 at 1:11 PM, Stefan Dösinger <span dir="ltr"><<a href="mailto:stefandoesinger@gmail.com" target="_blank">stefandoesinger@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">-----BEGIN PGP SIGNED MESSAGE-----<br>
Hash: SHA1<br>
<span class=""><br>
Am 2015-04-29 um 23:47 schrieb Aaryaman Vasishta:<br>
> -    if (!refcount)<br>
> +    if (!refcount && !InterlockedDecrement(&d3drm->numIfaces))<br>
>          HeapFree(GetProcessHeap(), 0, d3drm);<br>
</span>This works, but this way you have the code that destroys the object 3<br>
times. Not much of an issue as long as it is just a HeapFree, but I<br>
think it would be nicer to have a separate function. Either add a<br>
d3drm_destroy function or a pair of d3drm_add_iface and<br>
d3drm_release_iface functions that also take care of<br>
InterlockedIncrement / Decrement for iface_count)<br>
<span class=""><br>
> -        { &IID_IDirect3DRM3,               &IID_IDirect3DRM3,      S_OK,                            TRUE  },<br>
> -        { &IID_IDirect3DRM2,               &IID_IDirect3DRM2,      S_OK,                            TRUE  },<br>
> +        { &IID_IDirect3DRM3,               &IID_IDirect3DRM3,      S_OK,                            FALSE },<br>
> +        { &IID_IDirect3DRM2,               &IID_IDirect3DRM2,      S_OK,                            FALSE },<br>
</span>Afaics all the TODOs are gone, so you can remove this field from the<br>
table and the todo_wine code from test_qi(). Either in the same patch<br>
or a separate one.<br></blockquote><div>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 :)<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> -    LONG ref;<br>
> +    LONG ref1, ref2, ref3, numIfaces;<br>
>  };<br>
</span>Please use iface_count. Yeah, it's just a plain style thing, but d3drm<br>
is nicely consistent so far in not using CamelCase (except in cases<br>
where Microsoft determined the name).<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
(Yesterday on IRC I said num_ifaces. Henri says xxx_count is the way<br>
he prefers.)<br></blockquote><div><div>I don't like CamelCase either, glad to use iface_count :) <br></div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
> +    if (refcount == 1) InterlockedIncrement(&d3drm->numIfaces);<br>
</span>Another minor style issue: Please put the statement on a separate<br>
line, in consistency with the existing code.<br>
-----BEGIN PGP SIGNATURE-----<br>
Version: GnuPG v2<br></blockquote><div>I'll make the changes and resend the patch as try-2<br><br></div><div>Thank you!<br></div><div>Jam <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
iQIcBAEBAgAGBQJVQdzBAAoJEN0/YqbEcdMw5WAP/0H52sqgEXMdSmh6sXH/dZMQ<br>
nTZY9uRhElUbb3r6D6HG0ylit994pPaq887t9BSl8F4vbjsPXLjruDbVSwoScCfp<br>
rIDKy9xET+ek4p5TSFC9cX1BFAiC2VKN7YYWmr/wHKVPZc+aFudU9ykac02bi9p2<br>
Oe+9wMBcFbwv33UhNL+2dugA4+0yj6egfPJWmju9C6ff6CPGqgXH70rHsrSf1AYs<br>
VkNPRhGpfI9BxojBChVtgc0zwPM1HnfOwfmKWCW75YNgNg9lL0o+bdoy2wCcxiLq<br>
FYwLLT2o9K+RnPQqRDhdJBKWwY8VrJI/fHZEURRau7RW1ia26j4UrrsAdXTQwhPs<br>
Iy8A63pYOxyZsphq5KJ8cwGABYIlBPUG0lXZQRgKh1Jw8f39HIR5cfnu1at/fPSc<br>
Cw6tfL/t+g4KeHaZDWZnROwyp704kIZGgvSaPuJjwkPtHh2MN14cp9KQj389kCUM<br>
4KfV6FxsAByRuJ8ur1HSaV2OeIOKOJkaDTzaiiMqc581A0awOeCy5SuAUSrQu4IE<br>
4gAIjryIJXd+kTDw46y6OW5UNXHdNJKGWAMiih25yLBi0BLBfUiW0U7qW5Lcerls<br>
e7zefeUYNT+Umj3WU4sDu6B8B6auSZgKrLu9JAwYH2RUhKX9R+z3uTY25VUbJkoP<br>
9WJfJJuosIA5zqLbjxu+<br>
=/BI/<br>
-----END PGP SIGNATURE-----<br>
</blockquote></div><br></div></div>