Re: [Qemu-devel] [PATCH] qdev: Use GList for global properties

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jan 28, 2016 at 01:02:26PM -0200, Eduardo Habkost wrote:
> If the same GlobalProperty struct is registered twice, the list
> entry gets corrupted, making tqe_next points to itself, and
> qdev_prop_set_globals() gets stuck in a loop. The bug can be
> easily reproduced by running:
> 
>   $ qemu-system-x86_64 -rtc-td-hack -rtc-td-hack
> 
> Change global_props to use GList instead of queue.h, making the
> code simpler and able to deal with properties being registered
> twice.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/core/qdev-properties.c | 17 ++++++++++-------
>  include/hw/qdev-core.h    |  1 -
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index aacad66..180e0da 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1,3 +1,5 @@
> +#include <glib.h>
> +
>  #include "net/net.h"
>  #include "hw/qdev.h"
>  #include "qapi/qmp/qerror.h"
> @@ -1009,12 +1011,11 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value)
>      *ptr = value;
>  }
>  
> -static QTAILQ_HEAD(, GlobalProperty) global_props =
> -        QTAILQ_HEAD_INITIALIZER(global_props);
> +static GList *global_props;
>  
>  void qdev_prop_register_global(GlobalProperty *prop)
>  {
> -    QTAILQ_INSERT_TAIL(&global_props, prop, next);
> +    global_props = g_list_append(global_props, prop);
>  }
>  
>  void qdev_prop_register_global_list(GlobalProperty *props)
> @@ -1028,10 +1029,11 @@ void qdev_prop_register_global_list(GlobalProperty *props)
>  
>  int qdev_prop_check_globals(void)
>  {
> -    GlobalProperty *prop;
> +    GList *l;
>      int ret = 0;
>  
> -    QTAILQ_FOREACH(prop, &global_props, next) {
> +    for (l = global_props; l; l = l->next) {
> +        GlobalProperty *prop = l->data;
>          ObjectClass *oc;
>          DeviceClass *dc;
>          if (prop->used) {
> @@ -1062,9 +1064,10 @@ int qdev_prop_check_globals(void)
>  static void qdev_prop_set_globals_for_type(DeviceState *dev,
>                                  const char *typename)
>  {
> -    GlobalProperty *prop;
> +    GList *l;
>  
> -    QTAILQ_FOREACH(prop, &global_props, next) {
> +    for (l = global_props; l; l = l->next) {
> +        GlobalProperty *prop = l->data;
>          Error *err = NULL;
>  
>          if (strcmp(typename, prop->driver) != 0) {
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index abcdee8..1fdbaaa 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -268,7 +268,6 @@ typedef struct GlobalProperty {
>      const char *value;
>      bool user_provided;
>      bool used;
> -    QTAILQ_ENTRY(GlobalProperty) next;
>  } GlobalProperty;
>  
>  /*** Board API.  This should go away once we have a machine config file.  ***/
> -- 
> 2.1.0