Daniel Shahaf wrote on Wed, Mar 27, 2013 at 23:31:29 +0200: > Branko Čibej wrote on Wed, Mar 27, 2013 at 12:57:13 +0100: > > On 27.03.2013 07:54, Daniel Shahaf wrote: > > > Branko Čibej wrote on Wed, Mar 27, 2013 at 05:14:51 +0100: > > >> On 26.03.2013 23:11, Daniel Shahaf wrote: > > >>> [[[ > > >>> Run the per-revision verify code on a transaction just before it becomes > > >>> a revision. The intent is to catch corruption bugs as early as > > >>> possible. > > >>> > > >>> * subversion/libsvn_fs/fs-loader.c > > >>> (svn_fs_commit_txn): As above. > > >>> ]]] > > >>> > > >>> Maybe this should be optional behaviour in release mode, too? > > And here's the new one. (I haven't colored the section name bikeshed > yet, nor moved the new macros to svn_fs.h.) > > I wonder whether doing a verify of a revision root as the very last > thing before incrementing db/current -- specifically, in commit_body() > immediately before the sole call to write_final_current() --- would be > better. Thoughts?
I have a problem implementing that one: I'd like to open a new svn_fs_t, but the public API functions that do this are defined in a shared library I cannot call into (libsvn_fs) and the vtable function (fs.c:fs_open) should be passed the *same* COMMON_POOL algorithm that libsvn_fs uses (since it uses apr_pool_userdata_get() on that pool to access fs_fs_shared_data_t, which hosts the intra-process per-filesystem mutexes). How can fs_fs.c code open an svn_fs_t handle to another filesystem? (or an independent handle to the current filesystem) Daniel Index: subversion/libsvn_fs_fs/fs_fs.c =================================================================== --- subversion/libsvn_fs_fs/fs_fs.c (revision 1461737) +++ subversion/libsvn_fs_fs/fs_fs.c (working copy) @@ -8262,6 +8262,31 @@ write_current(svn_fs_t *fs, svn_revnum_t rev, cons return move_into_place(tmp_name, name, name, pool); } +/* */ +static svn_error_t * +verify_as_revision_before_current_plus_plus(svn_fs_t *fs, + svn_revnum_t new_rev, + apr_pool_t *pool) +{ + /* fs++ == ft */ + svn_fs_t *ft; + svn_fs_root_t *root; + + SVN_ERR(svn_fs_open(&ft, fs->path, NULL /* ### TODO fs_config */, pool)); + + /* Time travel! */ + { + fs_fs_data_t *ft_ffd = ft->fsap_data; + ft_ffd->youngest_rev_cache = new_rev; + } + + SVN_ERR(svn_fs_fs__revision_root(&root, ft, new_rev, pool)); +#if 0 + SVN_ERR(svn_fs_verify_root(root, pool)); +#endif + return SVN_NO_ERROR; +} + /* Update the 'current' file to hold the correct next node and copy_ids from transaction TXN_ID in filesystem FS. The current revision is set to REV. Perform temporary allocations in POOL. */ @@ -8532,6 +8557,7 @@ commit_body(void *baton, apr_pool_t *pool) old_rev_filename, pool)); /* Update the 'current' file. */ + SVN_ERR(verify_as_revision_before_current_plus_plus(cb->fs, new_rev, pool)); SVN_ERR(write_final_current(cb->fs, cb->txn->id, new_rev, start_node_id, start_copy_id, pool));