Philip Martin wrote:
> julianfoad_at_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
Received on 2009-12-23 13:40:21 CET