Any thoughts or concerns about this patch to add a path manipulation API
for '/fs/relative/style' aka '/repo/relative/style' paths ...

  svn_fspath__is_canonical()
  svn_fspath__join()
  svn_fspath__is_child()
  svn_fspath__basename()

so that we can stop abusing svn_uri_*() for this purpose, as discussed
in the thread <http://svn.haxx.se/dev/archive-2010-11/0277.shtml>?

If not, I'll commit it soon.

- Julian

Introduce a private API for manipulating FS paths that start with a '/'.
This includes only the functions that are currently needed in libsvn_fs*,
libsvn_repos and libsvn_ra*.

* subversion/include/svn_dirent_uri.h,
  subversion/libsvn_subr/dirent_uri.c
  (svn_fspath__is_canonical, svn_fspath__join, svn_fspath__is_child,
   svn_fspath__basename): New functions.

* subversion/tests/libsvn_subr/dirent_uri-test.c
  (test_fspath_is_canonical, test_fspath_join, test_fspath_is_child,
   test_fspath_basename): New tests.
  (test_funcs): Add the new tests.
--This line, and those below, will be ignored--

Index: subversion/include/svn_dirent_uri.h
===================================================================
--- subversion/include/svn_dirent_uri.h	(revision 1036564)
+++ subversion/include/svn_dirent_uri.h	(working copy)
@@ -803,6 +803,63 @@ svn_uri_get_file_url_from_dirent(const c
                                  const char *dirent,
                                  apr_pool_t *pool);
 
+
+/**
+ * A private API for Subversion filesystem paths, otherwise known as paths
+ * within a repository, that start with a '/'.  The rules for a fspath are
+ * the same as for a relpath except for the leading '/'.  A fspath never
+ * ends with '/' except when the whole path is just '/'.
+ */
+
+/** Return TRUE iff @a fspath is canonical.  Use @a scratch_pool for
+ * temporary allocations.  @a fspath need not be canonical, of course.
+ *
+ * @since New in 1.7.
+ */
+svn_boolean_t
+svn_fspath__is_canonical(const char *fspath,
+                         apr_pool_t *scratch_pool);
+
+/** Return the fspath composed of @a fspath with @a relpath appended.
+ * Allocate the result in @a result_pool.
+ *
+ * @since New in 1.7.
+ */
+char *
+svn_fspath__join(const char *fspath,
+                 const char *relpath,
+                 apr_pool_t *result_pool);
+
+
+/** Test if @a child_fspath is a child of @a parent_fspath.  If not, return
+ * NULL.  If so, return the relpath which, if joined to @a parent_fspath,
+ * would yield @a child_fspath.
+ *
+ * If @a child_fspath is the same as @a parent_fspath, it is not considered
+ * a child, so the result is NULL; an empty string is never returned.
+ *
+ * If @a pool is NULL, a pointer into @a child_fspath will be returned to
+ * identify the remainder fspath.
+ *
+ * @since New in 1.7.
+ */
+const char *
+svn_fspath__is_child(const char *parent_fspath,
+                     const char *child_fspath,
+                     apr_pool_t *pool);
+
+
+/** Return the last component of @a fspath.  The returned value will have no
+ * slashes in it.  If @a pool is NULL, return a pointer to within @a fspath,
+ * else allocate the result in @a pool.
+ *
+ * @since New in 1.7.
+ */
+const char *
+svn_fspath__basename(const char *fspath,
+                     apr_pool_t *pool);
+
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
Index: subversion/libsvn_subr/dirent_uri.c
===================================================================
--- subversion/libsvn_subr/dirent_uri.c	(revision 1036564)
+++ subversion/libsvn_subr/dirent_uri.c	(working copy)
@@ -2444,3 +2444,49 @@ svn_uri_get_file_url_from_dirent(const c
 
   return SVN_NO_ERROR;
 }
+
+
+/* ------------------------ The fspath API ------------------------ */
+
+svn_boolean_t
+svn_fspath__is_canonical(const char *fspath,
+                         apr_pool_t *scratch_pool)
+{
+  return fspath[0] == '/'
+    && svn_relpath_is_canonical(fspath + 1, scratch_pool);
+}
+
+char *
+svn_fspath__join(const char *fspath,
+                 const char *relpath,
+                 apr_pool_t *result_pool)
+{
+  assert(svn_fspath__is_canonical(fspath, result_pool));
+  assert(svn_relpath_is_canonical(relpath, result_pool));
+
+  return svn_uri_join(fspath, relpath, result_pool);
+}
+
+
+const char *
+svn_fspath__is_child(const char *parent_fspath,
+                     const char *child_fspath,
+                     apr_pool_t *pool)
+{
+  assert(svn_fspath__is_canonical(parent_fspath, pool));
+  assert(svn_fspath__is_canonical(child_fspath, pool));
+
+  return svn_relpath_is_child(parent_fspath + 1, child_fspath + 1, pool);
+}
+
+
+const char *
+svn_fspath__basename(const char *fspath,
+                     apr_pool_t *pool)
+{
+  assert(svn_fspath__is_canonical(fspath, pool));
+
+  return svn_relpath_basename(fspath + 1, pool);
+}
+
+
Index: subversion/tests/libsvn_subr/dirent_uri-test.c
===================================================================
--- subversion/tests/libsvn_subr/dirent_uri-test.c	(revision 1036564)
+++ subversion/tests/libsvn_subr/dirent_uri-test.c	(working copy)
@@ -2859,6 +2859,155 @@ test_dirent_is_under_root(apr_pool_t *po
   return SVN_NO_ERROR;
 }
 
