| Message ID | 20200902224311.1321159-17-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qom: Rename macros for consistency | expand |
On 9/3/20 12:42 AM, Eduardo Habkost wrote: > This will make the type name constant consistent with the name of > the type checking macro. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Eduardo Habkost <ehabkost@redhat.com> > Cc: qemu-devel@nongnu.org > --- > hw/i386/kvm/i8259.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/hw/i386/kvm/i8259.c b/hw/i386/kvm/i8259.c > index 3f8bf69e9c..687c0cd536 100644 > --- a/hw/i386/kvm/i8259.c > +++ b/hw/i386/kvm/i8259.c > @@ -19,10 +19,10 @@ > #include "sysemu/kvm.h" > #include "qom/object.h" > > -#define TYPE_KVM_I8259 "kvm-i8259" > +#define TYPE_KVM_PIC "kvm-i8259" I disagree with this patch, as we have various KVM INTC and only one KVM_I8259. TYPE_KVM_ARM_GIC and TYPE_KVM_S390_FLIC are kind of TYPE_KVM_INTC ... Can we rename it KVM_I8259_PIC? > typedef struct KVMPICClass KVMPICClass; > DECLARE_CLASS_CHECKERS(KVMPICClass, KVM_PIC, > - TYPE_KVM_I8259) > + TYPE_KVM_PIC) > > /** > * KVMPICClass: > @@ -133,8 +133,8 @@ static void kvm_pic_realize(DeviceState *dev, Error **errp) > > qemu_irq *kvm_i8259_init(ISABus *bus) > { > - i8259_init_chip(TYPE_KVM_I8259, bus, true); > - i8259_init_chip(TYPE_KVM_I8259, bus, false); > + i8259_init_chip(TYPE_KVM_PIC, bus, true); > + i8259_init_chip(TYPE_KVM_PIC, bus, false); > > return qemu_allocate_irqs(kvm_pic_set_irq, NULL, ISA_NUM_IRQS); > } > @@ -152,7 +152,7 @@ static void kvm_i8259_class_init(ObjectClass *klass, void *data) > } > > static const TypeInfo kvm_i8259_info = { > - .name = TYPE_KVM_I8259, > + .name = TYPE_KVM_PIC, > .parent = TYPE_PIC_COMMON, > .instance_size = sizeof(PICCommonState), > .class_init = kvm_i8259_class_init, >
On Thu, Sep 03, 2020 at 02:53:52PM +0200, Philippe Mathieu-Daudé wrote: > On 9/3/20 12:42 AM, Eduardo Habkost wrote: > > This will make the type name constant consistent with the name of > > the type checking macro. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > Cc: "Michael S. Tsirkin" <mst@redhat.com> > > Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Richard Henderson <rth@twiddle.net> > > Cc: Eduardo Habkost <ehabkost@redhat.com> > > Cc: qemu-devel@nongnu.org > > --- > > hw/i386/kvm/i8259.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/hw/i386/kvm/i8259.c b/hw/i386/kvm/i8259.c > > index 3f8bf69e9c..687c0cd536 100644 > > --- a/hw/i386/kvm/i8259.c > > +++ b/hw/i386/kvm/i8259.c > > @@ -19,10 +19,10 @@ > > #include "sysemu/kvm.h" > > #include "qom/object.h" > > > > -#define TYPE_KVM_I8259 "kvm-i8259" > > +#define TYPE_KVM_PIC "kvm-i8259" > > I disagree with this patch, as we have various KVM INTC and only one > KVM_I8259. > > TYPE_KVM_ARM_GIC and TYPE_KVM_S390_FLIC are kind of TYPE_KVM_INTC ... > > Can we rename it KVM_I8259_PIC? I'm inclined to agree, but I'm not completely sure. Why is it OK to have a macro named KVM_PIC, a struct named KVMPICClass, a struct named PICCommonState, but not OK to have a constant named TYPE_KVM_PIC? What about the TYPE_PIC_COMMON constant? All these symbols are internal to the i8259 code and aren't expected to be unique globally. Are TYPE_* names more special and expected to be unique globally? If yes, why?
diff --git a/hw/i386/kvm/i8259.c b/hw/i386/kvm/i8259.c index 3f8bf69e9c..687c0cd536 100644 --- a/hw/i386/kvm/i8259.c +++ b/hw/i386/kvm/i8259.c @@ -19,10 +19,10 @@ #include "sysemu/kvm.h" #include "qom/object.h" -#define TYPE_KVM_I8259 "kvm-i8259" +#define TYPE_KVM_PIC "kvm-i8259" typedef struct KVMPICClass KVMPICClass; DECLARE_CLASS_CHECKERS(KVMPICClass, KVM_PIC, - TYPE_KVM_I8259) + TYPE_KVM_PIC) /** * KVMPICClass: @@ -133,8 +133,8 @@ static void kvm_pic_realize(DeviceState *dev, Error **errp) qemu_irq *kvm_i8259_init(ISABus *bus) { - i8259_init_chip(TYPE_KVM_I8259, bus, true); - i8259_init_chip(TYPE_KVM_I8259, bus, false); + i8259_init_chip(TYPE_KVM_PIC, bus, true); + i8259_init_chip(TYPE_KVM_PIC, bus, false); return qemu_allocate_irqs(kvm_pic_set_irq, NULL, ISA_NUM_IRQS); } @@ -152,7 +152,7 @@ static void kvm_i8259_class_init(ObjectClass *klass, void *data) } static const TypeInfo kvm_i8259_info = { - .name = TYPE_KVM_I8259, + .name = TYPE_KVM_PIC, .parent = TYPE_PIC_COMMON, .instance_size = sizeof(PICCommonState), .class_init = kvm_i8259_class_init,
This will make the type name constant consistent with the name of the type checking macro. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Richard Henderson <rth@twiddle.net> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: qemu-devel@nongnu.org --- hw/i386/kvm/i8259.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)