qom: Consolidate OBJECT_{DEFINE|DECLARE}_SIMPLE_TYPE*

[PATCH 1/9] qom/object: Improve the doc of macros related with simple type

Posted by Zhao Liu 6 months ago

There're 2 changes:
 - For OBJECT_DECLARE_SIMPLE_TYPE:

   Since a clase may not only has virtual method, but also field, thus
   mention when class is not needed for either, then there is no need
   to have the class.

 - For OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES:

   And the words like OBJECT_DEFINE_SIMPLE_TYPE about when the type is
   declared by OBJECT_DECLARE_SIMPLE_TYPE, then user should consider
   to define the type via OBJECT_DEFINE_SIMPLE_TYPE or
   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <eduardo@habkost.net>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 include/qom/object.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 1d5b03372429..14f84ae454d3 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -249,7 +249,7 @@ struct Object
  * declared.
  *
  * This macro should be used unless the class struct needs to have
- * virtual methods declared.
+ * virtual methods or fields declared.
  */
 #define OBJECT_DECLARE_SIMPLE_TYPE(InstanceType, MODULE_OBJ_NAME) \
     typedef struct InstanceType InstanceType; \
@@ -402,7 +402,8 @@ struct Object
  *
  * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
  * the case of a non-abstract type, with interfaces, and with no requirement
- * for a class struct.
+ * for a class struct. If you declared your type with OBJECT_DECLARE_SIMPLE_TYPE
+ * then this is probably the right choice for defining it.
  */
 #define OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(ModuleObjName, \
                                                   module_obj_name, \
-- 
2.34.1

Re: [PATCH 1/9] qom/object: Improve the doc of macros related with simple type

Posted by BALATON Zoltan 6 months ago

On Wed, 14 May 2025, Zhao Liu wrote:
> There're 2 changes:
> - For OBJECT_DECLARE_SIMPLE_TYPE:
>
>   Since a clase may not only has virtual method, but also field, thus

class may not only have virtual methods but also fields, clarify when 
class is not needed

>   mention when class is not needed for either, then there is no need
>   to have the class.
>
> - For OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES:
>
>   And the words like OBJECT_DEFINE_SIMPLE_TYPE about when the type is
>   declared by OBJECT_DECLARE_SIMPLE_TYPE, then user should consider
>   to define the type via OBJECT_DEFINE_SIMPLE_TYPE or
>   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> include/qom/object.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 1d5b03372429..14f84ae454d3 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -249,7 +249,7 @@ struct Object
>  * declared.
>  *
>  * This macro should be used unless the class struct needs to have
> - * virtual methods declared.
> + * virtual methods or fields declared.
>  */
> #define OBJECT_DECLARE_SIMPLE_TYPE(InstanceType, MODULE_OBJ_NAME) \
>     typedef struct InstanceType InstanceType; \
> @@ -402,7 +402,8 @@ struct Object
>  *
>  * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
>  * the case of a non-abstract type, with interfaces, and with no requirement
> - * for a class struct.
> + * for a class struct. If you declared your type with OBJECT_DECLARE_SIMPLE_TYPE
> + * then this is probably the right choice for defining it.

Is "probably" correct here? Is it a must or can still use other defining 
macros? If correct maybe saying "this might be the right choice" is 
simpler.

Regards,
BALATON Zoltan

>  */
> #define OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES(ModuleObjName, \
>                                                   module_obj_name, \
>

Re: [PATCH 1/9] qom/object: Improve the doc of macros related with simple type

Posted by Zhao Liu 6 months ago

Hi BALATON,

On Wed, May 14, 2025 at 02:02:48PM +0200, BALATON Zoltan wrote:
> Date: Wed, 14 May 2025 14:02:48 +0200
> From: BALATON Zoltan <balaton@eik.bme.hu>
> Subject: Re: [PATCH 1/9] qom/object: Improve the doc of macros related with
>  simple type
> 
> On Wed, 14 May 2025, Zhao Liu wrote:
> > There're 2 changes:
> > - For OBJECT_DECLARE_SIMPLE_TYPE:
> > 
> >   Since a clase may not only has virtual method, but also field, thus
> 
> class may not only have virtual methods but also fields, clarify when class
> is not needed

Thanks for the polish! It sounds better.

