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

[Laszlo Ersek]

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.
> 
> This series changes OBJECT_CLASS, OBJECT_CLASS_CHECK,
> 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.)

Thanks
Laszlo