[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] verify at each commit

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Thu, 28 Mar 2013 00:29:24 +0200

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));
 
Received on 2013-03-27 23:30:01 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.