> >   mention when class is not needed for either, then there is no need
> >   to have the class.
> > 
> > - For OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES:
> > 
> >   And the words like OBJECT_DEFINE_SIMPLE_TYPE about when the type is
> >   declared by OBJECT_DECLARE_SIMPLE_TYPE, then user should consider
> >   to define the type via OBJECT_DEFINE_SIMPLE_TYPE or
> >   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES.
> > 
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: "Daniel P. Berrang?" <berrange@redhat.com>
> > Cc: Eduardo Habkost <eduardo@habkost.net>
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> > include/qom/object.h | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/qom/object.h b/include/qom/object.h
> > index 1d5b03372429..14f84ae454d3 100644
> > --- a/include/qom/object.h
> > +++ b/include/qom/object.h
> > @@ -249,7 +249,7 @@ struct Object
> >  * declared.
> >  *
> >  * This macro should be used unless the class struct needs to have
> > - * virtual methods declared.
> > + * virtual methods or fields declared.
> >  */
> > #define OBJECT_DECLARE_SIMPLE_TYPE(InstanceType, MODULE_OBJ_NAME) \
> >     typedef struct InstanceType InstanceType; \
> > @@ -402,7 +402,8 @@ struct Object
> >  *
> >  * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
> >  * the case of a non-abstract type, with interfaces, and with no requirement
> > - * for a class struct.
> > + * for a class struct. If you declared your type with OBJECT_DECLARE_SIMPLE_TYPE
> > + * then this is probably the right choice for defining it.
> 
> Is "probably" correct here? Is it a must or can still use other defining
> macros?

Yes, because there's another choice: OBJECT_DEFINE_SIMPLE_TYPE.

> If correct maybe saying "this might be the right choice" is simpler.

:-) I copied this sentence from the documentation of
OBJECT_DEFINE_SIMPLE_TYPE. The difference between these 2 macros is
OBJECT_DEFINE_SIMPLE_TYPE doesn't support interface.

So I'd like to keep this sentence, consistent with the description of
OBJECT_DEFINE_SIMPLE_TYPE.

Thanks,
Zhao

Re: [PATCH 1/9] qom/object: Improve the doc of macros related with simple type

Posted by BALATON Zoltan 6 months ago

On Wed, 14 May 2025, Zhao Liu wrote:
> Hi BALATON,

In case you did not know:
https://en.wikipedia.org/wiki/Name_order#Hungary

> On Wed, May 14, 2025 at 02:02:48PM +0200, BALATON Zoltan wrote:
>> Date: Wed, 14 May 2025 14:02:48 +0200
>> From: BALATON Zoltan <balaton@eik.bme.hu>
>> Subject: Re: [PATCH 1/9] qom/object: Improve the doc of macros related with
>>  simple type
>>
>> On Wed, 14 May 2025, Zhao Liu wrote:
>>> There're 2 changes:
>>> - For OBJECT_DECLARE_SIMPLE_TYPE:
>>>
>>>   Since a clase may not only has virtual method, but also field, thus
>>
>> class may not only have virtual methods but also fields, clarify when class
>> is not needed
>
> Thanks for the polish! It sounds better.

It's also grammatically more correct. But I'm not a native English speaker 
so may still be wrong.

>>>   mention when class is not needed for either, then there is no need
>>>   to have the class.
>>>
>>> - For OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES:
>>>
>>>   And the words like OBJECT_DEFINE_SIMPLE_TYPE about when the type is
>>>   declared by OBJECT_DECLARE_SIMPLE_TYPE, then user should consider
>>>   to define the type via OBJECT_DEFINE_SIMPLE_TYPE or
>>>   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES.
>>>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: "Daniel P. Berrang?" <berrange@redhat.com>
>>> Cc: Eduardo Habkost <eduardo@habkost.net>
>>> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
>>> ---
>>> include/qom/object.h | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/qom/object.h b/include/qom/object.h
>>> index 1d5b03372429..14f84ae454d3 100644
>>> --- a/include/qom/object.h
>>> +++ b/include/qom/object.h
>>> @@ -249,7 +249,7 @@ struct Object
>>>  * declared.
>>>  *
>>>  * This macro should be used unless the class struct needs to have
>>> - * virtual methods declared.
>>> + * virtual methods or fields declared.
>>>  */
>>> #define OBJECT_DECLARE_SIMPLE_TYPE(InstanceType, MODULE_OBJ_NAME) \
>>>     typedef struct InstanceType InstanceType; \
>>> @@ -402,7 +402,8 @@ struct Object
>>>  *
>>>  * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
>>>  * the case of a non-abstract type, with interfaces, and with no requirement
>>> - * for a class struct.
>>> + * for a class struct. If you declared your type with OBJECT_DECLARE_SIMPLE_TYPE
>>> + * then this is probably the right choice for defining it.
>>
>> Is "probably" correct here? Is it a must or can still use other defining
>> macros?
>
> Yes, because there's another choice: OBJECT_DEFINE_SIMPLE_TYPE.
>
>> If correct maybe saying "this might be the right choice" is simpler.
>
> :-) I copied this sentence from the documentation of
> OBJECT_DEFINE_SIMPLE_TYPE. The difference between these 2 macros is
> OBJECT_DEFINE_SIMPLE_TYPE doesn't support interface.
>
> So I'd like to keep this sentence, consistent with the description of
> OBJECT_DEFINE_SIMPLE_TYPE.

