On Wed, Mar 29, 2017 at 09:56:54PM +0200, Laszlo Ersek wrote: > 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.
I will take a look, thanks! > > (2) Can you CC the maintainers of the code being modified on each patch? I can, on the shorter ones, but this will be difficult on patch 7, which is composed mostly of mechanical changes generated using Coccinelle. I didn't want to generate a huge CC list just because of those mechanical changes. > > (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.) The coccinelle-generated parts of patch 7 will be hard to split, but I can try to split the rest of the changes later. I tried to keep the patch count smaller on purpose on the RFC, because first I want to hear what people think of this proposal. If people like it, I will split a few patches, especially: * [RFC v2 2/9] Simplify code using *MACHINE_GET_CLASS (split the pc, machine core, and pci changes into separate patches) * [RFC v2 5/9] Make class parameter const at some functions (split into one patch for each subsystem being changed) * [RFC v2 6/9] Explicitly cast the *_GET_CLASS() value when we break the rules (not sure if I I will split it, but I will surely CC the maintainers for each subsystem) * [RFC v2 7/9] Use const variables for *_GET_CLASS values (probably I will separate the coccinelle-generated changes from the others, and split the manual changes per subsystem) -- Eduardo
