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 };