You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2012/07/06 20:04:10 UTC
svn commit: r1358322 - /subversion/trunk/subversion/libsvn_fs_fs/tree.c
Author: stefan2
Date: Fri Jul 6 18:04:09 2012
New Revision: 1358322
URL: http://svn.apache.org/viewvc?rev=1358322&view=rev
Log:
Relax overly strict consistency check that is not covered by the FSFS
format specification.
* subversion/libsvn_fs_fs/tree.c
(svn_fs_fs__verify_root): fix test and add commentary
Modified:
subversion/trunk/subversion/libsvn_fs_fs/tree.c
Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1358322&r1=1358321&r2=1358322&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Fri Jul 6 18:04:09 2012
@@ -3881,6 +3881,7 @@ svn_fs_fs__verify_root(svn_fs_root_t *ro
const svn_fs_id_t *pred_id;
dag_node_t *pred;
svn_revnum_t pred_rev;
+ svn_revnum_t delta;
/* Only r0 should have no predecessor. */
SVN_ERR(svn_fs_fs__dag_get_predecessor_id(&pred_id, frd->root_dir));
@@ -3898,8 +3899,23 @@ svn_fs_fs__verify_root(svn_fs_root_t *ro
{
SVN_ERR(svn_fs_fs__dag_get_node(&pred, root->fs, pred_id, pool));
SVN_ERR(svn_fs_fs__dag_get_revision(&pred_rev, pred, pool));
- if (pred_rev+1 != root->rev)
- /* Issue #4129. */
+
+ /* Issue #4129: bogus predecessors. */
+ /* Check 1: predecessor must be an earlier revision.
+ */
+ if (pred_rev >= root->rev)
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
+ "r%ld's root node's predecessor is r%ld",
+ root->rev, pred_rev);
+
+ /* Check 2: distances must be a power of 2.
+ * Note that this condition is not defined by the FSFS format but
+ * merely a byproduct of the current implementation. Therefore,
+ * it may help to spot corruptions for the time being but might
+ * need to be removed / relaxed in later versions.
+ */
+ delta = root->rev - pred_rev;
+ if (delta & (delta - 1))
return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
"r%ld's root node's predecessor is r%ld",
root->rev, pred_rev);
Re: svn commit: r1358322 - /subversion/trunk/subversion/libsvn_fs_fs/tree.c
Posted by Blair Zajac <bl...@orcaware.com>.
On 7/6/12 11:04 AM, stefan2@apache.org wrote:
> Author: stefan2
> Date: Fri Jul 6 18:04:09 2012
> New Revision: 1358322
>
> URL: http://svn.apache.org/viewvc?rev=1358322&view=rev
> Log:
> Relax overly strict consistency check that is not covered by the FSFS
> format specification.
>
> * subversion/libsvn_fs_fs/tree.c
> (svn_fs_fs__verify_root): fix test and add commentary
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_fs/tree.c
>
> Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1358322&r1=1358321&r2=1358322&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Fri Jul 6 18:04:09 2012
> @@ -3881,6 +3881,7 @@ svn_fs_fs__verify_root(svn_fs_root_t *ro
> const svn_fs_id_t *pred_id;
> dag_node_t *pred;
> svn_revnum_t pred_rev;
> + svn_revnum_t delta;
>
> /* Only r0 should have no predecessor. */
> SVN_ERR(svn_fs_fs__dag_get_predecessor_id(&pred_id, frd->root_dir));
> @@ -3898,8 +3899,23 @@ svn_fs_fs__verify_root(svn_fs_root_t *ro
> {
> SVN_ERR(svn_fs_fs__dag_get_node(&pred, root->fs, pred_id, pool));
> SVN_ERR(svn_fs_fs__dag_get_revision(&pred_rev, pred, pool));
> - if (pred_rev+1 != root->rev)
> - /* Issue #4129. */
> +
> + /* Issue #4129: bogus predecessors. */
> + /* Check 1: predecessor must be an earlier revision.
> + */
> + if (pred_rev >= root->rev)
> + return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
> + "r%ld's root node's predecessor is r%ld",
> + root->rev, pred_rev);
Suggest describing in the error message the condition that failed.
> +
> + /* Check 2: distances must be a power of 2.
> + * Note that this condition is not defined by the FSFS format but
> + * merely a byproduct of the current implementation. Therefore,
> + * it may help to spot corruptions for the time being but might
> + * need to be removed / relaxed in later versions.
> + */
> + delta = root->rev - pred_rev;
> + if (delta & (delta - 1))
> return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
> "r%ld's root node's predecessor is r%ld",
> root->rev, pred_rev);
Same here. This will make it easier for people who are not familiar with fsfs's
internals to see which condition failed.
Blair