d3drm: Fix refcounting for IDirect3DRM{1-3}

Aaryaman Vasishta jem456.vasishta at gmail.com
Thu Apr 30 03:00:24 CDT 2015


On Thu, Apr 30, 2015 at 1:11 PM, Stefan Dösinger <stefandoesinger at 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-----
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20150430/f661598c/attachment-0001.html>


More information about the wine-devel mailing list