Re: [Qemu-devel] [PATCH] qom: add helpers REGISTER_STATIC_TYPE[S]()

[Eduardo Habkost]

On Tue, Oct 03, 2017 at 06:37:37PM +0200, Igor Mammedov wrote:
> On Tue, 3 Oct 2017 12:37:22 -0300
> Eduardo Habkost <address@hidden> wrote:
> 
> > On Tue, Oct 03, 2017 at 05:01:39PM +0200, Igor Mammedov wrote:
> > > On Tue, 3 Oct 2017 11:25:01 -0300
> > > Eduardo Habkost <address@hidden> wrote:
> > >   
> > > > On Tue, Oct 03, 2017 at 02:14:46PM +0200, Igor Mammedov wrote:  
> > > > > it will help to remove code duplication in places
> > > > > that currently open code registration of static
> > > > > type[s] and remove necessity to declare function
> > > > > for type_init() to call, when only static types
> > > > > need to be registered.
> > > > > 
> > > > > Signed-off-by: Igor Mammedov <address@hidden>
> > > > > ---
> > > > > I'm going to use it for CPU types in followup patches
> > > > > 
> > > > > CC: address@hidden
> > > > > ---
> > > > >  include/qemu/module.h | 10 ++++++++++
> > > > >  include/qom/object.h  | 10 ++++++++++
> > > > >  qom/object.c          |  9 +++++++++
> > > > >  3 files changed, 29 insertions(+)
> > > > > 
> > > > > diff --git a/include/qemu/module.h b/include/qemu/module.h
> > > > > index 56dd218..60dd632 100644
> > > > > --- a/include/qemu/module.h
> > > > > +++ b/include/qemu/module.h
> > > > > @@ -52,6 +52,16 @@ typedef enum {
> > > > >  #define type_init(function) module_init(function, MODULE_INIT_QOM)
> > > > >  #define trace_init(function) module_init(function, MODULE_INIT_TRACE)
> > > > >  
> > > > > +#define REGISTER_STATIC_TYPES(t, nr)                                 
> > > > >        \    
> > > > 
> > > > I'm unsure about the name.  We already have a declarative way to
> > > > register machine types (DEFINE_MACHINE), maybe use "DEFINE_*" for
> > > > consistency?
> > > > 
> > > > Also, this function won't work if t isn't a static variable, will
> > > > it?  The "STATIC" part looks redundant.
> > > > 
> > > > In other words, how about "DEFINE_TYPE" and "DEFINE_TYPES"?  
> > > 'define' word remind me more of declaration but here we actually
> > > register types that's why 'register' looks better to me as it says
> > > exactly what's going here.  
> > 
> > Making it look like a declaration and not an action like
> > "register" is on purpose, as it's not a function call anymore:
> > it's a declaration that will make the type get registered
> > automatically.
> 
> so if there is no objections I'd respin patch with
>  "DEFINE_TYPE" and "DEFINE_TYPES"
> that Eduardo suggested and put these into object.h instead of module.h
> 
> I also would prefer to drop/postpone "DEFINE_TYPE" till someone takes
> on a quest to actually use it.

No problem to me.  Missing type_register_static_array() was a
problem, but missing DEFINE_TYPE is not a big deal.

-- 
Eduardo