| Message ID | 20200902224311.1321159-15-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: Paolo Bonzini <pbonzini@redhat.com> > Cc: qemu-devel@nongnu.org > --- > include/hw/timer/i8254.h | 4 ++-- > hw/timer/i8254.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h > index 1a522a2457..ddd925074f 100644 > --- a/include/hw/timer/i8254.h > +++ b/include/hw/timer/i8254.h > @@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass; > DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass, > PIT_COMMON, TYPE_PIT_COMMON) > > -#define TYPE_I8254 "isa-pit" > +#define TYPE_PIT "isa-pit" I disagree with this patch, as we have various PIT and only one I8254. > #define TYPE_KVM_I8254 "kvm-pit" > > static inline ISADevice *i8254_pit_init(ISABus *bus, int base, int isa_irq, > @@ -54,7 +54,7 @@ static inline ISADevice *i8254_pit_init(ISABus *bus, int base, int isa_irq, > DeviceState *dev; > ISADevice *d; > > - d = isa_new(TYPE_I8254); > + d = isa_new(TYPE_PIT); > dev = DEVICE(d); > qdev_prop_set_uint32(dev, "iobase", base); > isa_realize_and_unref(d, bus, &error_fatal); > diff --git a/hw/timer/i8254.c b/hw/timer/i8254.c > index c01ee2c72a..86f455f67e 100644 > --- a/hw/timer/i8254.c > +++ b/hw/timer/i8254.c > @@ -39,7 +39,7 @@ > > typedef struct PITClass PITClass; > DECLARE_CLASS_CHECKERS(PITClass, PIT, > - TYPE_I8254) > + TYPE_PIT) > > struct PITClass { > PITCommonClass parent_class; > @@ -370,7 +370,7 @@ static void pit_class_initfn(ObjectClass *klass, void *data) > } > > static const TypeInfo pit_info = { > - .name = TYPE_I8254, > + .name = TYPE_PIT, > .parent = TYPE_PIT_COMMON, > .instance_size = sizeof(PITCommonState), > .class_init = pit_class_initfn, >
On Thu, Sep 03, 2020 at 12:18:09PM -0400, Eduardo Habkost wrote: > On Thu, Sep 03, 2020 at 02:47:03PM +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: Paolo Bonzini <pbonzini@redhat.com> > > > Cc: qemu-devel@nongnu.org > > > --- > > > include/hw/timer/i8254.h | 4 ++-- > > > hw/timer/i8254.c | 4 ++-- > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h > > > index 1a522a2457..ddd925074f 100644 > > > --- a/include/hw/timer/i8254.h > > > +++ b/include/hw/timer/i8254.h > > > @@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass; > > > DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass, > > > PIT_COMMON, TYPE_PIT_COMMON) > > > > > > -#define TYPE_I8254 "isa-pit" > > > +#define TYPE_PIT "isa-pit" > > > > I disagree with this patch, as we have various PIT and only one I8254. > > I was unsure about this, and I agree with your point. I will > suggest renaming the PIT macro to I8254 instead. IMHO the macro name should be directly related to the object name string with non-alnum characters replaced by underscore. ie since the object type is "isa-pit", then the macro should be TYPE_ISA_PIT Regards, Daniel
On Thu, Sep 03, 2020 at 05:44:29PM +0100, Daniel P. Berrangé wrote: > On Thu, Sep 03, 2020 at 12:18:09PM -0400, Eduardo Habkost wrote: > > On Thu, Sep 03, 2020 at 02:47:03PM +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: Paolo Bonzini <pbonzini@redhat.com> > > > > Cc: qemu-devel@nongnu.org > > > > --- > > > > include/hw/timer/i8254.h | 4 ++-- > > > > hw/timer/i8254.c | 4 ++-- > > > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h > > > > index 1a522a2457..ddd925074f 100644 > > > > --- a/include/hw/timer/i8254.h > > > > +++ b/include/hw/timer/i8254.h > > > > @@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass; > > > > DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass, > > > > PIT_COMMON, TYPE_PIT_COMMON) > > > > > > > > -#define TYPE_I8254 "isa-pit" > > > > +#define TYPE_PIT "isa-pit" > > > > > > I disagree with this patch, as we have various PIT and only one I8254. > > > > I was unsure about this, and I agree with your point. I will > > suggest renaming the PIT macro to I8254 instead. > > IMHO the macro name should be directly related to the object name > string with non-alnum characters replaced by underscore. > > ie since the object type is "isa-pit", then the macro should be > TYPE_ISA_PIT I think that's a good idea in this specific case because it's a short name (I will do it). But I don't think we'll be able to always follow that rule, as the QOM type name is user-visible.
On Thu, Sep 03, 2020 at 09:26:16PM +0200, Philippe Mathieu-Daudé wrote: > On 9/3/20 6:55 PM, Eduardo Habkost wrote: > > On Thu, Sep 03, 2020 at 05:44:29PM +0100, Daniel P. Berrangé wrote: > >> On Thu, Sep 03, 2020 at 12:18:09PM -0400, Eduardo Habkost wrote: > >>> On Thu, Sep 03, 2020 at 02:47:03PM +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: Paolo Bonzini <pbonzini@redhat.com> > >>>>> Cc: qemu-devel@nongnu.org > >>>>> --- > >>>>> include/hw/timer/i8254.h | 4 ++-- > >>>>> hw/timer/i8254.c | 4 ++-- > >>>>> 2 files changed, 4 insertions(+), 4 deletions(-) > >>>>> > >>>>> diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h > >>>>> index 1a522a2457..ddd925074f 100644 > >>>>> --- a/include/hw/timer/i8254.h > >>>>> +++ b/include/hw/timer/i8254.h > >>>>> @@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass; > >>>>> DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass, > >>>>> PIT_COMMON, TYPE_PIT_COMMON) > >>>>> > >>>>> -#define TYPE_I8254 "isa-pit" > >>>>> +#define TYPE_PIT "isa-pit" > >>>> > >>>> I disagree with this patch, as we have various PIT and only one I8254. > >>> > >>> I was unsure about this, and I agree with your point. I will > >>> suggest renaming the PIT macro to I8254 instead. > >> > >> IMHO the macro name should be directly related to the object name > >> string with non-alnum characters replaced by underscore. > >> > >> ie since the object type is "isa-pit", then the macro should be > >> TYPE_ISA_PIT > > > > I think that's a good idea in this specific case because it's a > > short name (I will do it). But I don't think we'll be able to > > always follow that rule, as the QOM type name is user-visible. > > Only user-visible if user-creatable, right? All of them are user-visible. All devices have their properties configurable using -global, and are visible in the QOM and qdev trees. It doesn't mean we can't change them, but it means that changing them is more than just code refactoring.
diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h index 1a522a2457..ddd925074f 100644 --- a/include/hw/timer/i8254.h +++ b/include/hw/timer/i8254.h @@ -45,7 +45,7 @@ typedef struct PITCommonClass PITCommonClass; DECLARE_OBJ_CHECKERS(PITCommonState, PITCommonClass, PIT_COMMON, TYPE_PIT_COMMON) -#define TYPE_I8254 "isa-pit" +#define TYPE_PIT "isa-pit" #define TYPE_KVM_I8254 "kvm-pit" static inline ISADevice *i8254_pit_init(ISABus *bus, int base, int isa_irq, @@ -54,7 +54,7 @@ static inline ISADevice *i8254_pit_init(ISABus *bus, int base, int isa_irq, DeviceState *dev; ISADevice *d; - d = isa_new(TYPE_I8254); + d = isa_new(TYPE_PIT); dev = DEVICE(d); qdev_prop_set_uint32(dev, "iobase", base); isa_realize_and_unref(d, bus, &error_fatal); diff --git a/hw/timer/i8254.c b/hw/timer/i8254.c index c01ee2c72a..86f455f67e 100644 --- a/hw/timer/i8254.c +++ b/hw/timer/i8254.c @@ -39,7 +39,7 @@ typedef struct PITClass PITClass; DECLARE_CLASS_CHECKERS(PITClass, PIT, - TYPE_I8254) + TYPE_PIT) struct PITClass { PITCommonClass parent_class; @@ -370,7 +370,7 @@ static void pit_class_initfn(ObjectClass *klass, void *data) } static const TypeInfo pit_info = { - .name = TYPE_I8254, + .name = TYPE_PIT, .parent = TYPE_PIT_COMMON, .instance_size = sizeof(PITCommonState), .class_init = pit_class_initfn,
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: Paolo Bonzini <pbonzini@redhat.com> Cc: qemu-devel@nongnu.org --- include/hw/timer/i8254.h | 4 ++-- hw/timer/i8254.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)