subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stef...@apache.org
Subject svn commit: r1448810 - in /subversion/trunk/subversion: include/private/svn_fs_util.h libsvn_fs_fs/tree.c libsvn_fs_util/fs-util.c
Date Thu, 21 Feb 2013 21:32:26 GMT
Author: stefan2
Date: Thu Feb 21 21:32:26 2013
New Revision: 1448810

URL: http://svn.apache.org/r1448810
Log:
Follow-up to r1442088:  revise the use of open_path() and make
it require canonical values for PATH.  Eliminate the need for
svn_fs__is_canonical_abspath and consequently drop that function.
It is now an internal optimization in svn_fs__canonicalize_abspath.

All callers of svn_fs__canonicalize_abspath have been checked
whether simply returning PATH is o.k.

* subversion/include/private/svn_fs_util.h
  (svn_fs__is_canonical_abspath): drop
  (svn_fs__canonicalize_abspath): state we may simply return PATH.

* subversion/libsvn_fs_util/fs-util.c
  (svn_fs__is_canonical_abspath): rename and simplify to ...
  (is_canonical_abspath): ... this new version
  (svn_fs__canonicalize_abspath): before doing expensive operations,
   check whether canonicalization is necessary

* subversion/libsvn_fs_fs/tree.c
  (open_path_is_canonical): drop
  (open_path_node_only): renumber
  (open_path): assert() PATH parameter to be canonical
  (get_dag,
   fs_change_node_prop,
   fs_make_dir,
   fs_delete_node,
   fs_make_file,
   fs_apply_textdelta,
   fs_apply_text,
   fs_closest_copy, 
   assemble_history,
   get_mergeinfo_for_path_internal): update direct and indirect
   callers ensuring PATH is canonical

Suggested by: julianfoad

Modified:
    subversion/trunk/subversion/include/private/svn_fs_util.h
    subversion/trunk/subversion/libsvn_fs_fs/tree.c
    subversion/trunk/subversion/libsvn_fs_util/fs-util.c

Modified: subversion/trunk/subversion/include/private/svn_fs_util.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_fs_util.h?rev=1448810&r1=1448809&r2=1448810&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_fs_util.h (original)
+++ subversion/trunk/subversion/include/private/svn_fs_util.h Thu Feb 21 21:32:26 2013
@@ -37,14 +37,16 @@
 extern "C" {
 #endif /* __cplusplus */
 
-/* Return a canonicalized version of a filesystem PATH, allocated in
-   POOL.  While the filesystem API is pretty flexible about the
-   incoming paths (they must be UTF-8 with '/' as separators, but they
-   don't have to begin with '/', and multiple contiguous '/'s are
-   ignored) we want any paths that are physically stored in the
-   underlying database to look consistent.  Specifically, absolute
-   filesystem paths should begin with '/', and all redundant and trailing '/'
-   characters be removed.
+/* If the filesystem PATH is not already in canonical form,  return a
+   canonicalized version of it, allocated in POOL.  Otherwise, return
+   PATH directly.
+
+   While the filesystem API is pretty flexible about the incoming paths
+   (they must be UTF-8 with '/' as separators, but they don't have to
+   begin with '/', and multiple contiguous '/'s are ignored) we want any
+   paths that are physically stored in the underlying database to look
+   consistent.  Specifically, absolute filesystem paths should begin with
+   '/', and all redundant and trailing '/' characters be removed.
 
    This is similar to svn_fspath__canonicalize() but doesn't treat "."
    segments as special.
@@ -52,12 +54,6 @@ extern "C" {
 const char *
 svn_fs__canonicalize_abspath(const char *path, apr_pool_t *pool);
 
-/* Return FALSE, if a svn_fs__canonicalize_abspath will return a
-   different value than PATH (despite creating a copy).
-*/
-svn_boolean_t
-svn_fs__is_canonical_abspath(const char *path);
-
 /* If EXPECT_OPEN, verify that FS refers to an open database;
    otherwise, verify that FS refers to an unopened database.  Return
    an appropriate error if the expectation fails to match the

Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1448810&r1=1448809&r2=1448810&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Thu Feb 21 21:32:26 2013
@@ -857,24 +857,21 @@ typedef enum open_path_flags_t {
      has no functional impact.  */
   open_path_uncached = 2,
 
-  /* Assume that the path parameter is already in canonical form. */
-  open_path_is_canonical = 4,
-
   /* The caller does not care about the parent node chain but only
      the final DAG node. */
-  open_path_node_only = 8
+  open_path_node_only = 4
 } open_path_flags_t;
 
 
 /* Open the node identified by PATH in ROOT, allocating in POOL.  Set
    *PARENT_PATH_P to a path from the node up to ROOT.  The resulting
    **PARENT_PATH_P value is guaranteed to contain at least one
-   *element, for the root directory.
+   *element, for the root directory.  PATH must be in canonical form.
 
    If resulting *PARENT_PATH_P will eventually be made mutable and
    modified, or if copy ID inheritance information is otherwise
    needed, TXN_ID should be the ID of the mutability transaction.  If
-   TXN_ID is NULL, no copy ID in heritance information will be
+   TXN_ID is NULL, no copy ID inheritance information will be
    calculated for the *PARENT_PATH_P chain.
 
    If FLAGS & open_path_last_optional is zero, return the error
@@ -887,7 +884,8 @@ typedef enum open_path_flags_t {
 
    The remaining bits in FLAGS are hints that allow this function
    to take shortcuts based on knowledge that the caller provides,
-   such as the fact that PATH is already in canonical form.
+   such as the caller is not actually being interested in PARENT_PATH_P,
+   but only in (*PARENT_PATH_P)->NODE.
 
    NOTE: Public interfaces which only *read* from the filesystem
    should not call this function directly, but should instead use
@@ -907,21 +905,18 @@ open_path(parent_path_t **parent_path_p,
   const char *rest; /* The portion of PATH we haven't traversed yet.  */
 
   /* ensure a canonical path representation */
