kfogel@collab.net wrote:
> "Max Bowsher" <maxb@ukf.net> writes:
>> Therefore I propose changing the interface from:
>> const char *svn_fs_type (svn_fs_t *fs);
>> to:
>> svn_error_t *svn_fs_type (const char **fs_type, const char *path,
>> apr_pool_t *pool);
>>
>> and replacing the current implementation with one based on the 'read
>> the fs-type file' code in fs-loader.c:fs_library_vtable(), as I
>> suggested in my previous patch.
>
> I started on this, and right away ran into a question:
>
> Should the 'path' parameter be path to repos, or path to the fs subdir
> inside the repos?
>
> At first I thought: "Obviously, it should be the repository path,
> because theoretically, if you don't know the type of the fs, you can't
> know where inside the repos to find it. Sure, currently all fs's live
> in the "db/" subdir, but maybe that won't always be true."
>
> But then I looked at some of our existing svn_fs_*() APIs that take a
> path, and they actually want the path to repos/db, not to repos. They
> don't document this clearly, but an examination of callers shows it to
> be true. See svn_fs_create() and svn_fs_open(), for example.
>
> Now I'm not sure what to do. My API design instincts say repos path,
> but my consistency instincts say fs-inside-repos path.
It has to be the path including "db/". At least theoretically, libsvn_fs
could be used free from any libsvn_repos wrapper. For convenience of the API
consumers, there could be a svn_repos_fs_type() wrapper.
Here is my patch (the docstring could use further tweaking):
Index: subversion/include/svn_fs.h
===================================================================
--- subversion/include/svn_fs.h (revision 16201)
+++ subversion/include/svn_fs.h (working copy)
@@ -170,9 +170,9 @@
apr_hash_t *config, apr_pool_t *pool);
/**
- * Return a constant string describing the back-end type of @a fs,
- * e.g. "fsfs", "bdb". If the type is not yet known, for example
- * because @a fs has been created but not opened, just return NULL.
+ * Return a string describing the back-end type of Subversion
+ * filesystem located in @a path.
+ * e.g. "fsfs", "bdb".
*
* For filesystem back ends that ship with Subversion, the string
* returned here is the same as the corresponding @c SVN_FS_TYPE_*
@@ -184,7 +184,8 @@
*
* @since New in 1.3.
*/
-const char *svn_fs_type (svn_fs_t *fs);
+svn_error_t *svn_fs_type (const char **fs_type, const char *path,
+ apr_pool_t *pool);
/**
* Return the path to @a fs's repository, allocated in @a pool.
Index: subversion/libsvn_fs/fs-loader.c
===================================================================
--- subversion/libsvn_fs/fs-loader.c (revision 16201)
+++ subversion/libsvn_fs/fs-loader.c (working copy)
@@ -163,12 +163,10 @@
_("Unknown FS type '%s'"), fs_type);
}
-/* Fetch the library vtable for an existing FS. */
-static svn_error_t *
-fs_library_vtable (fs_library_vtable_t **vtable, const char *path,
- apr_pool_t *pool)
+svn_error_t *
+svn_fs_type (const char **fs_type, const char *path, apr_pool_t *pool)
{
- const char *filename, *fs_type;
+ const char *filename;
char buf[128];
svn_error_t *err;
apr_file_t *file;
@@ -180,18 +178,29 @@
if (err && APR_STATUS_IS_ENOENT (err->apr_err))
{
svn_error_clear (err);
- fs_type = SVN_FS_TYPE_BDB;
+ *fs_type = apr_pstrdup (pool, SVN_FS_TYPE_BDB);
+ return SVN_NO_ERROR;
}
else if (err)
return err;
- else
- {
- len = sizeof(buf);
- SVN_ERR (svn_io_read_length_line (file, buf, &len, pool));
- SVN_ERR (svn_io_file_close (file, pool));
- fs_type = buf;
- }
+ len = sizeof(buf);
+ SVN_ERR (svn_io_read_length_line (file, buf, &len, pool));
+ SVN_ERR (svn_io_file_close (file, pool));
+ *fs_type = apr_pstrdup (pool, buf);
+
+ return SVN_NO_ERROR;
+}
+
+/* Fetch the library vtable for an existing FS. */
+static svn_error_t *
+fs_library_vtable (fs_library_vtable_t **vtable, const char *path,
+ apr_pool_t *pool)
+{
+ const char *fs_type;
+
+ SVN_ERR (svn_fs_type (&fs_type, path, pool));
+
/* Fetch the library vtable by name, now that we've chosen one. */
return get_library_vtable (vtable, fs_type, pool);
}
@@ -361,12 +370,6 @@
}
const char *
-svn_fs_type (svn_fs_t *fs)
-{
- return fs->type;
-}
-
-const char *
svn_fs_path (svn_fs_t *fs, apr_pool_t *pool)
{
return apr_pstrdup (pool, fs->path);
Max.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Sep 24 00:22:13 2005