fs-test 14 gives a valgrind warning:

==6748== Invalid read of size 8
==6748==    at 0x587B590: verify_moves (transaction.c:3235)
==6748==    by 0x587BC03: commit_body (transaction.c:3397)
==6748==    by 0x5854151: with_some_lock_file (fs_fs.c:184)
==6748==    by 0x5854222: svn_fs_fs__with_write_lock (fs_fs.c:202)
==6748==    by 0x587CAEC: svn_fs_fs__commit (transaction.c:3626)
==6748==    by 0x5881217: svn_fs_fs__commit_txn (tree.c:2114)
==6748==    by 0x403531A: svn_fs_commit_txn2 (fs-loader.c:822)
==6748==    by 0x403543E: svn_fs_commit_txn (fs-loader.c:859)
==6748==    by 0x40D415: delete (fs-test.c:2662)
==6748==    by 0x402A0FE: do_test_num (svn_test_main.c:273)
==6748==    by 0x402AC39: main (svn_test_main.c:577)
==6748==  Address 0x6e672c8 is 8 bytes after a block of size 48 alloc'd
==6748==    at 0x4C28BED: malloc (vg_replace_malloc.c:263)
==6748==    by 0x5079DDB: pool_alloc (apr_pools.c:1463)
==6748==    by 0x5079F57: apr_palloc_debug (apr_pools.c:1504)
==6748==    by 0x506DA1E: apr_pmemdup (apr_strings.c:119)
==6748==    by 0x5874D56: fold_change (transaction.c:752)
==6748==    by 0x5874E55: process_changes (transaction.c:789)
==6748==    by 0x58750AF: svn_fs_fs__txn_changes_fetch (transaction.c:870)
==6748==    by 0x587BB9B: commit_body (transaction.c:3391)
==6748==    by 0x5854151: with_some_lock_file (fs_fs.c:184)
==6748==    by 0x5854222: svn_fs_fs__with_write_lock (fs_fs.c:202)
==6748==    by 0x587CAEC: svn_fs_fs__commit (transaction.c:3626)
==6748==    by 0x5881217: svn_fs_fs__commit_txn (tree.c:2114)

In fold_changes a key-value pair is added to the hash where the value is
of type svn_fs_path_change2_t.  In verify_moves the value is retrieved
as type change_t.

We could convert from one to other as below, but it might be better to
get fold_changes to store a change_t or have the rest of verify_moves
accept a svn_fs_path_change2_t.

Index: subversion/libsvn_fs_fs/transaction.c
===================================================================
--- subversion/libsvn_fs_fs/transaction.c       (revision 1534698)
+++ subversion/libsvn_fs_fs/transaction.c       (working copy)
@@ -3227,24 +3227,34 @@ verify_moves(svn_fs_t *fs,
 
   for (hi = apr_hash_first(pool, changed_paths); hi; hi = apr_hash_next(hi))
     {
+      const void *key;
+      apr_ssize_t len;
+      void *val;
       const char *path;
-      apr_ssize_t len;
-      change_t *change;
-      apr_hash_this(hi, (const void**)&path, &len, (void**)&change);
+      svn_fs_path_change2_t *info;
 
-      if (   change->info.copyfrom_path
-          && (   change->info.change_kind == svn_fs_path_change_move
-              || change->info.change_kind == svn_fs_path_change_movereplace))
+      apr_hash_this(hi, &key, &len, &val);
+      path = key;
+      info = val;
+
+      if (   info->copyfrom_path
+          && (   info->change_kind == svn_fs_path_change_move
+              || info->change_kind == svn_fs_path_change_movereplace))
         {
           svn_sort__item_t *item = apr_array_push(moves);
+          change_t *change = apr_palloc(pool, sizeof(change_t));
+          
+          change->path.data = path;
+          change->path.len = len;
+          change->info = *info;
           item->key = path;
           item->klen = len;
           item->value = change;
         }
 
-      if (   change->info.change_kind == svn_fs_path_change_delete
-          || change->info.change_kind == svn_fs_path_change_replace
-          || change->info.change_kind == svn_fs_path_change_movereplace)
+      if (   info->change_kind == svn_fs_path_change_delete
+          || info->change_kind == svn_fs_path_change_replace
+          || info->change_kind == svn_fs_path_change_movereplace)
         APR_ARRAY_PUSH(deletions, const char *) = path;
     }
 

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*

Reply via email to