[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r40218 - trunk/subversion/svn

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Sat, 24 Oct 2009 23:42:52 +0100

Greg Stein wrote:
> On Sat, Oct 24, 2009 at 08:35, Hyrum K. Wright <hyrum_at_hyrumwright.org> wrote:
> >...
> > +++ trunk/subversion/svn/proplist-cmd.c Sat Oct 24 05:35:12 2009 (r40218)
> >...
> > else /* operate on normal, versioned properties (not revprops) */
> > {
> > - apr_pool_t *subpool = svn_pool_create(pool);
> > + apr_pool_t *iterpool;
> > svn_proplist_receiver_t pl_receiver;
> >
> > if (opt_state->xml)
> > {
> > - SVN_ERR(svn_cl__xml_print_header("properties", pool));
> > + SVN_ERR(svn_cl__xml_print_header("properties", scratch_pool));
> > pl_receiver = proplist_receiver_xml;
> > }
> > else
> > @@ -181,6 +180,7 @@ svn_cl__proplist(apr_getopt_t *os,
> > if (opt_state->depth == svn_depth_unknown)
> > opt_state->depth = svn_depth_empty;
> >
> > + iterpool = svn_pool_create(scratch_pool);
> > for (i = 0; i < targets->nelts; i++)
>
> If you construct the iterpool in the declaration (as before), then you
> can use it for the call to svn_cl__xml_print_header(), as its
> scratch_pool. Any mem used by the call will be cleared on the first
> iteration of the loop.
>
> >...
> > @@ -204,17 +204,16 @@ svn_cl__proplist(apr_getopt_t *os,
> > opt_state->depth,
> > opt_state->changelists,
> > pl_receiver, &pl_baton,
> > - ctx, subpool),
> > + ctx, iterpool),
> > NULL, opt_state->quiet,
> > SVN_ERR_UNVERSIONED_RESOURCE,
> > SVN_ERR_ENTRY_NOT_FOUND,
> > SVN_NO_ERROR));
> > }
> > + svn_pool_destroy(iterpool);
> >
> > if (opt_state->xml)
> > - SVN_ERR(svn_cl__xml_print_footer("properties", pool));
> > -
> > - svn_pool_destroy(subpool);
> > + SVN_ERR(svn_cl__xml_print_footer("properties", scratch_pool));
> > }
> >
> > return SVN_NO_ERROR;
>
> And if you don't destroy it so soon, then you get to use it for that
> last call, too.
[...]

That's true, there is the opportunity to do that, but is it really worth
breaking the well-defined "iterpool" pattern for such a little extra
optimisation? I don't think so. Keep it simple.

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2411102
Received on 2009-10-25 00:43:09 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.