<div dir="ltr"><div><div><div>I've gone for the second option, I agree it makes things easier for the caller to understand what's happening in terms of refcounting.<br></div>I've attached the patches changed from the ones sent previously. The rest of the patches are the same as in the previous emails. Let me know of any further changes required.<br><br><br></div>Thanks!<br></div>Jam<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 30, 2015 at 4:43 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">-----BEGIN PGP SIGNED MESSAGE-----<br>
Hash: SHA1<br>
<br>
</span><span class="">Am 2015-07-30 um 12:07 schrieb Aaryaman Vasishta:<br>
> d3drm_device_create_surfaces_from_clipper sets the clipper and<br>
> primary surface, and init_device sets ddraw and d3drm first before<br>
> anything else, so even if any of them fail d3drm_device_destroy<br>
> will ensure they're destroyed. If I explicitly destroy ddraw, how<br>
> should I handle destroying of ddraw in d3drm_device_destroy (if<br>
> it's not destroyed at that point)?<br>
</span>The problem with the current way is that what init_device does if it<br>
fails is an implementation detail that the caller shouldn't know.<br>
Ideally nothing is ever changed when a function fails.<br>
<br>
I see two options:<br>
<br>
1) Make sure init_device doesn't set any objects in case of failure.<br>
<br>
2) Always AddRef ddraw and render target in init_device and always<br>
(including the success case) release them in the caller.<br>
<br>
I prefer the second one because that way you have a clear pairing of<br>
create/addref and release calls in both places.<br>
(d3drm1_CreateDeviceFromClipper create + release, and init_device<br>
Addref + d3drm_device_destroy Release).<br>
<br>
I think I mentioned this before, but "init_device" and<br>
"d3drm_device_destroy" are inconsistent names. I recommend to rename<br>
"init_device" to "d3drm_device_init".<br>
<span class=""><br>
-----BEGIN PGP SIGNATURE-----<br>
Version: GnuPG v2<br>
<br>
</span>iQIcBAEBAgAGBQJVugbeAAoJEN0/YqbEcdMwIBkP/R4QwAu7FKYO67M2u+bf27sF<br>
styuLfk1LjmzUg5e9bnWpKaZqdsVash8csXbskwkx1wIsP+Zf8hMG5rDP4pAdpA1<br>
aF+BxjVc8Vzd4uY11E5Qqp8TMvJb2vbJSCHIx9uOL2T18fOxGQWZKzLtdWiYzWaq<br>
r5BsjvJ7bvA08mTfbo6EjBydQrCdhTEWTNQg22AR6GcgNjX4sWwLyPmuRsUPciBS<br>
k3TLnV2x67O56l2XSJHEXKE/dwaleOXUELVEOsUoGrfkNd64Ptd2EhE0sUZzu2us<br>
MdyCohhmJyjjI9olA1DeJdRywa/0emmacHocveeaKILzraSxfZSQD7/ZX3u9UWBd<br>
7ECeD77XBMXefrxA8/1Jxddmq8qi2UUwt3r3UnsozmpbIpb39X0Dsv1MxF2+KF1W<br>
G5B5JfW+qB+jUv/cBT7ah/nah9l/WTSFRbxcKq9YY/dCve759LaDoC1rOQMevHuI<br>
kgohAVhfFuJl0hHDMqe3u2hACp1VgaLcQMTNOe9z9PJ01fetF4bmcYNrOQq6Vvo0<br>
ir7QeE1LohwjxGibfdbv6MqmhnGpAJVa0LeXyn4wEaCWkoQ/6Tv4kX/uoSEv2aZr<br>
beV2+T5LmkgBcwHg6d3PzoZL2yBZeP94FyLsb8kq//P3u5J1VLYE0PLAs3zucvQQ<br>
KSj7C89fLJMPYHielQKE<br>
=CBf1<br>
-----END PGP SIGNATURE-----<br>
</blockquote></div><br></div>