svn commit: r983248 - /subversion/trunk/subversion/svnrdump/load_editor.c

[ar...@apache.org]

Author: artagnon
Date: Sat Aug  7 15:41:20 2010
New Revision: 983248

URL: http://svn.apache.org/viewvc?rev=983248&view=rev
Log:
svnrdump: Rework the directory baton linked-list code in the load
editor fixing crashes that went unnoticed earlier.

* subversion/svnrdump/load_editor.c (new_node_record):

  residual_path needs to be split into two separate variables:
  residual_close_path and residual_open_path because they're
  fundamentally asymmetric; (number of directories to close) is not
  necessarily equal to (number of directories to subsequently open to
  get where we want): DFS only guarantees that former number is less
  than or equal to the latter, although we're not using this property.

  Correct the arguments in the call to svn_path_compare_paths, and
  replace nb->path by the dirname of nb->path; it makes no sense to
  compare a path *with* a filename against one that does not. For
  directories, add_directory takes care of adding to the linked-list
  anyway.

  In add_directory, correct the relpath of the child_baton: there's no
  need to invoke some complex appending logic- provided that the code
  prior to reaching points works fine and calls open_directory
  appropriately, the relpath should just be the path of the directory
  in nb->path.

  When creating a new child_baton, don't forget to set the parent.

Modified:
    subversion/trunk/subversion/svnrdump/load_editor.c

Modified: subversion/trunk/subversion/svnrdump/load_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/load_editor.c?rev=983248&r1=983247&r2=983248&view=diff
==============================================================================
--- subversion/trunk/subversion/svnrdump/load_editor.c (original)
+++ subversion/trunk/subversion/svnrdump/load_editor.c Sat Aug  7 15:41:20 2010
@@ -114,7 +114,9 @@ new_node_record(void **node_baton,
   void *child_baton;
   void *commit_edit_baton;
   char *ancestor_path;
-  apr_array_header_t *residual_path;
+  apr_array_header_t *residual_open_path;
+  apr_array_header_t *residual_close_path;
+  const char *nb_dirname;
   int i;
 
   rb = revision_baton;
@@ -157,8 +159,8 @@ new_node_record(void **node_baton,
       child_db = apr_pcalloc(rb->pool, sizeof(*child_db));
       child_db->baton = child_baton;
       child_db->depth = 0;
-      child_db->parent = NULL;
       child_db->relpath = svn_relpath_canonicalize("/", rb->pool);
+      child_db->parent = NULL;
       rb->db = child_db;
   }
 
@@ -200,23 +202,28 @@ new_node_record(void **node_baton,
                                       rb->pool);
     }
 
-  if (svn_path_compare_paths(svn_relpath_dirname(nb->path, pool),
-                             rb->db->baton) != 0)
+  nb_dirname = svn_relpath_dirname(nb->path, pool);
+  if (svn_path_compare_paths(nb_dirname,
+                             rb->db->relpath) != 0)
     {
       /* Before attempting to handle the action, call open_directory
          for all the path components and set the directory baton
          accordingly */
       ancestor_path =
-        svn_relpath_get_longest_ancestor(nb->path,
+        svn_relpath_get_longest_ancestor(nb_dirname,
                                          rb->db->relpath, pool);
-      residual_path =
-        svn_path_decompose(svn_relpath_skip_ancestor(nb->path,
-                                                     ancestor_path),
+      residual_close_path =
+        svn_path_decompose(svn_relpath_skip_ancestor(ancestor_path,
+                                                     rb->db->relpath),
+                           rb->pool);
+      residual_open_path =
+        svn_path_decompose(svn_relpath_skip_ancestor(ancestor_path,
+                                                     nb_dirname),
                            rb->pool);
 
       /* First close all as many directories as there are after
          skip_ancestor, and then open fresh directories */
-      for (i = 0; i < residual_path->nelts; i ++)
+      for (i = 0; i < residual_close_path->nelts; i ++)
         {
           /* Don't worry about destroying the actual rb->db object,
              since the pool we're using has the lifetime of one
@@ -226,9 +233,9 @@ new_node_record(void **node_baton,
           rb->db = rb->db->parent;
         }
         
-      for (i = 0; i < residual_path->nelts; i ++)
+      for (i = 0; i < residual_open_path->nelts; i ++)
         {
-          SVN_ERR(commit_editor->open_directory(residual_path->elts + i,
+          SVN_ERR(commit_editor->open_directory(residual_open_path->elts + i,
                                                 rb->db->baton,
                                                 rb->rev - 1,
                                                 rb->pool, &child_baton));
@@ -237,11 +244,11 @@ new_node_record(void **node_baton,
           child_db->baton = child_baton;
           child_db->depth = rb->db->depth + 1;
           child_db->relpath = svn_relpath_join(rb->db->relpath,
-                                               residual_path->elts + i,
+                                               residual_open_path->elts + i,
                                                rb->pool);
+          child_db->parent = rb->db;
           rb->db = child_db;
         }
-
     }
 
   switch (nb->action)
@@ -265,9 +272,8 @@ new_node_record(void **node_baton,
           child_db = apr_pcalloc(rb->pool, sizeof(*child_db));
           child_db->baton = child_baton;
           child_db->depth = rb->db->depth + 1;
-          child_db->relpath = svn_relpath_join(rb->db->relpath,
-                                               residual_path->elts + i,
-                                               rb->pool);
+          child_db->relpath = apr_pstrdup(rb->pool, nb->path);
+          child_db->parent = rb->db;
           rb->db = child_db;
           break;
         default: