Re: [Qemu-devel] [PATCH for-3.2 v5 12/19] qdev-props: convert global_props to GPtrArray

[Igor Mammedov]

On Tue, 11 Dec 2018 16:12:58 +0400
Marc-André Lureau <address@hidden> wrote:

> Hi
> 
> On Mon, Dec 10, 2018 at 9:07 PM Igor Mammedov <address@hidden> wrote:
> >
> > On Tue,  4 Dec 2018 18:20:16 +0400
> > Marc-André Lureau <address@hidden> wrote:
> >  
> > > A step towards being able to call a common function,
> > > object_apply_global_props().
> > >
> > > Signed-off-by: Marc-André Lureau <address@hidden>
> > > ---
> > >  hw/core/qdev-properties.c | 29 ++++++++++++++++++++---------
> > >  1 file changed, 20 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > > index 43c30a57f4..3467e0485c 100644
> > > --- a/hw/core/qdev-properties.c
> > > +++ b/hw/core/qdev-properties.c
> > > @@ -1173,22 +1173,32 @@ void qdev_prop_set_ptr(DeviceState *dev, const 
> > > char *name, void *value)
> > >      *ptr = value;
> > >  }
> > >
> > > -static GList *global_props;
> > > +static GPtrArray *global_props(void)
> > > +{
> > > +    static GPtrArray *gp;
> > > +
> > > +    if (!gp) {
> > > +        gp = g_ptr_array_new();  
> > one more leak?  
> 
> We leak the global_props list before and using a list is going to use
> more memory than using a GPtrArray.
> 
> If you worry about the global leaks, we can adress it with
> destructors. I don't think that matters here.

We don't do it for a bunch of 'static' objects, so as Eduardo
pointed out it's not worth it or more precisely it's broken
currently and it's out of the scope of this series to fix it
up.

> 
> >  
> > > +    }
> > > +
> > > +    return gp;
> > > +}
> > >
> > >  void qdev_prop_register_global(GlobalProperty *prop)
> > >  {
> > > -    global_props = g_list_append(global_props, prop);
> > > +    g_ptr_array_add(global_props(), prop);
> > >  }
> > >
> > >  int qdev_prop_check_globals(void)
> > >  {
> > > -    GList *l;
> > > -    int ret = 0;
> > > +    int i, ret = 0;
> > >
> > > -    for (l = global_props; l; l = l->next) {
> > > -        GlobalProperty *prop = l->data;
> > > +    for (i = 0; i < global_props()->len; i++) {
> > > +        GlobalProperty *prop;
> > >          ObjectClass *oc;
> > >          DeviceClass *dc;
> > > +
> > > +        prop = g_ptr_array_index(global_props(), i);
> > >          if (prop->used) {
> > >              continue;
> > >          }
> > > @@ -1213,12 +1223,13 @@ int qdev_prop_check_globals(void)
> > >
> > >  void qdev_prop_set_globals(DeviceState *dev)
> > >  {
> > > -    GList *l;
> > > +    int i;
> > >
> > > -    for (l = global_props; l; l = l->next) {
> > > -        GlobalProperty *prop = l->data;
> > > +    for (i = 0; i < global_props()->len; i++) {
> > > +        GlobalProperty *prop;
> > >          Error *err = NULL;
> > >
> > > +        prop = g_ptr_array_index(global_props(), i);
> > >          if (object_dynamic_cast(OBJECT(dev), prop->driver) == NULL) {
> > >              continue;
> > >          }  
> >
> >  
> 
>