Re: [PATCH] libsvn_fs_util: New library to reuse FSFS and BDBfunctions

On Sun, 2007-04-01 at 11:06 -0700, Daniel Rall wrote:
> On Sat, 31 Mar 2007, Bhuvaneswaran Arumugam wrote:
>
> > Hello,
> >
> > Please find attached the patch for review.
> >
> > As of today, we have defined the functions both in libsvn_fs_fs and
> > libsvn_fs_base libraries even though they are functionally identical.
> > This patch creates the initial framework to re-use the utility functions
> > used by FSFS and BDB file systems to avoid duplicate code lying in
> > libraries related to FSFS and BDB file systems.
>
> +1 on the concept. Mike's been wanting to do this for quite some
> time; it was part of the impetus for creating a new libsvn_fs_util
> module in the first place.
>
> > For now, this patch defines a new function
> > svn_fs__canonicalize_abspath() to canonicalize the absolute path. This
> > function is a replacement for svn_fs_fs__canonicalize_abspath() and
> > svn_fs_base__canonicalize_abspath() functions used internally by FSFS
> > and BDB file systems respectively. The new function is also identical to
> > the older counterpart.
> >
> > If this approach is right, we can define and re-use other possible
> > functions to avoid duplicate code lying in libraries related to FSFS and
> > BDB file systems. FWIW, I have run 'make check' with this patch and I
> > have not noticed any issues.
>
> The patch looks good to me, but I haven't tested it. Bhuvan, have you
> tested over ra_svn and ra_dav as well?

I'm testing this patch over ra_svn and ra_dav support. I'll let you know
how it goes.

> Do you have a list of other redundant functions?

Yes, I've prepared the list of other redundant functions. I found such
functions in fs.c, err.c, dag.c, id.c and lock.c belonging to FSFS and
BDB. They are as follows:

List of redundant functions in libsvn_fs_fs/fs.c & libsvn_fs_base/fs.c:
check_already_open(), check_already_open()
svn_fs_fs__canonicalize_abspath(), svn_fs_base__canonicalize_abspath()
fs_version(), base_version()

List of redundant functions in libsvn_fs_fs/dag.c &
libsvn_fs_base/dag.c:
svn_fs_fs__dag_make_file(), svn_fs_base__dag_make_file()
svn_fs_fs__dag_make_dir(), svn_fs_base__dag_make_dir()
svn_fs_fs__dag_node_kind(), svn_fs_base__dag_node_kind()
svn_fs_fs__dag_get_id(), svn_fs_fs__dag_get_id()
svn_fs_fs__dag_get_created_path(), svn_fs_base__dag_get_created_path()
svn_fs_fs__dag_get_fs(), svn_fs_base__dag_get_fs()
svn_fs_fs__dag_check_mutable, svn_fs_base__dag_check_mutable()

List of redundant functions in libsvn_fs_fs/err.c &
libsvn_fs_base/err.c:
svn_fs_fs__check_fs(), svn_fs_base__check_fs()
svn_fs_fs__err_not_mutable(), svn_fs_base__err_not_mutable()
svn_fs_fs__err_txn_not_mutable(), svn_fs_base__err_txn_not_mutable()
svn_fs_fs__err_not_directory(), svn_fs_base__err_not_directory()
svn_fs_fs__err_not_file(), svn_fs_base__err_not_file()
svn_fs_fs__err_path_already_locked(), svn_fs_base__err_path_already_locked()
svn_fs_fs__err_no_such_lock(), svn_fs_base__err_no_such_lock()
svn_fs_fs__err_lock_expired(), svn_fs_base__err_lock_expired()
svn_fs_fs__err_no_user(), svn_fs_base__err_no_user()
svn_fs_fs__err_lock_owner_mismatch(), svn_fs_base__err_lock_owner_mismatch()
svn_fs_fs__err_path_already_locked(), svn_fs_base__err_path_already_locked()
svn_fs_fs__err_no_such_lock(), svn_fs_base__err_no_such_lock()

List of redundant functions in libsvn_fs_fs/id.c & libsvn_fs_base/id.c:
svn_fs_fs__id_node_id(), svn_fs_base__id_node_id()
svn_fs_fs__id_copy_id(), svn_fs_base__id_copy_id()
svn_fs_fs__id_txn_id(), svn_fs_base__id_txn_id()

List of redundant functions in libsvn_fs_fs/lock.c &
libsvn_fs_base/lock.c:
svn_fs_fs__check_path(), base_check_path()
next_entry_name(), next_entry_name()
make_parent_path(), make_parent_path()
parent_path_path(), parent_path_path()

-- 
Regards,
Bhuvaneswaran