[PATCH 5/7] d3drm: Fix device not assigning width and height after creation.

[Henri Verbeet]

On 29 June 2016 at 17:54, Aaryaman Vasishta <jem456.vasishta at gmail.com> wrote:
> On Wed, Jun 29, 2016 at 8:01 PM, Henri Verbeet <hverbeet at gmail.com> wrote:
>> On 29 June 2016 at 16:13, Aaryaman Vasishta <jem456.vasishta at gmail.com>
>> wrote:
>> > This is a helper function, so the caller would be handling the
>> > references.
>> > See CreateDeviceFromD3D in device.c.
>> >
>> That construction makes it harder to verify correctness, so in general
>> I'd recommend against that. Regardless, d3drm_device_destroy() takes
>> care of the device reference, but doesn't help for the IDirect3DRM
>> reference, since that's only released when device->ddraw is set.
>
> device->ddraw is set while returning. What should be the better approach
> here?
>
You can probably fix the immediate issue just by storing the return
value and releasing the reference if the QI failed.

The larger issue is that code becomes much easier to follow when you
can assume that functions are supposed to leave things (more or less)
the way they found them on failure. (Unless explicitly indicated.)
I.e., when d3drm_device_set_ddraw_device_d3d() releases all the
references it created when it fails, and d3drm1_CreateDeviceFromD3D()
can assume the device is in the same state as after calling
d3drm_device_create().

Of course the even larger issue is that
d3drm_device_set_ddraw_device_d3d() doesn't make a lot of sense. In a
way similar to e.g. d3drm3_CreateTexture(),
d3drm1_CreateDeviceFromD3D() should probably just call
IDirect3DRMDevice::InitFromD3D().