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 <ag...@suse.de> > Cc: Hervé Poussineau <hpous...@reactos.org> > Cc: Juergen Gross <jgr...@suse.com> > Cc: Matthew Rosato <mjros...@linux.vnet.ibm.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: "Dr. David Alan Gilbert" <dgilb...@redhat.com> > Cc: Igor Mammedov <imamm...@redhat.com> > Cc: Andreas Färber <afaer...@suse.de> > > 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