hash mismatch on log-addressing branch

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*