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(&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


Reply via email to