OK in that case no problem to keep consistency. It still sounds a bit 
unspecific for documentation to me because it leaves the question open of 
what is it when it's not the right choice but if this is already there 
then that's another unrelated question.

Regards,
BALATON Zoltan

Re: [PATCH 1/9] qom/object: Improve the doc of macros related with simple type

Posted by Zhao Liu 6 months ago

On Wed, May 14, 2025 at 06:13:33PM +0200, BALATON Zoltan wrote:
> Date: Wed, 14 May 2025 18:13:33 +0200
> From: BALATON Zoltan <balaton@eik.bme.hu>
> Subject: Re: [PATCH 1/9] qom/object: Improve the doc of macros related with
>  simple type
> 
> On Wed, 14 May 2025, Zhao Liu wrote:
> > Hi BALATON,
> 
> In case you did not know:
> https://en.wikipedia.org/wiki/Name_order#Hungary
> 
> > On Wed, May 14, 2025 at 02:02:48PM +0200, BALATON Zoltan wrote:
> > > Date: Wed, 14 May 2025 14:02:48 +0200
> > > From: BALATON Zoltan <balaton@eik.bme.hu>
> > > Subject: Re: [PATCH 1/9] qom/object: Improve the doc of macros related with
> > >  simple type
> > > 
> > > On Wed, 14 May 2025, Zhao Liu wrote:
> > > > There're 2 changes:
> > > > - For OBJECT_DECLARE_SIMPLE_TYPE:
> > > > 
> > > >   Since a clase may not only has virtual method, but also field, thus
> > > 
> > > class may not only have virtual methods but also fields, clarify when class
> > > is not needed
> > 
> > Thanks for the polish! It sounds better.
> 
> It's also grammatically more correct. But I'm not a native English speaker
> so may still be wrong.

Neither am I. But I like this advice.

> > > >   mention when class is not needed for either, then there is no need
> > > >   to have the class.
> > > > 
> > > > - For OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES:
> > > > 
> > > >   And the words like OBJECT_DEFINE_SIMPLE_TYPE about when the type is
> > > >   declared by OBJECT_DECLARE_SIMPLE_TYPE, then user should consider
> > > >   to define the type via OBJECT_DEFINE_SIMPLE_TYPE or
> > > >   OBJECT_DEFINE_SIMPLE_TYPE_WITH_INTERFACES.
> > > > 
> > > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > > Cc: "Daniel P. Berrang?" <berrange@redhat.com>
> > > > Cc: Eduardo Habkost <eduardo@habkost.net>
> > > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > > > ---
> > > > include/qom/object.h | 5 +++--
> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/include/qom/object.h b/include/qom/object.h
> > > > index 1d5b03372429..14f84ae454d3 100644
> > > > --- a/include/qom/object.h
> > > > +++ b/include/qom/object.h
> > > > @@ -249,7 +249,7 @@ struct Object
> > > >  * declared.
> > > >  *
> > > >  * This macro should be used unless the class struct needs to have
> > > > - * virtual methods declared.
> > > > + * virtual methods or fields declared.
> > > >  */
> > > > #define OBJECT_DECLARE_SIMPLE_TYPE(InstanceType, MODULE_OBJ_NAME) \
> > > >     typedef struct InstanceType InstanceType; \
> > > > @@ -402,7 +402,8 @@ struct Object
> > > >  *
> > > >  * This is a variant of OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for
> > > >  * the case of a non-abstract type, with interfaces, and with no requirement
> > > > - * for a class struct.
> > > > + * for a class struct. If you declared your type with OBJECT_DECLARE_SIMPLE_TYPE
> > > > + * then this is probably the right choice for defining it.
> > > 
> > > Is "probably" correct here? Is it a must or can still use other defining
> > > macros?
> > 
> > Yes, because there's another choice: OBJECT_DEFINE_SIMPLE_TYPE.
> > 
> > > If correct maybe saying "this might be the right choice" is simpler.
> > 
> > :-) I copied this sentence from the documentation of
> > OBJECT_DEFINE_SIMPLE_TYPE. The difference between these 2 macros is
> > OBJECT_DEFINE_SIMPLE_TYPE doesn't support interface.
> > 
> > So I'd like to keep this sentence, consistent with the description of
> > OBJECT_DEFINE_SIMPLE_TYPE.
> 
> OK in that case no problem to keep consistency. It still sounds a bit
> unspecific for documentation to me because it leaves the question open of
> what is it when it's not the right choice but if this is already there then
> that's another unrelated question.

I get your point.

Before this sentance, the doc said "This is a variant of
OBJECT_DEFINE_TYPE_EXTENDED, which is suitable for the common case of a
non-abstract type, without any interfaces, and with no requirement for a
class struct."

IMO it's the pretty accurate description about in which case this macro
is used. This description could clarify your question :-).

Thanks,
Zhao