Am 10. Mai 2023 07:56:15 UTC schrieb "Philippe Mathieu-Daudé"
<phi...@linaro.org>:
>On 30/4/23 23:48, Bernhard Beschow wrote:
>>
>>
>> Am 28. April 2023 16:48:21 UTC schrieb Gurchetan Singh
>> <gurchetansi...@chromium.org>:
>>> From: Gurchetan Singh <gurchetansi...@chromium.org>
>>>
>>> This reduces the amount of renderer backend specific needed to
>>> be exposed to the GL device. We only need one realize function
>>> per renderer backend.
>>>
>>> Signed-off-by: Gurchetan Singh <gurchetansi...@chromium.org>
>>> Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
>>> ---
>>> v1: - Remove NULL inits (Philippe)
>>> - Use VIRTIO_GPU_BASE where possible (Philippe)
>>> v2: - Fix unnecessary line break (Akihiko)
>>>
>>> hw/display/virtio-gpu-gl.c | 15 ++++++---------
>>> hw/display/virtio-gpu-virgl.c | 35 ++++++++++++++++++++++++----------
>>> include/hw/virtio/virtio-gpu.h | 7 -------
>>> 3 files changed, 31 insertions(+), 26 deletions(-)
>
>
>>> void virtio_gpu_virgl_device_realize(DeviceState *qdev, Error **errp)
>>> {
>>> - VirtIOGPU *g = VIRTIO_GPU(qdev);
>>> + VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
>>> + VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
>>> +
>>> + VirtIOGPUBase *bdev = VIRTIO_GPU_BASE(qdev);
>>> + VirtIOGPUBaseClass *vbc = VIRTIO_GPU_BASE_GET_CLASS(bdev);
>>> +
>>> + VirtIOGPU *gpudev = VIRTIO_GPU(qdev);
>>> + VirtIOGPUClass *vgc = VIRTIO_GPU_GET_CLASS(gpudev);
>>> +
>>> + vbc->gl_flushed = virtio_gpu_virgl_flushed;
>>> + vgc->handle_ctrl = virtio_gpu_virgl_handle_ctrl;
>>> + vgc->process_cmd = virtio_gpu_virgl_process_cmd;
>>> + vgc->update_cursor_data = virtio_gpu_virgl_update_cursor;
>>> +
>>> + vdc->reset = virtio_gpu_virgl_reset;
>>
>> A realize method is supposed to modify a single instance only while we're
>> modifying the behavior of whole classes here, i.e. will affect every
>> instance of these classes. This goes against QOM design principles and will
>> therefore be confusing for people who are familiar with QOM in particular
>> and OOP in general. I think the code should be cleaned up in a different way
>> if really needed.
>
>Doh I was too quick and totally missed this was an instance,
>thanks for being careful Bernhard!
My obligation ;)
I wonder if *_GET_CLASS() macros could return const pointers in order for the
compiler to catch such uses? Are there use cases at all in retrieving non-const
class pointers from instances?
Best regards,
Bernhard
