[gtk+] Bug 586374 – code does not follow documentation

[Kristian Rietveld <kristian src gnome org>]

commit d743b2fbeab01f9a9904712a5b751afd0519696d
Author: Kristian Rietveld <kris gtk org>
Date:   Thu Jul 30 21:23:47 2009 +0200

    Bug 586374 â?? code does not follow documentation
    
    Properly invalidate iterators in iter_next, iter_children,
    iter_nth_child and iter_parent when FALSE is returned.  ListStore was
    actually already mostly in order as the GSequence pointers are very
    strictly checked, but to be a little stricter we've added stamp = 0
    lines.  TreeStore did not invalidate its iterators, so stamp = 0 lines
    were added where appropriate.  Unit tests have been added to check this.
    
    Fixes bug 586374, reported by Mike Gemunde.

 gtk/gtkliststore.c    |   21 +++++++++++--
 gtk/gtktreestore.c    |   20 ++++++++++--
 gtk/tests/liststore.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++++
 gtk/tests/treestore.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 194 insertions(+), 7 deletions(-)
---
diff --git a/gtk/gtkliststore.c b/gtk/gtkliststore.c
index 470c9f8..dd28629 100644
--- a/gtk/gtkliststore.c
+++ b/gtk/gtkliststore.c
@@ -494,10 +494,16 @@ static gboolean
 gtk_list_store_iter_next (GtkTreeModel  *tree_model,
 			  GtkTreeIter   *iter)
 {
+  gboolean retval;
+
   g_return_val_if_fail (GTK_LIST_STORE (tree_model)->stamp == iter->stamp, FALSE);
   iter->user_data = g_sequence_iter_next (iter->user_data);
 
-  return !g_sequence_iter_is_end (iter->user_data);
+  retval = g_sequence_iter_is_end (iter->user_data);
+  if (retval)
+    iter->stamp = 0;
+
+  return !retval;
 }
 
 static gboolean
@@ -509,7 +515,10 @@ gtk_list_store_iter_children (GtkTreeModel *tree_model,
   
   /* this is a list, nodes have no children */
   if (parent)
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 
   if (g_sequence_get_length (list_store->seq) > 0)
     {
@@ -518,7 +527,10 @@ gtk_list_store_iter_children (GtkTreeModel *tree_model,
       return TRUE;
     }
   else
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 }
 
 static gboolean
@@ -551,6 +563,8 @@ gtk_list_store_iter_nth_child (GtkTreeModel *tree_model,
   GtkListStore *list_store = (GtkListStore *) tree_model;
   GSequenceIter *child;
 
+  iter->stamp = 0;
+
   if (parent)
     return FALSE;
 
@@ -570,6 +584,7 @@ gtk_list_store_iter_parent (GtkTreeModel *tree_model,
 			    GtkTreeIter  *iter,
 			    GtkTreeIter  *child)
 {
+  iter->stamp = 0;
   return FALSE;
 }
 
diff --git a/gtk/gtktreestore.c b/gtk/gtktreestore.c
index 1fb178d..9708408 100644
--- a/gtk/gtktreestore.c
+++ b/gtk/gtktreestore.c
@@ -620,7 +620,10 @@ gtk_tree_store_iter_next (GtkTreeModel  *tree_model,
       return TRUE;
     }
   else
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 }
 
 static gboolean
@@ -646,7 +649,10 @@ gtk_tree_store_iter_children (GtkTreeModel *tree_model,
       return TRUE;
     }
   else
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 }
 
 static gboolean
@@ -708,7 +714,10 @@ gtk_tree_store_iter_nth_child (GtkTreeModel *tree_model,
       return TRUE;
     }
   else
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 }
 
 static gboolean
@@ -733,7 +742,10 @@ gtk_tree_store_iter_parent (GtkTreeModel *tree_model,
       return TRUE;
     }
   else
-    return FALSE;
+    {
+      iter->stamp = 0;
+      return FALSE;
+    }
 }
 
 
diff --git a/gtk/tests/liststore.c b/gtk/tests/liststore.c
index 588f725..6452fc2 100644
--- a/gtk/tests/liststore.c
+++ b/gtk/tests/liststore.c
@@ -850,6 +850,72 @@ list_store_test_move_before_single (void)
   g_object_unref (store);
 }
 
+
+/* iter invalidation */
+
+static void
+list_store_test_iter_next_invalid (ListStore     *fixture,
+                                   gconstpointer  user_data)
+{
+  GtkTreePath *path;
+  GtkTreeIter iter;
+
+  path = gtk_tree_path_new_from_indices (4, -1);
+  gtk_tree_model_get_iter (GTK_TREE_MODEL (fixture->store), &iter, path);
+  gtk_tree_path_free (path);
+
+  g_assert (gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store),
+                                      &iter) == FALSE);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &iter) == FALSE);
+  g_assert (iter.stamp == 0);
+}
+
+static void
+list_store_test_iter_children_invalid (ListStore     *fixture,
+                                       gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &iter);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &iter) == TRUE);
+
+  g_assert (gtk_tree_model_iter_children (GTK_TREE_MODEL (fixture->store),
+                                          &child, &iter) == FALSE);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &child) == FALSE);
+  g_assert (child.stamp == 0);
+}
+
+static void
+list_store_test_iter_nth_child_invalid (ListStore     *fixture,
+                                        gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &iter);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &iter) == TRUE);
+
+  g_assert (gtk_tree_model_iter_nth_child (GTK_TREE_MODEL (fixture->store),
+                                           &child, &iter, 0) == FALSE);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &child) == FALSE);
+  g_assert (child.stamp == 0);
+}
+
+static void
+list_store_test_iter_parent_invalid (ListStore     *fixture,
+                                     gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &child);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &child) == TRUE);
+
+  g_assert (gtk_tree_model_iter_parent (GTK_TREE_MODEL (fixture->store),
+                                        &iter, &child) == FALSE);
+  g_assert (gtk_list_store_iter_is_valid (fixture->store, &iter) == FALSE);
+  g_assert (iter.stamp == 0);
+}
+
+
 /* main */
 
 int