+static svn_error_t *
+test_fspath_is_canonical(apr_pool_t *pool)
+{
+  struct {
+    const char *path;
+    svn_boolean_t canonical;
+  } tests[] = {
+    { "",                      FALSE },
+    { ".",                     FALSE },
+    { "/",                     TRUE },
+    { "/a",                    TRUE },
+    { "/a/",                   FALSE },
+    { "//a",                   FALSE },
+    { "/a/b",                  TRUE },
+    { "/a//b",                 FALSE },
+    { "\\",                    FALSE },
+    { "\\a",                   FALSE },
+    { "/\\a",                  TRUE },  /* a single component */
+    { "/a\\",                  TRUE },  /* a single component */
+    { "/a\\b",                 TRUE },  /* a single component */
+  };
+  int i;
+
+  for (i = 0; i < COUNT_OF(tests); i++)
+    {
+      svn_boolean_t canonical
+        = svn_fspath__is_canonical(tests[i].path, pool);
+
+      if (tests[i].canonical != canonical)
+        return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+                                 "svn_fspath__is_canonical(\"%s\") returned "
+                                 "\"%s\" expected \"%s\"",
+                                 tests[i].path,
+                                 canonical ? "TRUE" : "FALSE",
+                                 tests[i].canonical ? "TRUE" : "FALSE");
+    }
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+test_fspath_join(apr_pool_t *pool)
+{
+  int i;
+
+  static const char * const joins[][3] = {
+    { "/",    "",     "/" },
+    { "/",    "d",    "/d" },
+    { "/",    "d/e",  "/d/e" },
+    { "/abc", "",     "/abc" },
+    { "/abc", "d",    "/abc/d" },
+    { "/abc", "d/e",  "/abc/d/e" },
+  };
+
+  for (i = 0; i < COUNT_OF(joins); i++ )
+    {
+      const char *base = joins[i][0];
+      const char *comp = joins[i][1];
+      const char *expect = joins[i][2];
+      char *result = svn_fspath__join(base, comp, pool);
+
+      if (strcmp(result, expect))
+        return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+                                 "svn_fspath__join(\"%s\", \"%s\") returned "
+                                 "\"%s\". expected \"%s\"",
+                                 base, comp, result, expect);
+    }
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+test_fspath_is_child(apr_pool_t *pool)
+{
+  int i, j;
+
+  static const char * const paths[] = {
+    "/",
+    "/f",
+    "/foo",
+    "/foo/bar",
+    "/foo/bars",
+    "/foo/bar/baz",
+    };
+
+  static const char * const
+    remainders[COUNT_OF(paths)][COUNT_OF(paths)] = {
+    { 0,  "f",  "foo",  "foo/bar",  "foo/bars", "foo/bar/baz" },
+    { 0,  0,    0,      0,          0,          0             },
+    { 0,  0,    0,      "bar",      "bars",     "bar/baz"     },
+    { 0,  0,    0,      0,          0,          "baz"         },
+    { 0,  0,    0,      0,          0,          0             },
+    { 0,  0,    0,      0,          0,          0             },
+  };
+
+  for (i = 0; i < COUNT_OF(paths); i++)
+    {
+      for (j = 0; j < COUNT_OF(paths); j++)
+        {
+          const char *remainder
+            = svn_fspath__is_child(paths[i], paths[j], pool);
+
+          if (((remainder) && (! remainders[i][j]))
+              || ((! remainder) && (remainders[i][j]))
+              || (remainder && strcmp(remainder, remainders[i][j])))
+            return svn_error_createf
+              (SVN_ERR_TEST_FAILED, NULL,
+               "svn_fspath__is_child (%s, %s) returned '%s' instead of '%s'",
+               paths[i], paths[j],
+               remainder ? remainder : "(null)",
+               remainders[i][j] ? remainders[i][j] : "(null)" );
+        }
+    }
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+test_fspath_basename(apr_pool_t *pool)
+{
+  int i;
+
+  struct {
+    const char *path;
+    const char *result;
+  } tests[] = {
+    { "/", "" },
+    { "/a", "a" },
+    { "/abc", "abc" },
+    { "/x/abc", "abc" },
+  };
+
+  for (i = 0; i < COUNT_OF(tests); i++)
+    {
+      const char *path = tests[i].path;
+      const char *expect = tests[i].result;
+      const char *result
+        = svn_fspath__basename(path, pool);
+
+      if (strcmp(result, expect))
+        return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+                                 "svn_fspath__basename(\"%s\") returned "
+                                 "\"%s\". expected \"%s\"",
+                                 path, result, expect);
+    }
+
+  return SVN_NO_ERROR;
+}
+
 
 /* The test table.  */
 
@@ -2959,5 +3108,13 @@ struct svn_test_descriptor_t test_funcs[
                    "test svn_uri_get_file_url_from_dirent"),
     SVN_TEST_PASS2(test_dirent_is_under_root,
                    "test svn_dirent_is_under_root"),
+    SVN_TEST_PASS2(test_fspath_is_canonical,
+                   "test svn_fspath__is_canonical"),
+    SVN_TEST_PASS2(test_fspath_join,
+                   "test svn_fspath__join"),
+    SVN_TEST_PASS2(test_fspath_is_child,
+                   "test svn_fspath__is_child"),
+    SVN_TEST_PASS2(test_fspath_basename,
+                   "test svn_fspath__basename"),
     SVN_TEST_NULL
   };

Reply via email to