You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2011/10/19 16:07:06 UTC

svn commit: r1186231 - in /subversion/trunk/subversion/libsvn_fs_fs: fs_fs.c tree.c tree.h

Author: danielsh
Date: Wed Oct 19 14:07:05 2011
New Revision: 1186231

URL: http://svn.apache.org/viewvc?rev=1186231&view=rev
Log:
Relax a validation to not break only on new instance of the bug it tests for,
without also croaking on every repository that contains an instance of the bug
somewhere in its history.


First, promote a public API's implementation to be library-scope:

* subversion/libsvn_fs_fs/tree.h
  (svn_fs_fs__node_id): New, implements svn_fs_node_id().

* subversion/libsvn_fs_fs/tree.c
  (fs_node_id): Rename to..
  (svn_fs_fs__node_id): .. this.
  (node_kind, fs_node_origin_rev, root_vtable): Track rename.


Second, relax the validation, using the newly-visible function:

* subversion/libsvn_fs_fs/fs_fs.c
  (validate_root_noderev): Relax validation.
    Add FS and POOL parameters.  Drop APR_INLINE marker.
  (write_final_rev):
    Update callers.

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
    subversion/trunk/subversion/libsvn_fs_fs/tree.c
    subversion/trunk/subversion/libsvn_fs_fs/tree.h

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1186231&r1=1186230&r2=1186231&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Wed Oct 19 14:07:05 2011
@@ -5837,17 +5837,50 @@ write_hash_rep(svn_filesize_t *size,
 }
 
 /* Sanity check ROOT_NODEREV, a candidate for being the root node-revision
-   of (not yet committed) revision REV.
+   of (not yet committed) revision REV in FS.  Use POOL for temporary
+   allocations.
  */
-static APR_INLINE svn_error_t *
-validate_root_noderev(node_revision_t *root_noderev,
-                      svn_revnum_t rev)
-{
-  /* Bogosity seen on svn.apache.org; see
+static svn_error_t *
+validate_root_noderev(svn_fs_t *fs,
+                      node_revision_t *root_noderev,
+                      svn_revnum_t rev,
+                      apr_pool_t *pool)
+{
+  svn_revnum_t head_revnum = rev-1;
+  int head_predecessor_count;
+
+  SVN_ERR_ASSERT(rev > 0);
+
+  /* Compute HEAD_PREDECESSOR_COUNT. */
+  {
+    svn_fs_root_t *head_revision;
+    const svn_fs_id_t *head_root_id;
+    node_revision_t *head_root_noderev;
+
+    /* Get /@HEAD's noderev. */
+    SVN_ERR(svn_fs_fs__revision_root(&head_revision, fs, head_revnum, pool));
+    SVN_ERR(svn_fs_fs__node_id(&head_root_id, head_revision, "/", pool));
+    SVN_ERR(svn_fs_fs__get_node_revision(&head_root_noderev, fs, head_root_id,
+                                         pool));
+
+    head_predecessor_count = head_root_noderev->predecessor_count;
+  }
+
+  /* Check that the root noderev's predecessor count equals REV.
+
+     This kind of corruption was seen on svn.apache.org (both on
+     the root noderev and on other fspaths' noderevs); see
        http://mid.gmane.org/20111002202833.GA12373@daniel3.local
+
+     Normally (rev == root_noderev->predecessor_count), but here we
+     use a more roundabout check that should only trigger on new instances
+     of the corruption, rather then trigger on each and every new commit
+     to a repository that has triggered the bug somewhere in its root
+     noderev's history.
    */
   if (root_noderev->predecessor_count != -1
-      && root_noderev->predecessor_count != rev)
+      && (root_noderev->predecessor_count - head_predecessor_count)
+         != (rev - head_revnum))
     {
       return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
                                _("predecessor count for "
@@ -6028,7 +6061,7 @@ write_final_rev(const svn_fs_id_t **new_
 
   /* Write out our new node-revision. */
   if (at_root)
-    SVN_ERR(validate_root_noderev(noderev, rev));
+    SVN_ERR(validate_root_noderev(fs, noderev, rev, pool));
   SVN_ERR(svn_fs_fs__write_noderev(svn_stream_from_aprfile2(file, TRUE, pool),
                                    noderev, ffd->format,
                                    svn_fs_fs__fs_supports_mergeinfo(fs),

Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1186231&r1=1186230&r2=1186231&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Wed Oct 19 14:07:05 2011
@@ -870,11 +870,11 @@ add_change(svn_fs_t *fs,
 
 /* Get the id of a node referenced by path PATH in ROOT.  Return the
    id in *ID_P allocated in POOL. */
-static svn_error_t *
-fs_node_id(const svn_fs_id_t **id_p,
-           svn_fs_root_t *root,
-           const char *path,
-           apr_pool_t *pool)
+svn_error_t *
+svn_fs_fs__node_id(const svn_fs_id_t **id_p,
+                   svn_fs_root_t *root,
+                   const char *path,
+                   apr_pool_t *pool)
 {
   if ((! root->is_txn_root)
       && (path[0] == '\0' || ((path[0] == '/') && (path[1] == '\0'))))
@@ -939,7 +939,7 @@ node_kind(svn_node_kind_t *kind_p,
   dag_node_t *node;
 
   /* Get the node id. */
-  SVN_ERR(fs_node_id(&node_id, root, path, pool));
+  SVN_ERR(svn_fs_fs__node_id(&node_id, root, path, pool));
 
   /* Use the node id to get the real kind. */
   SVN_ERR(svn_fs_fs__dag_get_node(&node, root->fs, node_id, pool));
@@ -2975,7 +2975,7 @@ fs_node_origin_rev(svn_revnum_t *revisio
   path = svn_fs__canonicalize_abspath(path, pool);
 
   /* Check the cache first. */
-  SVN_ERR(fs_node_id(&given_noderev_id, root, path, pool));
+  SVN_ERR(svn_fs_fs__node_id(&given_noderev_id, root, path, pool));
   node_id = svn_fs_fs__id_node_id(given_noderev_id);
 
   /* Is it a brand new uncommitted node? */
@@ -3047,7 +3047,7 @@ fs_node_origin_rev(svn_revnum_t *revisio
       }
 
     /* Walk the predecessor links back to origin. */
-    SVN_ERR(fs_node_id(&pred_id, curroot, lastpath->data, predidpool));
+    SVN_ERR(svn_fs_fs__node_id(&pred_id, curroot, lastpath->data, predidpool));
     do
       {
         svn_pool_clear(subpool);
@@ -3683,7 +3683,7 @@ static root_vtable_t root_vtable = {
   fs_paths_changed,
   svn_fs_fs__check_path,
   fs_node_history,
-  fs_node_id,
+  svn_fs_fs__node_id,
   svn_fs_fs__node_created_rev,
   fs_node_origin_rev,
   fs_node_created_path,

Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.h?rev=1186231&r1=1186230&r2=1186231&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.h Wed Oct 19 14:07:05 2011
@@ -61,6 +61,13 @@ svn_fs_fs__check_path(svn_node_kind_t *k
                       const char *path,
                       apr_pool_t *pool);
 
+/* Implement root_vtable_t.node_id(). */
+svn_error_t *
+svn_fs_fs__node_id(const svn_fs_id_t **id_p,
+                   svn_fs_root_t *root,
+                   const char *path,
+                   apr_pool_t *pool);
+
 /* Set *REVISION to the revision in which PATH under ROOT was created.
    Use POOL for any temporary allocations.  If PATH is in an
    uncommitted transaction, *REVISION will be set to