Patchwork [09/21] qapi: dealloc visitor, support freeing of nested lists

login
register
mail settings
Submitter Luiz Capitulino
Date Sept. 28, 2011, 2:44 p.m.
Message ID <1317221085-5825-10-git-send-email-lcapitulino@redhat.com>
Download mbox | patch
Permalink /patch/116835/
State New
Headers show

Comments

Luiz Capitulino - Sept. 28, 2011, 2:44 p.m.
From: Michael Roth <mdroth@linux.vnet.ibm.com>

Previously our logic for keeping track of when we're visiting the head
of a list was done via a global bool. This can be overwritten if dealing
with nested lists, so use stack entries to track this instead.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qapi/qapi-dealloc-visitor.c |   28 +++++++++++++++++++++-------
 1 files changed, 21 insertions(+), 7 deletions(-)
Anthony Liguori - Sept. 29, 2011, 12:51 p.m.
On 09/28/2011 09:44 AM, Luiz Capitulino wrote:
> From: Michael Roth<mdroth@linux.vnet.ibm.com>
>
> Previously our logic for keeping track of when we're visiting the head
> of a list was done via a global bool. This can be overwritten if dealing
> with nested lists, so use stack entries to track this instead.
>
> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>   qapi/qapi-dealloc-visitor.c |   28 +++++++++++++++++++++-------
>   1 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> index 6b586ad..a154523 100644
> --- a/qapi/qapi-dealloc-visitor.c
> +++ b/qapi/qapi-dealloc-visitor.c
> @@ -19,6 +19,7 @@
>   typedef struct StackEntry
>   {
>       void *value;
> +    bool is_list_head;
>       QTAILQ_ENTRY(StackEntry) node;
>   } StackEntry;
>
> @@ -39,6 +40,11 @@ static void qapi_dealloc_push(QapiDeallocVisitor *qov, void *value)
>       StackEntry *e = g_malloc0(sizeof(*e));
>
>       e->value = value;
> +
> +    /* see if we're just pushing a list head tracker */
> +    if (value == NULL) {
> +        e->is_list_head = true;
> +    }
>       QTAILQ_INSERT_HEAD(&qov->stack, e, node);
>   }
>
> @@ -72,7 +78,7 @@ static void qapi_dealloc_end_struct(Visitor *v, Error **errp)
>   static void qapi_dealloc_start_list(Visitor *v, const char *name, Error **errp)
>   {
>       QapiDeallocVisitor *qov = to_qov(v);
> -    qov->is_list_head = true;
> +    qapi_dealloc_push(qov, NULL);
>   }
>
>   static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList **listp,
> @@ -80,19 +86,27 @@ static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList **listp,
>   {
>       GenericList *list = *listp;
>       QapiDeallocVisitor *qov = to_qov(v);
> +    StackEntry *e = QTAILQ_FIRST(&qov->stack);
>
> -    if (!qov->is_list_head) {
> -        *listp = list->next;
> -        g_free(list);
> -        return *listp;
> +    if (e&&  e->is_list_head) {
> +        e->is_list_head = false;
> +        return list;
>       }
>
> -    qov->is_list_head = false;
> -    return list;
> +    if (list) {
> +        list = list->next;
> +        g_free(*listp);
> +        return list;
> +    }
> +
> +    return NULL;
>   }
>
>   static void qapi_dealloc_end_list(Visitor *v, Error **errp)
>   {
> +    QapiDeallocVisitor *qov = to_qov(v);
> +    void *obj = qapi_dealloc_pop(qov);
> +    assert(obj == NULL); /* should've been list head tracker with no payload */
>   }
>
>   static void qapi_dealloc_type_str(Visitor *v, char **obj, const char *name,

Patch

diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 6b586ad..a154523 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -19,6 +19,7 @@ 
 typedef struct StackEntry
 {
     void *value;
+    bool is_list_head;
     QTAILQ_ENTRY(StackEntry) node;
 } StackEntry;
 
@@ -39,6 +40,11 @@  static void qapi_dealloc_push(QapiDeallocVisitor *qov, void *value)
     StackEntry *e = g_malloc0(sizeof(*e));
 
     e->value = value;
+
+    /* see if we're just pushing a list head tracker */
+    if (value == NULL) {
+        e->is_list_head = true;
+    }
     QTAILQ_INSERT_HEAD(&qov->stack, e, node);
 }
 
@@ -72,7 +78,7 @@  static void qapi_dealloc_end_struct(Visitor *v, Error **errp)
 static void qapi_dealloc_start_list(Visitor *v, const char *name, Error **errp)
 {
     QapiDeallocVisitor *qov = to_qov(v);
-    qov->is_list_head = true;
+    qapi_dealloc_push(qov, NULL);
 }
 
 static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList **listp,
@@ -80,19 +86,27 @@  static GenericList *qapi_dealloc_next_list(Visitor *v, GenericList **listp,
 {
     GenericList *list = *listp;
     QapiDeallocVisitor *qov = to_qov(v);
+    StackEntry *e = QTAILQ_FIRST(&qov->stack);
 
-    if (!qov->is_list_head) {
-        *listp = list->next;
-        g_free(list);
-        return *listp;
+    if (e && e->is_list_head) {
+        e->is_list_head = false;
+        return list;
     }
 
-    qov->is_list_head = false;
-    return list;
+    if (list) {
+        list = list->next;
+        g_free(*listp);
+        return list;
+    }
+
+    return NULL;
 }
 
 static void qapi_dealloc_end_list(Visitor *v, Error **errp)
 {
+    QapiDeallocVisitor *qov = to_qov(v);
+    void *obj = qapi_dealloc_pop(qov);
+    assert(obj == NULL); /* should've been list head tracker with no payload */
 }
 
 static void qapi_dealloc_type_str(Visitor *v, char **obj, const char *name,