Re: svn commit: r22957 - trunk/subversion/libsvn_client

On Mon, 22 Jan 2007, Daniel Rall wrote:
> On Sun, 21 Jan 2007, Vlad Georgescu wrote:
>
> > Hi,
> >
> > hwright@tigris.org wrote:
> > [...]
> > > + err = setup_copy(commit_info_p,
> > > + src_paths, src_revision,
> > > + svn_path_join(dst_path, src_basename, > pool),
> > > + FALSE /* is_move */,
> > > + TRUE /* force, set to avoid deletion check */,
> > > + ctx,
> > > + subpool);
> > > }
> > >
> > > + svn_pool_destroy(subpool);
> > > +
> >
> > Looks like you're destroying the pool in which *commit_info_p was
> > allocated. (I compile APR with --enable-pool-debug, which makes it much
> > more likely that these kinds of problems will cause a segfault later
on).
> >
> > [...]
> > > + err = setup_copy(commit_info_p, src_paths, &src_revision,
> > > + svn_path_join(dst_path, src_basename, > pool),
> > > + TRUE /* is_move */,
> > > + force,
> > > + ctx,
> > > + subpool);
> > > }
> > >
> > > + svn_pool_destroy(subpool);
> > > +
> > > return err;
> > > }
> >
> > Same here.
>
> Thanks Vlad, taken care of.

Thanks for doing that, Dan. It still bugs me that we use the pool
passed to setup_copy() to allocate unbounded data structures. Would it
be possible/appropriate to use a subpool within setup_copy() itself? We
could use if for most of the operations, just making sure that we don't
allocate commit_info_p with the subpool.

Or does in that way madness lie?

-Hyrum