You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@lyra.org> on 2003/10/20 08:51:55 UTC

Re: svn commit: rev 7445 - trunk/subversion/libsvn_client

On Sat, Oct 18, 2003 at 06:15:53PM -0500, mbk@tigris.org wrote:
>...
> +++ trunk/subversion/libsvn_client/blame.c	Sat Oct 18 18:15:50 2003
>...
>    for (rev = lmb.eldest; rev; rev = rev->next)
>      {
>        const char *tmp;

You should be clearing the iterpool at the top of the loop. That pattern
is used to ensure that it always get cleared. In this particular change,
note that you won't clear the pool when last==NULL.

>        SVN_ERR (svn_io_open_unique_file (&file, &tmp, "", ".tmp",
> -                                        FALSE, subpool));
> -      stream = svn_stream_from_aprfile (file, pool);
> +                                        FALSE, pool));

The file should be created within the iterpool. Otherwise, you're creating
an unbounded number of files in the main pool (Badness).

>...

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: rev 7445 - trunk/subversion/libsvn_client

Posted by mark benedetto king <mb...@lowlatency.com>.
On Mon, Oct 20, 2003 at 01:51:55AM -0700, Greg Stein wrote:
> On Sat, Oct 18, 2003 at 06:15:53PM -0500, mbk@tigris.org wrote:
> >...
> > +++ trunk/subversion/libsvn_client/blame.c	Sat Oct 18 18:15:50 2003
> >...
> >    for (rev = lmb.eldest; rev; rev = rev->next)
> >      {
> >        const char *tmp;
> 
> You should be clearing the iterpool at the top of the loop. That pattern
> is used to ensure that it always get cleared. In this particular change,
> note that you won't clear the pool when last==NULL.

Right, which is on the first iteration, which is when there's not too
much to clear.  Still, there's no harm in clearing it at the top of
the loop.

> 
> >        SVN_ERR (svn_io_open_unique_file (&file, &tmp, "", ".tmp",
> > -                                        FALSE, subpool));
> > -      stream = svn_stream_from_aprfile (file, pool);
> > +                                        FALSE, pool));
> 
> The file should be created within the iterpool. Otherwise, you're creating
> an unbounded number of files in the main pool (Badness).
> 

I did that, initially, but that created a bug: the path name of the
previous file (which "last" is set to point to at the end of the
loop) needs to live for *two* iterations.  So, perhaps I need
two iteration pools, and alternate between them.

Note that the "obvious" solution, to use two tempory files allocated
before the loop, is wrong in several ways.

--ben


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org