You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2010/05/14 13:46:12 UTC

svn commit: r944210 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

Author: gstein
Date: Fri May 14 11:46:12 2010
New Revision: 944210

URL: http://svn.apache.org/viewvc?rev=944210&view=rev
Log:
Remove an assumption about how the hash table uses the KEY parameter.
There is no guarantee that the newly-presented value will NOT be
used/retained, so we must ensure it has a proper lifetime.

* subversion/libsvn_fs_fs/fs_fs.c:
  (fold_change): always copy PATH into the target pool.

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=944210&r1=944209&r2=944210&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Fri May 14 11:46:12 2010
@@ -3828,16 +3828,13 @@ fold_change(apr_hash_t *changes,
 {
   apr_pool_t *pool = apr_hash_pool_get(changes);
   svn_fs_path_change2_t *old_change, *new_change;
-  const char *path = NULL;
+  const char *path;
 
   if ((old_change = apr_hash_get(changes, change->path, APR_HASH_KEY_STRING)))
     {
       /* This path already exists in the hash, so we have to merge
          this change into the already existing one. */
 
-      /* Since the path already exists in the hash, we don't have to
-         dup the allocation for the path itself. */
-      path = change->path;
       /* Sanity check:  only allow NULL node revision ID in the
          `reset' case. */
       if ((! change->noderev_id) && (change->kind != svn_fs_path_change_reset))
@@ -3960,13 +3957,18 @@ fold_change(apr_hash_t *changes,
           new_change->copyfrom_rev = SVN_INVALID_REVNUM;
           new_change->copyfrom_path = NULL;
         }
-      path = apr_pstrdup(pool, change->path);
     }
 
   if (new_change)
     new_change->node_kind = change->node_kind;
 
-  /* Add (or update) this path. */
+  /* Add (or update) this path.
+
+     Note: this key might already be present, and it would be nice to
+     re-use its value, but there is no way to fetch it. The API makes no
+     guarantees that this (new) key will not be retained. Thus, we (again)
+     copy the key into the target pool to ensure a proper lifetime.  */
+  path = apr_pstrdup(pool, change->path);
   apr_hash_set(changes, path, APR_HASH_KEY_STRING, new_change);
 
   /* Update the copyfrom cache, if any. */