svn commit: r1665456 - in /subversion/trunk/subversion: libsvn_ra/ra_loader.c libsvn_repos/replay.c tests/libsvn_ra/ra-test.c

[rhuijben]

Author: rhuijben
Date: Tue Mar 10 10:38:19 2015
New Revision: 1665456

URL: http://svn.apache.org/r1665456
Log:
Make all ra layers behave the same way on passing invalid revisions to
svn_ra_replay().

* subversion/libsvn_ra/ra_loader.c
  (svn_ra_replay): Add assertion on requirements as already checked by DAV
    and SVN.

* subversion/libsvn_repos/replay.c
  (path_driver_cb_func): Let the actual editor decide how it uses its batons.
    The standard delta editor always uses NULL batons, which doesn't make it
    invalid.
  (svn_repos_replay2): Apply sanity check instead of correcting.
  (svn_repos__replay_ev2): Use proper check.

* subversion/tests/libsvn_ra/ra-test.c
  (ra_revision_errors): Extend test.

Modified:
    subversion/trunk/subversion/libsvn_ra/ra_loader.c
    subversion/trunk/subversion/libsvn_repos/replay.c
    subversion/trunk/subversion/tests/libsvn_ra/ra-test.c

Modified: subversion/trunk/subversion/libsvn_ra/ra_loader.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra/ra_loader.c?rev=1665456&r1=1665455&r2=1665456&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra/ra_loader.c (original)
+++ subversion/trunk/subversion/libsvn_ra/ra_loader.c Tue Mar 10 10:38:19 2015
@@ -1121,6 +1121,8 @@ svn_error_t *svn_ra_replay(svn_ra_sessio
                            void *edit_baton,
                            apr_pool_t *pool)
 {
+  SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision)
+                 && SVN_IS_VALID_REVNUM(low_water_mark));
   return session->vtable->replay(session, revision, low_water_mark,
                                  text_deltas, editor, edit_baton, pool);
 }

Modified: subversion/trunk/subversion/libsvn_repos/replay.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/replay.c?rev=1665456&r1=1665455&r2=1665456&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/replay.c (original)
+++ subversion/trunk/subversion/libsvn_repos/replay.c Tue Mar 10 10:38:19 2015
@@ -555,10 +555,6 @@ path_driver_cb_func(void **dir_baton,
         return svn_error_create(SVN_ERR_FS_ALREADY_EXISTS, NULL,
                                 _("Root directory already exists."));
 
-      /* A NULL parent_baton will cause a segfault.  It should never be
-          NULL for non-root paths. */
-      SVN_ERR_ASSERT(parent_baton);
-
       /* Was this node copied? */
       SVN_ERR(fill_copyfrom(&copyfrom_root, &copyfrom_path, &copyfrom_rev,
                             &src_readable, root, change,
@@ -865,6 +861,8 @@ svn_repos_replay2(svn_fs_root_t *root,
   apr_array_header_t *paths;
   struct path_driver_cb_baton cb_baton;
 
+  SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(low_water_mark));
+
   /* Special-case r0, which we know is an empty revision; if we don't
      special-case it we might end up trying to compare it to "r-1". */
   if (svn_fs_is_revision_root(root) && svn_fs_revision_root_revision(root) == 
0)
@@ -922,11 +920,6 @@ svn_repos_replay2(svn_fs_root_t *root,
         }
     }
 
-  /* If we were not given a low water mark, assume that everything is there,
-     all the way back to revision 0. */
-  if (! SVN_IS_VALID_REVNUM(low_water_mark))
-    low_water_mark = 0;
-
   /* Initialize our callback baton. */
   cb_baton.editor = editor;
   cb_baton.edit_baton = edit_baton;
@@ -971,6 +964,8 @@ svn_repos_replay2(svn_fs_root_t *root,
   const char *repos_root = "";
   void *unlock_baton;
 
+  SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(low_water_mark));
+
   /* Special-case r0, which we know is an empty revision; if we don't
      special-case it we might end up trying to compare it to "r-1". */
   if (svn_fs_is_revision_root(root)
@@ -1493,7 +1488,7 @@ svn_repos__replay_ev2(svn_fs_root_t *roo
   svn_error_t *err = SVN_NO_ERROR;
   int i;
 
-  SVN_ERR_ASSERT(!svn_dirent_is_absolute(base_repos_relpath));
+  SVN_ERR_ASSERT(svn_relpath_is_canonical(base_repos_relpath));
 
   /* Special-case r0, which we know is an empty revision; if we don't
      special-case it we might end up trying to compare it to "r-1". */

Modified: subversion/trunk/subversion/tests/libsvn_ra/ra-test.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_ra/ra-test.c?rev=1665456&r1=1665455&r2=1665456&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_ra/ra-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_ra/ra-test.c Tue Mar 10 10:38:19 
2015
@@ -1214,7 +1214,18 @@ ra_revision_errors(const svn_test_opts_t
     SVN_TEST_ASSERT(lock == NULL);
   }
 
-  /* ### TODO: Replay and replay range */
+  {
+    SVN_TEST_ASSERT_ERROR(svn_ra_replay(ra_session, 2, 0, TRUE,
+                                        svn_delta_default_editor(pool), NULL,
+                                        pool),
+                          SVN_ERR_FS_NO_SUCH_REVISION);
+
+    /* Simply assumes everything is there*/
+    SVN_ERR(svn_ra_replay(ra_session, 1, 2, TRUE,
+                          svn_delta_default_editor(pool), NULL,
+                          pool));
+  }
+  /* ### TODO: Replay range */
 
   {
     svn_revnum_t del_rev;