-  const char *canon_path = (   (flags & open_path_is_canonical)
-                            || svn_fs__is_canonical_abspath(path))
-                         ? path
-                         : svn_fs__canonicalize_abspath(path, pool);
   const char *path_so_far = "/";
   apr_pool_t *iterpool = svn_pool_create(pool);
 
   /* callers often traverse the tree in some path-based order.  That means
-     a sibbling of PATH has been resently accessed.  Try to start the lookup
+     a sibling of PATH has been presently accessed.  Try to start the lookup
      directly at the parent node, if the caller did not requested the full
      parent chain. */
   const char *directory;
+  assert(path == svn_fs__canonicalize_abspath(path, pool));
   if (flags & open_path_node_only)
     {
-      directory = svn_dirent_dirname(canon_path, pool);
+      directory = svn_dirent_dirname(path, pool);
       if (directory[1] != 0) /* root nodes are covered anyway */
         SVN_ERR(dag_node_cache_get(&here, root, directory, TRUE, pool));
     }
@@ -930,14 +925,14 @@ open_path(parent_path_t **parent_path_p,
   if (here)
     {
       path_so_far = directory;
-      rest = canon_path + strlen(directory) + 1;
+      rest = path + strlen(directory) + 1;
     }
   else
     {
       /* Make a parent_path item for the root node, using its own current
          copy id.  */
       SVN_ERR(root_node(&here, root, pool));
-      rest = canon_path + 1; /* skip the leading '/', it saves in iteration */
+      rest = path + 1; /* skip the leading '/', it saves in iteration */
     }
  
   parent_path = make_parent_path(here, 0, 0, pool);
@@ -1182,9 +1177,10 @@ get_dag(dag_node_t **dag_node_p,
   if (! node)
     {
       /* Canonicalize the input PATH. */
-      if (!svn_fs__is_canonical_abspath(path))
+      const char *canon_path = svn_fs__canonicalize_abspath(path, pool);
+      if (canon_path != path)
         {
-          path = svn_fs__canonicalize_abspath(path, pool);
+          path = canon_path;
 
           /* Try again with the corrected path. */
           SVN_ERR(dag_node_cache_get(&node, root, path, needs_lock_cache,
@@ -1196,8 +1192,8 @@ get_dag(dag_node_t **dag_node_p,
           /* Call open_path with no flags, as we want this to return an
            * error if the node for which we are searching doesn't exist. */
           SVN_ERR(open_path(&parent_path, root, path,
-                            open_path_uncached | open_path_is_canonical
-                            | open_path_node_only, NULL, pool));
+                            open_path_uncached | open_path_node_only,
+                            NULL, pool));
           node = parent_path->node;
 
           /* No need to cache our find -- open_path() will do that for us. */
@@ -1424,6 +1420,7 @@ fs_change_node_prop(svn_fs_root_t *root,
     return SVN_FS__NOT_TXN(root);
   txn_id = root->txn;
 
+  path = svn_fs__canonicalize_abspath(path, pool);
   SVN_ERR(open_path(&parent_path, root, path, 0, txn_id, pool));
 
   /* Check (non-recursively) to see if path is locked; if so, check
@@ -2195,6 +2192,7 @@ fs_make_dir(svn_fs_root_t *root,
   dag_node_t *sub_dir;
   const char *txn_id = root->txn;
 
+  path = svn_fs__canonicalize_abspath(path, pool);
   SVN_ERR(open_path(&parent_path, root, path, open_path_last_optional,
                     txn_id, pool));
 
@@ -2246,6 +2244,7 @@ fs_delete_node(svn_fs_root_t *root,
   if (! root->is_txn_root)
     return SVN_FS__NOT_TXN(root);
 
+  path = svn_fs__canonicalize_abspath(path, pool);
   SVN_ERR(open_path(&parent_path, root, path, 0, txn_id, pool));
   kind = svn_fs_fs__dag_node_kind(parent_path->node);
 
@@ -2537,8 +2536,9 @@ fs_make_file(svn_fs_root_t *root,
   dag_node_t *child;
   const char *txn_id = root->txn;
 
+  path = svn_fs__canonicalize_abspath(path, pool);
   SVN_ERR(open_path(&parent_path, root, path, open_path_last_optional,
-                    txn_id, pool));
+                   txn_id, pool));
 
   /* If there's already a file by that name, complain.
      This also catches the case of trying to make a file named `/'.  */
@@ -2846,7 +2846,7 @@ fs_apply_textdelta(svn_txdelta_window_ha
   txdelta_baton_t *tb = apr_pcalloc(pool, sizeof(*tb));
 
   tb->root = root;
-  tb->path = path;
+  tb->path = svn_fs__canonicalize_abspath(path, pool);
   tb->pool = pool;
   tb->base_checksum = svn_checksum_dup(base_checksum, pool);
   tb->result_checksum = svn_checksum_dup(result_checksum, pool);
@@ -2979,7 +2979,7 @@ fs_apply_text(svn_stream_t **contents_p,
   struct text_baton_t *tb = apr_pcalloc(pool, sizeof(*tb));
 
   tb->root = root;
-  tb->path = path;
+  tb->path = svn_fs__canonicalize_abspath(path, pool);
   tb->pool = pool;
   tb->result_checksum = svn_checksum_dup(result_checksum, pool);
 
@@ -3199,6 +3199,7 @@ static svn_error_t *fs_closest_copy(svn_
   *root_p = NULL;
   *path_p = NULL;
 
+  path = svn_fs__canonicalize_abspath(path, pool);
   SVN_ERR(open_path(&parent_path, root, path, 0, NULL, pool));
 
   /* Find the youngest copyroot in the path of this node-rev, which
@@ -3677,7 +3678,7 @@ assemble_history(svn_fs_t *fs,
 {
   svn_fs_history_t *history = apr_pcalloc(pool, sizeof(*history));
   fs_history_data_t *fhd = apr_pcalloc(pool, sizeof(*fhd));
-  fhd->path = path;
+  fhd->path = svn_fs__canonicalize_abspath(path, pool);
   fhd->revision = revision;
   fhd->is_interesting = is_interesting;
   fhd->path_hint = path_hint;
@@ -3830,8 +3831,7 @@ get_mergeinfo_for_path_internal(svn_merg
 
   path = svn_fs__canonicalize_abspath(path, scratch_pool);
 
-  SVN_ERR(open_path(&parent_path, rev_root, path, open_path_is_canonical,
-                    NULL, scratch_pool));
+  SVN_ERR(open_path(&parent_path, rev_root, path, 0, NULL, scratch_pool));
 
   if (inherit == svn_mergeinfo_nearest_ancestor && ! parent_path->parent)
     return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/libsvn_fs_util/fs-util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_util/fs-util.c?rev=1448810&r1=1448809&r2=1448810&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_util/fs-util.c (original)
+++ subversion/trunk/subversion/libsvn_fs_util/fs-util.c Thu Feb 21 21:32:26 2013
@@ -35,26 +35,15 @@
 #include "private/svn_fspath.h"
 #include "../libsvn_fs/fs-loader.h"
 
-svn_boolean_t
-svn_fs__is_canonical_abspath(const char *path)
+/* Return TRUE, if PATH (already known to start with a '/') of PATH_LEN
+ * chars does not end with a '/' and does not contain duplicate '/'.
+ */
+static svn_boolean_t
+is_canonical_abspath(const char *path, size_t path_len)
 {
-  size_t path_len;
   const char *end;
 
-  /* No PATH?  No problem. */
-  if (! path)
-    return TRUE;
-
-  /* Empty PATH?  That's just "/". */
-  if (! *path)
-    return FALSE;
-
-  /* No leading slash?  Fix that. */
-  if (*path != '/')
-    return FALSE;
-
   /* check for trailing '/' */
-  path_len = strlen(path);
   if (path_len == 1)
     return TRUE;
   if (path[path_len - 1] == '/')
@@ -83,11 +72,15 @@ svn_fs__canonicalize_abspath(const char 
 
   /* Empty PATH?  That's just "/". */
   if (! *path)
-    return apr_pstrdup(pool, "/");
+    return "/";
+
+  /* Non-trivial cases.  Maybe, the path already is canonical after all? */
+  path_len = strlen(path);
+  if (is_canonical_abspath(path, path_len))
+    return path;
 
   /* Now, the fun begins.  Alloc enough room to hold PATH with an
      added leading '/'. */
-  path_len = strlen(path);
   newpath = apr_pcalloc(pool, path_len + 2);
 
   /* No leading slash?  Fix that. */



Mime
View raw message