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

Marc-André Lureau posted 19 patches 6 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [PATCH for-3.2 v5 12/19] qdev-props: convert global_props to GPtrArray
Posted by Marc-André Lureau 6 years, 10 months ago
A step towards being able to call a common function,
object_apply_global_props().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 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();
+    }
+
+    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;
         }
-- 
2.20.0.rc1


Re: [Qemu-devel] [PATCH for-3.2 v5 12/19] qdev-props: convert global_props to GPtrArray
Posted by Igor Mammedov 6 years, 9 months ago
On Tue,  4 Dec 2018 18:20:16 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> A step towards being able to call a common function,
> object_apply_global_props().
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  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();
> +    }
> +
> +    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;
>          }


Re: [Qemu-devel] [PATCH for-3.2 v5 12/19] qdev-props: convert global_props to GPtrArray
Posted by Igor Mammedov 6 years, 10 months ago
On Tue,  4 Dec 2018 18:20:16 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> A step towards being able to call a common function,
> object_apply_global_props().
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  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?

> +    }
> +
> +    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;
>          }


Re: [Qemu-devel] [PATCH for-3.2 v5 12/19] qdev-props: convert global_props to GPtrArray
Posted by Marc-André Lureau 6 years, 9 months ago
Hi

On Mon, Dec 10, 2018 at 9:07 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Tue,  4 Dec 2018 18:20:16 +0400
> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
>
> > A step towards being able to call a common function,
> > object_apply_global_props().
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  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.

>
> > +    }
> > +
> > +    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;
> >          }
>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH for-3.2 v5 12/19] qdev-props: convert global_props to GPtrArray
Posted by Igor Mammedov 6 years, 9 months ago
On Tue, 11 Dec 2018 16:12:58 +0400
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> Hi
> 
> On Mon, Dec 10, 2018 at 9:07 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Tue,  4 Dec 2018 18:20:16 +0400
> > Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> >  
> > > A step towards being able to call a common function,
> > > object_apply_global_props().
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  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;
> > >          }  
> >
> >  
> 
>