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
