On Thu, Jun 08, 2017 at 07:26:35PM +0200, Cédric Le Goater wrote: > On 06/08/2017 07:00 PM, Greg Kurz wrote: > > On Thu, 8 Jun 2017 18:08:44 +0200 > > Cédric Le Goater <c...@kaod.org> wrote: > > > >>>>> FWIW, other people do that as well (see hw/i386/pc_q35.c for example). > >>>>> > >>>> > >>>> well, I don't see the benefits of changing a string constant by a > >>>> define. > >>>> > >>> > >>> Improved semantics, especially since the "xics" string appears in > >>> many places with different meanings. > >> > >> ah ? If so, we should do a cleanup up. The code seems consistent from > >> what I can see. xics is a general name for : > >> > >> 'PowerPC interrupt controller (type 2)' > >> > >> and it is mostly used as a prefix. There are no "xics" object, only a > > > > I'm only talking about "xics" as a property name actually: > > > > $ git grep '"xics"' > > hw/intc/xics.c: obj = object_property_get_link(OBJECT(dev), "xics", > > &err); > > hw/intc/xics.c: obj = object_property_get_link(OBJECT(dev), "xics", > > &err); > > hw/ppc/pnv.c: object_property_add_const_link(OBJECT(&chip->psi), "xics", > > hw/ppc/pnv.c: object_property_add_const_link(OBJECT(pnv_core), > > "xics", > > hw/ppc/pnv_core.c: object_property_add_const_link(obj, "xics", > > OBJECT(xi), &error_abort); > > hw/ppc/pnv_core.c: xi = object_property_get_link(OBJECT(dev), "xics", > > &local_err); > > hw/ppc/pnv_psi.c: obj = object_property_get_link(OBJECT(dev), "xics", > > &err); > > hw/ppc/pnv_psi.c: object_property_add_const_link(OBJECT(ics), "xics", > > obj, &error_abort); > > hw/ppc/spapr.c: object_property_add_const_link(obj, "xics", > > OBJECT(spapr), &error_abort); > > hw/ppc/spapr_cpu_core.c: object_property_add_const_link(obj, "xics", > > OBJECT(spapr), &error_abort); > > > > You have to read the code to know which ones are related. > > The "xics" property link always point to the same object : > the XICSFabric object which is the machine, spapr or pnv. > > > With this patch applied, it is mostly obvious, even for the newbie: > > ah. the goal is to know where in the code the link was set. > It can be even more complex with aliases.
There doesn't seem to be a strong convention about whether to use raw property names or defines across qemu. I'm not all that fussed either way. I do see one small advantage to use defines: if you make a typo, it will probably result in a compile time error, whereas with a bare string it won't show up until a runtime error. In this case, I intend to take the macro patch, mostly just on the basis of avoiding further delays to rework the remaining patches. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature