You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2014/01/23 13:52:02 UTC
svn commit: r1560673 - in /subversion/trunk/subversion/libsvn_fs_fs:
rep-cache.c rep-cache.h transaction.c
Author: philip
Date: Thu Jan 23 12:52:02 2014
New Revision: 1560673
URL: http://svn.apache.org/r1560673
Log:
Remove some rep-cache validation code that was not used. Change an
error to that expected by the caller, this will cause commits that
would previously have succeeded, and thus hidden the potential
future corruption, to fail.
* subversion/libsvn_fs_fs/rep-cache.c
(rep_has_been_born): Remove.
(svn_fs_fs__get_rep_reference): Return SVN_ERR_FS_CORRUPT as expected
by caller.
(svn_fs_fs__set_rep_reference): Remove parameter that is always false.
* subversion/libsvn_fs_fs/rep-cache.h
(svn_fs_fs__get_rep_reference): Document error.
(svn_fs_fs__set_rep_reference): Remove parameter, document error.
* subversion/libsvn_fs_fs/transaction.c
(write_reps_to_cache): Adjust call.
Modified:
subversion/trunk/subversion/libsvn_fs_fs/rep-cache.c
subversion/trunk/subversion/libsvn_fs_fs/rep-cache.h
subversion/trunk/subversion/libsvn_fs_fs/transaction.c
Modified: subversion/trunk/subversion/libsvn_fs_fs/rep-cache.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/rep-cache.c?rev=1560673&r1=1560672&r2=1560673&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/rep-cache.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/rep-cache.c Thu Jan 23 12:52:02 2014
@@ -50,20 +50,6 @@ path_rep_cache_db(const char *fs_path,
return svn_dirent_join(fs_path, REP_CACHE_DB_NAME, result_pool);
}
-/* Check that REP refers to a revision that exists in FS. */
-static svn_error_t *
-rep_has_been_born(representation_t *rep,
- svn_fs_t *fs,
- apr_pool_t *pool)
-{
- SVN_ERR_ASSERT(rep);
-
- SVN_ERR(svn_fs_fs__ensure_revision_exists(rep->revision, fs, pool));
-
- return SVN_NO_ERROR;
-}
-
-
/** Library-private API's. **/
@@ -294,7 +280,16 @@ svn_fs_fs__get_rep_reference(representat
SVN_ERR(svn_sqlite__reset(stmt));
if (*rep)
- SVN_ERR(rep_has_been_born(*rep, fs, pool));
+ {
+ /* Check that REP refers to a revision that exists in FS. */
+ svn_error_t *err = svn_fs_fs__ensure_revision_exists((*rep)->revision,
+ fs, pool);
+ if (err)
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, err,
+ "Checksum '%s' in rep-cache is beyond HEAD",
+ svn_checksum_to_cstring_display(checksum,
+ pool));
+ }
return SVN_NO_ERROR;
}
@@ -302,7 +297,6 @@ svn_fs_fs__get_rep_reference(representat
svn_error_t *
svn_fs_fs__set_rep_reference(svn_fs_t *fs,
representation_t *rep,
- svn_boolean_t reject_dup,
apr_pool_t *pool)
{
fs_fs_data_t *ffd = fs->fsap_data;
@@ -341,34 +335,11 @@ svn_fs_fs__set_rep_reference(svn_fs_t *f
svn_error_clear(err);
/* Constraint failed so the mapping for SHA1_CHECKSUM->REP
- should exist. If so, and the value is the same one we were
- about to write, that's cool -- just do nothing. If, however,
- the value is *different*, that's a red flag! */
+ should exist. If so that's cool -- just do nothing. If not,
+ that's a red flag! */
SVN_ERR(svn_fs_fs__get_rep_reference(&old_rep, fs, &checksum, pool));
- if (old_rep)
- {
- if (reject_dup && ((old_rep->revision != rep->revision)
- || (old_rep->item_index != rep->item_index)
- || (old_rep->size != rep->size)
- || (old_rep->expanded_size != rep->expanded_size)))
- return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
- apr_psprintf(pool,
- _("Representation key for checksum '%%s' exists "
- "in filesystem '%%s' with a different value "
- "(%%ld,%%%s,%%%s,%%%s) than what we were about "
- "to store (%%ld,%%%s,%%%s,%%%s)"),
- APR_OFF_T_FMT, SVN_FILESIZE_T_FMT,
- SVN_FILESIZE_T_FMT, APR_OFF_T_FMT,
- SVN_FILESIZE_T_FMT, SVN_FILESIZE_T_FMT),
- svn_checksum_to_cstring_display(&checksum, pool),
- fs->path, old_rep->revision, old_rep->item_index,
- old_rep->size, old_rep->expanded_size, rep->revision,
- rep->item_index, rep->size, rep->expanded_size);
- else
- return SVN_NO_ERROR;
- }
- else
+ if (!old_rep)
{
/* Something really odd at this point, we failed to insert the
checksum AND failed to read an existing checksum. Do we need
Modified: subversion/trunk/subversion/libsvn_fs_fs/rep-cache.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/rep-cache.h?rev=1560673&r1=1560672&r2=1560673&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/rep-cache.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/rep-cache.h Thu Jan 23 12:52:02 2014
@@ -61,7 +61,8 @@ svn_fs_fs__walk_rep_reference(svn_fs_t *
/* Return the representation REP in FS which has fulltext CHECKSUM.
REP is allocated in POOL. If the rep cache database has not been
- opened, just set *REP to NULL. */
+ opened, just set *REP to NULL. Returns SVN_ERR_FS_CORRUPT if
+ a reference beyond HEAD is detected. */
svn_error_t *
svn_fs_fs__get_rep_reference(representation_t **rep,
svn_fs_t *fs,
@@ -69,16 +70,13 @@ svn_fs_fs__get_rep_reference(representat
apr_pool_t *pool);
/* Set the representation REP in FS, using REP->CHECKSUM.
- Use POOL for temporary allocations.
+ Use POOL for temporary allocations. Returns SVN_ERR_FS_CORRUPT if
+ an existing reference beyond HEAD is detected.
- If the rep cache database has not been opened, this may be a no op.
-
- If REJECT_DUP is TRUE, return an error if there is an existing
- match for REP->CHECKSUM. */
+ If the rep cache database has not been opened, this may be a no op. */
svn_error_t *
svn_fs_fs__set_rep_reference(svn_fs_t *fs,
representation_t *rep,
- svn_boolean_t reject_dup,
apr_pool_t *pool);
/* Delete from the cache all reps corresponding to revisions younger
Modified: subversion/trunk/subversion/libsvn_fs_fs/transaction.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/transaction.c?rev=1560673&r1=1560672&r2=1560673&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/transaction.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/transaction.c Thu Jan 23 12:52:02 2014
@@ -3987,9 +3987,7 @@ write_reps_to_cache(svn_fs_t *fs,
{
representation_t *rep = APR_ARRAY_IDX(reps_to_cache, i, representation_t *);
- /* FALSE because we don't care if another parallel commit happened to
- * collide with us. (Non-parallel collisions will not be detected.) */
- SVN_ERR(svn_fs_fs__set_rep_reference(fs, rep, FALSE, scratch_pool));
+ SVN_ERR(svn_fs_fs__set_rep_reference(fs, rep, scratch_pool));
}
return SVN_NO_ERROR;