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:

1) Make sure init_device doesn't set any objects in case of failure.

2) 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-----