Re: [Pan-devel] PAN 0.93:walk_and_add in header-pane.cc has too deep recursion

[Charles Kerr]

Sinan Karasu wrote:
> header-pane.cc 
> has the following recursive algorithm, which crashes if header pane has more 
> than about 40,000 entries.
>
>  void
>   walk_and_add (const Data                  & data,
>                 GtkTreeModel                * model,
>                 GtkTreeIter                 * parent,
>                 GtkTreeIter                 * cur,
>                 const parent_to_children_t  & mids_with_new_children,
>                 const Data::ArticleTree     * atree)
>   {
>     const Article *a (get_rec (model, cur).article);
>
>     parent_to_children_t::const_iterator i (mids_with_new_children.find 
> (a->message_id));
>     if (i != mids_with_new_children.end())
>       foreach_const (quarks_t, i->second, it)
>         add_article_to_model (data, GTK_TREE_STORE(model), cur, 
> atree->get_article(*it));
>
>     GtkTreeIter child;
>     if (gtk_tree_model_iter_children (model, &child, cur))
>       walk_and_add (data, model, cur, &child, mids_with_new_children, atree);
>
>     if (gtk_tree_model_iter_next (model, cur))
>       walk_and_add (data, model, parent, cur, mids_with_new_children, atree);
>   }
> }

And the sibling recursion is so _unnecessary_ here, too.

Thanks for catching this -- I've attached a patch.

cheers,
Charles

--- /tmp/pan-0.93/pan/gui/header-pane.cc        2006-04-14 19:17:11.000000000 
-0500
+++ header-pane.cc      2006-04-20 09:56:56.000000000 -0500
@@ -670,19 +670,26 @@
                 const parent_to_children_t  & mids_with_new_children,
                 const Data::ArticleTree     * atree)
   {
-    const Article *a (get_rec (model, cur).article);
+    for (;;)
+    {
+      // find the Article associated with this TreeNode
+      const Article *a (get_rec (model, cur).article);
 
-    parent_to_children_t::const_iterator i (mids_with_new_children.find 
(a->message_id));
-    if (i != mids_with_new_children.end())
-      foreach_const (quarks_t, i->second, it)
-        add_article_to_model (data, GTK_TREE_STORE(model), cur, 
atree->get_article(*it));
-
-    GtkTreeIter child;
-    if (gtk_tree_model_iter_children (model, &child, cur))
-      walk_and_add (data, model, cur, &child, mids_with_new_children, atree);
+      // add all of this Article's new children to the model
+      parent_to_children_t::const_iterator i (mids_with_new_children.find 
(a->message_id));
+      if (i != mids_with_new_children.end())
+        foreach_const (quarks_t, i->second, it)
+          add_article_to_model (data, GTK_TREE_STORE(model), cur, 
atree->get_article(*it));
+
+      // recurse to the children and repeat
+      GtkTreeIter child;
+      if (gtk_tree_model_iter_children (model, &child, cur))
+        walk_and_add (data, model, cur, &child, mids_with_new_children, atree);
 
-    if (gtk_tree_model_iter_next (model, cur))
-      walk_and_add (data, model, parent, cur, mids_with_new_children, atree);
+      // do the same for the siblings
+      if (!gtk_tree_model_iter_next (model, cur))
+        break;
+    }
   }
 }