Philip Martin wrote: > julianf...@apache.org writes: > > > + if (txn_obj->copies) > > + { > > + int i; > > + > > + for (i = 0; i < txn_obj->copies->nelts; i++) > > + { > > Is this loop big enough to warrent an iteration pool?
It could be large in some revisions, so yes. > > + const char *txn_copy_id = APR_ARRAY_IDX(txn_obj->copies, i, > > + const char *); > > + const char *final_copy_id; > > + copy_t *copy; > > + const char *id_node_id, *id_copy_id, *id_txn_id; > > + > > + /* Get the old copy */ > > + SVN_ERR(svn_fs_bdb__get_copy(©, trail->fs, txn_copy_id, > > trail, > > + pool)); > > + > > + /* Modify it: change dst_noderev_id's txn_id to the new txn's id > > */ > > + id_node_id = svn_fs_base__id_node_id(copy->dst_noderev_id); > > + id_copy_id = svn_fs_base__id_copy_id(copy->dst_noderev_id); > > + id_txn_id = svn_fs_base__id_txn_id(copy->dst_noderev_id); > > + /* SVN_ERR_ASSERT(svn_fs_base__key_compare(id_copy_id, > > old_copy_id) > > + == 0); */ > > + /* SVN_ERR_ASSERT(svn_fs_base__key_compare(id_txn_id, old_txn_id) > > + == 0); */ > > Do those comparisons fail or what? Oh, no, I commented them out because the function didn't have the old_copy_id and old_txn_id to compare with. Now it does have the old_txn_id, and the old copy_id is called txn_copy_id. > > + copy->dst_noderev_id = svn_fs_base__id_create(id_node_id, > > + id_copy_id, > > + txn->id, > > + pool); > > + > > + /* Save the new copy */ > > + final_copy_id = id_copy_id; > > + SVN_ERR(svn_fs_bdb__create_copy(trail->fs, final_copy_id, > > + copy->src_path, copy->src_txn_id, > > + copy->dst_noderev_id, copy->kind, > > + trail, pool)); > > + } > > + } Thank you for reviewing, Philip. I'll fix pool usage and re-enable the asserts. - Julian