On Wed, Mar 05, 2003 at 12:38:35PM -0600, cmpilato@tigris.org wrote:
>...
> * subversion/libsvn_fs/bdb/bdb-fs.c
> New.
> The following functions have been re-named and moved from fs.c:
> (bdb_check_already_open): was (check_already_open)
> (cleanup_fs_bdb): was (cleanup_fs_db)
> (bdb_cleanup_fs): was (cleanup_fs)
> (bdb_allocate_env): was (allocate_env)
> (bdb_print_fs_stats): was (print_fs_stats)
> (bdb_cleanup_fs_apr): was (cleanup_fs_apr)
> (bdb_set_berkeley_errcall): was (svn_fs_set_berkeley_errcall)
> (bdb_close_fs): was (svn_fs_close_fs)
> (svn_fs_create_berkeley): was (svn_fs_create_fs)
> (bdb_open_fs): was (svn_fs_open_berkeley)
> (bdb_recover_fs): was (svn_fs_berkeley_recover)
> (bdb_delete_fs): was (svn_fs_delete_berkeley)
Woah. That's not good. Those functions are visible outside of the link unit.
They should all be prefixed with svn_fs__
> * subversion/libsvn_fs/bdb/bdb-fs.h
> New. This file contains table "open function" declarations removed
> from their respective xxx-table.h include files. It also has
Why just the open functions? That seems like a strange division. The other
functions in the API are quite specific to the BDB implementation, so why
didn't they move, too? Why not just put all of the prototypes into bdb-fs.h?
I really don't understand the rationale for moving these. "but they'll go
into a vtable" Okay, but wouldn't svn_fs__bdb_delete_nodes_entry have to go
into a vtable, too? We can't have the FS calling svn_fs__bdb_* directly.
Thus, *every* function in bdb/*.h would need to be in the vtable. Which sort
of makes this move of the open stuff (seemingly) very arbitrary.
Call me -0.5 on this commit :-(
It would seem to be better to define the vtable, complete the discussion
around *that*, add it to the code, and then start migrating stuff into it.
With the current approach, a lot of stuff is shifting around, but the
reasoning isn't clear at all. And what that *really* means is that we have
no way to evaluate the *design* of this vtable stuff.
I'm highly suspect of the design right now. The FS is very dependent upon
the BDB-based design. I'd like to see and understand how that strong
coupling is going to be broken before changes like this happen. While I'm
fine with a vtable for the database portions (the "backend"), I'm really not
a fan of a vtable for the FS API itself. I don't really see a need to change
things at that level; the probability for changing the semantics is just too
great, leading to problems down the line. Just how much rope do we want to
provide?
>...
> (svn_fs_set_berkeley_errcall, svn_fs_close_fs, svn_fs_recover_fs,
> svn_fs_create_berkeley, svn_fs_open_fs, svn_fs_delete_fs): These
> are now just wrappers around new functions in bdb-fs.c which contain
> the old "guts" from these functions.
Hunh? There is no svn_fs_close_fs().
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 6 04:27:41 2003