Mailing-List: contact commits-help@subversion.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@subversion.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1135530 - in /subversion/trunk/subversion:
 include/svn_dirent_uri.h libsvn_subr/dirent_uri.c
 tests/libsvn_subr/dirent_uri-test.c
Date: Tue, 14 Jun 2011 12:33:53 -0000
To: commits@subversion.apache.org
From: julianfoad@apache.org
Message-Id: <20110614123354.1058E2388906@eris.apache.org>

Author: julianfoad
Date: Tue Jun 14 12:33:53 2011
New Revision: 1135530

URL: http://svn.apache.org/viewvc?rev=1135530&view=rev
Log:
Simplify the semantics of svn_relpath_skip_ancestor() to return NULL if the
supposed child is not in fact a child.

* subversion/include/svn_dirent_uri.h,
  subversion/libsvn_subr/dirent_uri.c
  (svn_relpath_skip_ancestor): Return NULL if 'child' is not a child.
  (svn_fspath__skip_ancestor): Simplify this caller.

* subversion/tests/libsvn_subr/dirent_uri-test.c
  (test_relpath_skip_ancestor): Adjust the test accordingly.

Modified:
    subversion/trunk/subversion/include/svn_dirent_uri.h
    subversion/trunk/subversion/libsvn_subr/dirent_uri.c
    subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c

Modified: subversion/trunk/subversion/include/svn_dirent_uri.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_dirent_uri.h?rev=1135530&r1=1135529&r2=1135530&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_dirent_uri.h (original)
+++ subversion/trunk/subversion/include/svn_dirent_uri.h Tue Jun 14 12:33:53 2011
@@ -646,10 +646,8 @@ svn_dirent_skip_ancestor(const char *par
 
 /** Return the relative path part of @a child_relpath that is below
  * @a parent_relpath, or just "" if @a parent_relpath is equal to
- * @a child_relpath. If @a child_relpath is not below @a parent_relpath,
- * return @a child_relpath.
- *
- * ### Returning the child in the no-match case is a bad idea.
+ * @a child_relpath. If @a child_relpath is not below or equal to
+ * @a parent_relpath, return NULL.
  *
  * @since New in 1.7.
  */

Modified: subversion/trunk/subversion/libsvn_subr/dirent_uri.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/dirent_uri.c?rev=1135530&r1=1135529&r2=1135530&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/dirent_uri.c (original)
+++ subversion/trunk/subversion/libsvn_subr/dirent_uri.c Tue Jun 14 12:33:53 2011
@@ -1499,8 +1499,11 @@ svn_relpath_skip_ancestor(const char *pa
   assert(relpath_is_canonical(parent_relpath));
   assert(relpath_is_canonical(child_relpath));
 
+  if (len == 0)
+    return child_relpath;
+
   if (0 != memcmp(parent_relpath, child_relpath, len))
-    return child_relpath; /* parent_relpath is no ancestor of child_relpath */
+    return NULL; /* parent_relpath is no ancestor of child_relpath */
 
   if (child_relpath[len] == 0)
     return ""; /* parent_relpath == child_relpath */
@@ -1508,7 +1511,7 @@ svn_relpath_skip_ancestor(const char *pa
   if (child_relpath[len] == '/')
     return child_relpath + len + 1;
 
-  return child_relpath;
+  return NULL;
 }
 
 
@@ -2419,19 +2422,10 @@ const char *
 svn_fspath__skip_ancestor(const char *parent_fspath,
                           const char *child_fspath)
 {
-  const char *result;
   assert(svn_fspath__is_canonical(parent_fspath));
   assert(svn_fspath__is_canonical(child_fspath));
 
-  if (svn_relpath_is_ancestor(parent_fspath + 1, child_fspath + 1))
-    {
-      result = svn_relpath_skip_ancestor(parent_fspath + 1, child_fspath + 1);
-      assert(svn_relpath_is_canonical(result));
-    }
-  else
-    result = NULL;
-
-  return result;
+  return svn_relpath_skip_ancestor(parent_fspath + 1, child_fspath + 1);
 }
 
 svn_boolean_t

Modified: subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c?rev=1135530&r1=1135529&r2=1135530&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/dirent_uri-test.c Tue Jun 14 12:33:53 2011
@@ -1527,17 +1527,17 @@ test_relpath_skip_ancestor(apr_pool_t *p
     const char *result;
   } tests[] = {
     { "foo",             "foo/bar",        "bar"},
-    { "foo/bar",         "foot/bar",       "foot/bar"},
+    { "foo/bar",         "foot/bar",       NULL },
     { "foo",             "foo",            ""},
-    { "foo",             "foot",           "foot"},
-    { "foot",            "foo",            "foo"},
+    { "foo",             "foot",           NULL },
+    { "foot",            "foo",            NULL },
     { "",                "foo",            "foo"},
-    { "foo/bar/bla",     "foo/bar",        "foo/bar"},
+    { "foo/bar/bla",     "foo/bar",        NULL },
     { "foo/bar",         "foo/bar/bla",    "bla"},
-    { "foo/bar",         "foo",            "foo"},
+    { "foo/bar",         "foo",            NULL },
     { "",                "bar/bla",        "bar/bla"},
     { "http:/server",    "http:/server/q", "q" },
-    { "svn:/server",     "http:/server/q", "http:/server/q" },
+    { "svn:/server",     "http:/server/q", NULL },
   };
 
   for (i = 0; i < COUNT_OF(tests); i++)
@@ -1545,7 +1545,9 @@ test_relpath_skip_ancestor(apr_pool_t *p
       const char* retval;
 
       retval = svn_relpath_skip_ancestor(tests[i].path1, tests[i].path2);
-      if (strcmp(tests[i].result, retval))
+      if ((tests[i].result == NULL)
+          ? (retval != NULL)
+          : (retval == NULL || strcmp(tests[i].result, retval) != 0))
         return svn_error_createf(
              SVN_ERR_TEST_FAILED, NULL,
              "svn_relpath_skip_ancestor (%s, %s) returned %s instead of %s",