@@ -958,5 +1024,19 @@ main (int    argc,
   g_test_add_func ("/list-store/move-before-single",
 		   list_store_test_move_before_single);
 
+  /* iter invalidation */
+  g_test_add ("/list-store/iter-next-invalid", ListStore, NULL,
+              list_store_setup, list_store_test_iter_next_invalid,
+              list_store_teardown);
+  g_test_add ("/list-store/iter-children-invalid", ListStore, NULL,
+              list_store_setup, list_store_test_iter_children_invalid,
+              list_store_teardown);
+  g_test_add ("/list-store/iter-nth-child-invalid", ListStore, NULL,
+              list_store_setup, list_store_test_iter_nth_child_invalid,
+              list_store_teardown);
+  g_test_add ("/list-store/iter-parent-invalid", ListStore, NULL,
+              list_store_setup, list_store_test_iter_parent_invalid,
+              list_store_teardown);
+
   return g_test_run ();
 }
diff --git a/gtk/tests/treestore.c b/gtk/tests/treestore.c
index e3a85c9..c9dbcff 100644
--- a/gtk/tests/treestore.c
+++ b/gtk/tests/treestore.c
@@ -853,6 +853,72 @@ tree_store_test_move_before_single (void)
   g_object_unref (store);
 }
 
+
+/* iter invalidation */
+
+static void
+tree_store_test_iter_next_invalid (TreeStore     *fixture,
+                                   gconstpointer  user_data)
+{
+  GtkTreePath *path;
+  GtkTreeIter iter;
+
+  path = gtk_tree_path_new_from_indices (4, -1);
+  gtk_tree_model_get_iter (GTK_TREE_MODEL (fixture->store), &iter, path);
+  gtk_tree_path_free (path);
+
+  g_assert (gtk_tree_model_iter_next (GTK_TREE_MODEL (fixture->store),
+                                      &iter) == FALSE);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &iter) == FALSE);
+  g_assert (iter.stamp == 0);
+}
+
+static void
+tree_store_test_iter_children_invalid (TreeStore     *fixture,
+                                       gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &iter);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &iter) == TRUE);
+
+  g_assert (gtk_tree_model_iter_children (GTK_TREE_MODEL (fixture->store),
+                                          &child, &iter) == FALSE);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &child) == FALSE);
+  g_assert (child.stamp == 0);
+}
+
+static void
+tree_store_test_iter_nth_child_invalid (TreeStore     *fixture,
+                                        gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &iter);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &iter) == TRUE);
+
+  g_assert (gtk_tree_model_iter_nth_child (GTK_TREE_MODEL (fixture->store),
+                                           &child, &iter, 0) == FALSE);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &child) == FALSE);
+  g_assert (child.stamp == 0);
+}
+
+static void
+tree_store_test_iter_parent_invalid (TreeStore     *fixture,
+                                     gconstpointer  user_data)
+{
+  GtkTreeIter iter, child;
+
+  gtk_tree_model_get_iter_first (GTK_TREE_MODEL (fixture->store), &child);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &child) == TRUE);
+
+  g_assert (gtk_tree_model_iter_parent (GTK_TREE_MODEL (fixture->store),
+                                        &iter, &child) == FALSE);
+  g_assert (gtk_tree_store_iter_is_valid (fixture->store, &iter) == FALSE);
+  g_assert (iter.stamp == 0);
+}
+
+
 /* main */
 
 int
@@ -961,5 +1027,19 @@ main (int    argc,
   g_test_add_func ("/tree-store/move-before-single",
 		   tree_store_test_move_before_single);
 
+  /* iter invalidation */
+  g_test_add ("/tree-store/iter-next-invalid", TreeStore, NULL,
+              tree_store_setup, tree_store_test_iter_next_invalid,
+              tree_store_teardown);
+  g_test_add ("/tree-store/iter-children-invalid", TreeStore, NULL,
+              tree_store_setup, tree_store_test_iter_children_invalid,
+              tree_store_teardown);
+  g_test_add ("/tree-store/iter-nth-child-invalid", TreeStore, NULL,
+              tree_store_setup, tree_store_test_iter_nth_child_invalid,
+              tree_store_teardown);
+  g_test_add ("/tree-store/iter-parent-invalid", TreeStore, NULL,
+              tree_store_setup, tree_store_test_iter_parent_invalid,
+              tree_store_teardown);
+
   return g_test_run ();
 }