[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC v2 0/9] qom: Make object_get_class()/*_GET_CLASS r

From: Laszlo Ersek
Subject: Re: [Qemu-devel] [RFC v2 0/9] qom: Make object_get_class()/*_GET_CLASS return const pointers
Date: Wed, 29 Mar 2017 21:56:54 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0

On 03/29/17 21:41, Eduardo Habkost wrote:
> The problem
> -----------
> QOM has a data model where class struct data is static: class
> structs are initialized at class_init, and never changed again.
> ...except for a few rare cases where class data is changed
> outside class_init. Those cases are:
>   qbus_realize(), code added by:
>   commit 61de36761b565a4138d8ad7ec75489ab28fe84b6
>   Date:   Thu Feb 6 16:08:15 2014 +0100
>       qdev: Keep global allocation counter per bus
>   mips_jazz_init(), code added by:
>   commit 54e755588cf1e90f0b1460c4e8e6b6a54b6d3a32
>   Date:   Mon Nov 4 23:26:17 2013 +0100
>       mips jazz: do not raise data bus exception when accessing invalid 
> addresses
>   xen_set_dynamic_sysbus(), code added by:
>   commit 3a6c9172ac5951e6dac2b3f6cbce3cfccdec5894
>   Date:   Tue Nov 22 07:10:58 2016 +0100
>       xen: create qdev for each backend device
>   s390_cpu_realizefn(), code added by:
>   commit c6644fc88bed1176f67b4b7a9df691bcf25c0c5b
>   Date:   Fri Mar 4 12:34:31 2016 -0500
>       s390x/cpu: Get rid of side effects when creating a vcpu
> I want to prevent that from happening again.
> Proposal
> --------
> I propose we make object_get_class() and *_GET_CLASS macros
> return const pointers. This way, anybody willing to change class
> structs outside class_init will (hopefully) be aware that they
> are doing something unexpected.
> This would be very simple and trivial, except that:
> * OBJECT_CLASS_CHECK cast its return value to a different
>   (non-const) pointer type.
> * OBJECT_CLASS_CHECK-based macros are used everywhere to
>   cast class pointers to different class types.
> I have addressed this problem using C11's _Generic keyword.
> _Generic allows us to make OBJECT_CLASS_CHECK and other macros
> return a const pointer if its argument is a const pointer, and a
> non-const pointer otherwise.
> object_class_dynamic_cast_assert(), object_class_dynamic_cast(),
> and object_class_get_parent() to exhibit this const-aware
> behavior.
> If the compiler doesn't support _Generic, we keep the old
> behavior, and the cast macros will keep returning non-const
> pointers.
> ---
> Cc: Alexander Graf <address@hidden>
> Cc: Hervé Poussineau <address@hidden>
> Cc: Juergen Gross <address@hidden>
> Cc: Matthew Rosato <address@hidden>
> Cc: Laszlo Ersek <address@hidden>
> Cc: Paolo Bonzini <address@hidden>
> Cc: "Dr. David Alan Gilbert" <address@hidden>
> Cc: Igor Mammedov <address@hidden>
> Cc: Andreas Färber <address@hidden>
> Eduardo Habkost (9):
>   configure: test if _Generic works as expected
>   Simplify code using *MACHINE_GET_CLASS
>   qom: QUALIFIED_CAST helper macro
>   qom: Make object_class_get_parent() const-aware
>   Make class parameter const at some functions
>   Explicitly cast the *_GET_CLASS() value when we break the rules
>   Use const variables for *_GET_CLASS values
>   qom: Make class cast macros/functions const-aware
>   qom: Make object_get_class() return const pointer

Only superficial comments:

(1) HACKING has some notes on C99 and C11 usage; I think you should
update them as well.

(2) Can you CC the maintainers of the code being modified on each patch?

(3) (Corollary of the former -- I'm not seeing patch 7 yet, which I
assume is the big one, from the cumulative diffstat) -- is it possible
to split up patch 7 more fine-grained? To help reviewers. (I don't
maintain anything in QEMU, so this is just a generic suggestion;
everyone feel free to disagree.)


reply via email to

[Prev in Thread] Current Thread [Next in Thread]