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. 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.
Thanks! Jam
On Thu, Jul 30, 2015 at 4:43 PM, Stefan Dösinger stefandoesinger@gmail.com wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2015-07-30 um 12:07 schrieb Aaryaman Vasishta:
d3drm_device_create_surfaces_from_clipper sets the clipper and primary surface, and init_device sets ddraw and d3drm first before anything else, so even if any of them fail d3drm_device_destroy will ensure they're destroyed. If I explicitly destroy ddraw, how should I handle destroying of ddraw in d3drm_device_destroy (if it's not destroyed at that point)?
The problem with the current way is that what init_device does if it fails is an implementation detail that the caller shouldn't know. Ideally nothing is ever changed when a function fails.
I see two options:
Make sure init_device doesn't set any objects in case of failure.
Always AddRef ddraw and render target in init_device and always
(including the success case) release them in the caller.
I prefer the second one because that way you have a clear pairing of create/addref and release calls in both places. (d3drm1_CreateDeviceFromClipper create + release, and init_device Addref + d3drm_device_destroy Release).
I think I mentioned this before, but "init_device" and "d3drm_device_destroy" are inconsistent names. I recommend to rename "init_device" to "d3drm_device_init".
-----BEGIN PGP SIGNATURE----- Version: GnuPG v2
iQIcBAEBAgAGBQJVugbeAAoJEN0/YqbEcdMwIBkP/R4QwAu7FKYO67M2u+bf27sF styuLfk1LjmzUg5e9bnWpKaZqdsVash8csXbskwkx1wIsP+Zf8hMG5rDP4pAdpA1 aF+BxjVc8Vzd4uY11E5Qqp8TMvJb2vbJSCHIx9uOL2T18fOxGQWZKzLtdWiYzWaq r5BsjvJ7bvA08mTfbo6EjBydQrCdhTEWTNQg22AR6GcgNjX4sWwLyPmuRsUPciBS k3TLnV2x67O56l2XSJHEXKE/dwaleOXUELVEOsUoGrfkNd64Ptd2EhE0sUZzu2us MdyCohhmJyjjI9olA1DeJdRywa/0emmacHocveeaKILzraSxfZSQD7/ZX3u9UWBd 7ECeD77XBMXefrxA8/1Jxddmq8qi2UUwt3r3UnsozmpbIpb39X0Dsv1MxF2+KF1W G5B5JfW+qB+jUv/cBT7ah/nah9l/WTSFRbxcKq9YY/dCve759LaDoC1rOQMevHuI kgohAVhfFuJl0hHDMqe3u2hACp1VgaLcQMTNOe9z9PJ01fetF4bmcYNrOQq6Vvo0 ir7QeE1LohwjxGibfdbv6MqmhnGpAJVa0LeXyn4wEaCWkoQ/6Tv4kX/uoSEv2aZr beV2+T5LmkgBcwHg6d3PzoZL2yBZeP94FyLsb8kq//P3u5J1VLYE0PLAs3zucvQQ KSj7C89fLJMPYHielQKE =CBf1 -----END PGP SIGNATURE-----