On Thu, 2011-07-14, Daniel Shahaf wrote:
> I've looked again at the code. Right now it opens the db, with
> svn_sqlite__mode_rwcreate, within an svn_atomic_init_once() block.
>
> Adding the special case "For <this> caller use svn_sqlite__mode_readwrite"
> means we can't use svn_atomic_init_once() any more. (since it's no
> longer "once", maybe the first call failed due to being non-rwcreate)
> And have to add some error handling logic around the open() call in that
> function.
>
> How about going for the less elegant but more readable solution,
>
> [[[
> Index: subversion/libsvn_fs_fs/rep-cache.c
> ===================================================================
> @@ -182,6 +187,11 @@ svn_fs_fs__walk_rep_reference(svn_fs_t *fs,
> }
> +
> + if (iterations == 0)
> + SVN_ERR(svn_io_remove_file2(path_rep_cache_db(fs->path, iterpool),
> + TRUE /* ignore_enoent */,
> + iterpool));
>
> SVN_ERR(svn_sqlite__reset(stmt));
>
> ]]]
>
> ?
I've just looked at the code to see what you're suggesting here. You're
suggesting to make svn_fs_fs__walk_rep_reference() delete the rep cache
DB file if it's empty. Relying on the fact that svn_fs_fs__verify() is
currently the only caller of that function. That seems too ugly.
- Julian
> > > We could make the code pass readwrite (rather than rwcreate) for that
> > > one caller, and then mask the error when the DB doesn't exist.
> >
> > +1.
> >
> > > (We do
> > > want to verify the DB if it's exists but fsfs.conf says "Don't use it",
> > > since fsfs.conf may get changed.)
> >
> > +1.
> >
> > > I'm not sure that non-creating an
> > > unused db justifies this additional code.
> >
> > I'm with Greg - it would be good for the users.
> >
> > - Julian
Received on 2011-07-19 20:13:14 CEST